Skip to content

Commit

Permalink
[BugFix] fix incorrect param type in some sorting interface (StarRock…
Browse files Browse the repository at this point in the history
  • Loading branch information
silverbullet233 authored Mar 7, 2023
1 parent f0195f5 commit 1854003
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 40 deletions.
16 changes: 8 additions & 8 deletions be/src/exec/sorting/sort_column.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ namespace starrocks {
// Sort a column by permtuation
class ColumnSorter final : public ColumnVisitorAdapter<ColumnSorter> {
public:
explicit ColumnSorter(const bool& cancel, const SortDesc& sort_desc, SmallPermutation& permutation, Tie& tie,
std::pair<int, int> range, bool build_tie)
explicit ColumnSorter(const std::atomic<bool>& cancel, const SortDesc& sort_desc, SmallPermutation& permutation,
Tie& tie, std::pair<int, int> range, bool build_tie)
: ColumnVisitorAdapter(this),
_cancel(cancel),
_sort_desc(sort_desc),
Expand Down Expand Up @@ -143,7 +143,7 @@ class ColumnSorter final : public ColumnVisitorAdapter<ColumnSorter> {
}

private:
const bool& _cancel;
const std::atomic<bool>& _cancel;
const SortDesc& _sort_desc;
SmallPermutation& _permutation;
Tie& _tie;
Expand Down Expand Up @@ -392,13 +392,13 @@ class VerticalColumnSorter final : public ColumnVisitorAdapter<VerticalColumnSor
size_t _pruned_limit; // The pruned limit during partial sorting
};

Status sort_and_tie_column(const bool cancel, const ColumnPtr& column, const SortDesc& sort_desc,
Status sort_and_tie_column(const std::atomic<bool>& cancel, const ColumnPtr& column, const SortDesc& sort_desc,
SmallPermutation& permutation, Tie& tie, std::pair<int, int> range, bool build_tie) {
ColumnSorter column_sorter(cancel, sort_desc, permutation, tie, range, build_tie);
return column->accept(&column_sorter);
}

Status sort_and_tie_column(const bool cancel, ColumnPtr& column, const SortDesc& sort_desc,
Status sort_and_tie_column(const std::atomic<bool>& cancel, ColumnPtr& column, const SortDesc& sort_desc,
SmallPermutation& permutation, Tie& tie, std::pair<int, int> range, bool build_tie) {
if (column->is_nullable() && !column->is_constant()) {
ColumnHelper::as_column<NullableColumn>(column)->fill_null_with_default();
Expand All @@ -407,7 +407,7 @@ Status sort_and_tie_column(const bool cancel, ColumnPtr& column, const SortDesc&
return column->accept(&column_sorter);
}

Status sort_and_tie_columns(const bool cancel, const Columns& columns, const SortDescs& sort_desc,
Status sort_and_tie_columns(const std::atomic<bool>& cancel, const Columns& columns, const SortDescs& sort_desc,
Permutation* permutation) {
if (columns.size() < 1) {
return Status::OK();
Expand All @@ -429,7 +429,7 @@ Status sort_and_tie_columns(const bool cancel, const Columns& columns, const Sor
return Status::OK();
}

Status stable_sort_and_tie_columns(const bool cancel, const Columns& columns, const SortDescs& sort_desc,
Status stable_sort_and_tie_columns(const std::atomic<bool>& cancel, const Columns& columns, const SortDescs& sort_desc,
SmallPermutation* small_perm) {
if (columns.size() < 1) {
return Status::OK();
Expand All @@ -451,7 +451,7 @@ Status stable_sort_and_tie_columns(const bool cancel, const Columns& columns, co
int range_first = ti.range_first, range_last = ti.range_last;
if (range_last - range_first > 1) {
::pdqsort(
cancel, small_perm->begin() + range_first, small_perm->begin() + range_last,
small_perm->begin() + range_first, small_perm->begin() + range_last,
[](SmallPermuteItem lhs, SmallPermuteItem rhs) { return lhs.index_in_chunk < rhs.index_in_chunk; });
}
}
Expand Down
8 changes: 4 additions & 4 deletions be/src/exec/sorting/sort_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ static inline Status sort_and_tie_helper_nullable(const std::atomic<bool>& cance
}

template <class DataComparator, class PermutationType>
static inline Status sort_and_tie_helper(const bool& cancel, const Column* column, bool is_asc_order,
static inline Status sort_and_tie_helper(const std::atomic<bool>& cancel, const Column* column, bool is_asc_order,
PermutationType& permutation, Tie& tie, DataComparator cmp,
std::pair<int, int> range, bool build_tie, size_t limit = 0,
size_t* limited = nullptr) {
Expand Down Expand Up @@ -218,16 +218,16 @@ static inline Status sort_and_tie_helper(const bool& cancel, const Column* colum
*limited = limit + equal_count;
} else {
if (is_asc_order) {
::pdqsort(cancel, begin, end, lesser);
::pdqsort(begin, end, lesser);
} else {
::pdqsort(cancel, begin, end, greater);
::pdqsort(begin, end, greater);
}
}
};

TieIterator iterator(tie, range.first, range.second);
while (iterator.next()) {
if (UNLIKELY(cancel)) {
if (UNLIKELY(cancel.load(std::memory_order_acquire))) {
return Status::Cancelled("Sort cancelled");
}
int range_first = iterator.range_first;
Expand Down
8 changes: 4 additions & 4 deletions be/src/exec/sorting/sorting.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,17 @@ struct SortDescs;
// @param permutation input and output permutation
// @param tie input and output tie
// @param range sort range, {0, 0} means not build tie but sort data
Status sort_and_tie_column(const bool cancel, ColumnPtr& column, const SortDesc& sort_desc,
Status sort_and_tie_column(const std::atomic<bool>& cancel, ColumnPtr& column, const SortDesc& sort_desc,
SmallPermutation& permutation, Tie& tie, std::pair<int, int> range, const bool build_tie);
Status sort_and_tie_column(const bool cancel, const ColumnPtr& column, const SortDesc& sort_desc,
Status sort_and_tie_column(const std::atomic<bool>& cancel, const ColumnPtr& column, const SortDesc& sort_desc,
SmallPermutation& permutation, Tie& tie, std::pair<int, int> range, const bool build_tie);

// Sort multiple columns using column-wise algorithm, output the order in permutation array
Status sort_and_tie_columns(const bool cancel, const Columns& columns, const SortDescs& sort_desc,
Status sort_and_tie_columns(const std::atomic<bool>& cancel, const Columns& columns, const SortDescs& sort_desc,
Permutation* permutation);

// Sort multiple columns, and stable
Status stable_sort_and_tie_columns(const bool cancel, const Columns& columns, const SortDescs& sort_desc,
Status stable_sort_and_tie_columns(const std::atomic<bool>& cancel, const Columns& columns, const SortDescs& sort_desc,
SmallPermutation* permutation);

// Sort multiple columns in vertical
Expand Down
4 changes: 2 additions & 2 deletions be/src/exprs/agg/percentile_cont.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class PercentileContAggregateFunction final
memcpy(bytes.data() + old_size + sizeof(double), &items_size, sizeof(size_t));
memcpy(bytes.data() + old_size + sizeof(double) + sizeof(size_t), this->data(state).items.data(),
items_size * sizeof(InputCppType));
pdqsort(false, reinterpret_cast<InputCppType*>(bytes.data() + old_size + sizeof(double) + sizeof(size_t)),
pdqsort(reinterpret_cast<InputCppType*>(bytes.data() + old_size + sizeof(double) + sizeof(size_t)),
reinterpret_cast<InputCppType*>(bytes.data() + old_size + sizeof(double) + sizeof(size_t) +
items_size * sizeof(InputCppType)));

Expand All @@ -111,7 +111,7 @@ class PercentileContAggregateFunction final
void finalize_to_column(FunctionContext* ctx, ConstAggDataPtr __restrict state, Column* to) const override {
using CppType = RunTimeCppType<LT>;
std::vector<CppType> new_vector = this->data(state).items;
pdqsort(false, new_vector.begin(), new_vector.end());
pdqsort(new_vector.begin(), new_vector.end());
const double& rate = this->data(state).rate;

ResultColumnType* column = down_cast<ResultColumnType*>(to);
Expand Down
4 changes: 2 additions & 2 deletions be/src/exprs/array_functions.tpp
Original file line number Diff line number Diff line change
Expand Up @@ -535,14 +535,14 @@ protected:
const auto& data = down_cast<const ColumnType&>(src_column).get_data();

auto less_fn = [&data](uint32_t l, uint32_t r) -> bool { return data[l] < data[r]; };
pdqsort(false, sort_index->begin() + offset, sort_index->begin() + offset + count, less_fn);
pdqsort(sort_index->begin() + offset, sort_index->begin() + offset + count, less_fn);
}

// For JSON type
static void _sort_column(std::vector<uint32_t>* sort_index, const JsonColumn& src_column, size_t offset,
size_t count) {
auto less_fn = [&](uint32_t l, uint32_t r) -> bool { return src_column.compare_at(l, r, src_column, -1) < 0; };
pdqsort(false, sort_index->begin() + offset, sort_index->begin() + offset + count, less_fn);
pdqsort(sort_index->begin() + offset, sort_index->begin() + offset + count, less_fn);
}

static void _sort_item(std::vector<uint32_t>* sort_index, const Column& src_column,
Expand Down
30 changes: 12 additions & 18 deletions be/src/util/orlp/pdqsort.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ applications, and to alter it and redistribute it freely, subject to the followi
#pragma once

#include <algorithm>
#include <atomic>
#include <cstddef>
#include <functional>
#include <iterator>
Expand Down Expand Up @@ -465,14 +466,9 @@ inline Iter partition_left(Iter begin, Iter end, Compare comp) {
}

template <class Iter, class Compare, bool Branchless>
inline void pdqsort_loop(const bool& is_canceled, Iter begin, Iter end, Compare comp, int bad_allowed,
bool leftmost = true) {
inline void pdqsort_loop(Iter begin, Iter end, Compare comp, int bad_allowed, bool leftmost = true) {
typedef typename std::iterator_traits<Iter>::difference_type diff_t;

if (UNLIKELY(is_canceled)) {
return;
}

// Use a while loop for tail recursion elimination.
while (true) {
diff_t size = end - begin;
Expand Down Expand Up @@ -560,45 +556,43 @@ inline void pdqsort_loop(const bool& is_canceled, Iter begin, Iter end, Compare

// Sort the left partition first using recursion and do tail recursion elimination for
// the right-hand partition.
pdqsort_loop<Iter, Compare, Branchless>(is_canceled, begin, pivot_pos, comp, bad_allowed, leftmost);
pdqsort_loop<Iter, Compare, Branchless>(begin, pivot_pos, comp, bad_allowed, leftmost);
begin = pivot_pos + 1;
leftmost = false;
}
}
} // namespace pdqsort_detail

template <class Iter, class Compare>
inline void pdqsort(const bool& is_cancelled, Iter begin, Iter end, Compare comp) {
inline void pdqsort(Iter begin, Iter end, Compare comp) {
if (begin == end) return;

#if __cplusplus >= 201103L
pdqsort_detail::pdqsort_loop<Iter, Compare,
pdqsort_detail::is_default_compare<typename std::decay<Compare>::type>::value &&
std::is_arithmetic<typename std::iterator_traits<Iter>::value_type>::value>(
is_cancelled, begin, end, comp, pdqsort_detail::log2(end - begin));
begin, end, comp, pdqsort_detail::log2(end - begin));
#else
pdqsort_detail::pdqsort_loop<Iter, Compare, false>(is_canceled, begin, end, comp,
pdqsort_detail::log2(end - begin));
pdqsort_detail::pdqsort_loop<Iter, Compare, false>(begin, end, comp, pdqsort_detail::log2(end - begin));
#endif
}

template <class Iter>
inline void pdqsort(const bool& is_cancelled, Iter begin, Iter end) {
inline void pdqsort(Iter begin, Iter end) {
typedef typename std::iterator_traits<Iter>::value_type T;
pdqsort(is_cancelled, begin, end, std::less<T>());
pdqsort(begin, end, std::less<T>());
}

template <class Iter, class Compare>
inline void pdqsort_branchless(const bool& is_cancelled, Iter begin, Iter end, Compare comp) {
inline void pdqsort_branchless(Iter begin, Iter end, Compare comp) {
if (begin == end) return;
pdqsort_detail::pdqsort_loop<Iter, Compare, true>(is_cancelled, begin, end, comp,
pdqsort_detail::log2(end - begin));
pdqsort_detail::pdqsort_loop<Iter, Compare, true>(begin, end, comp, pdqsort_detail::log2(end - begin));
}

template <class Iter>
inline void pdqsort_branchless(const bool& is_cancelled, Iter begin, Iter end) {
inline void pdqsort_branchless(Iter begin, Iter end) {
typedef typename std::iterator_traits<Iter>::value_type T;
pdqsort_branchless(is_cancelled, begin, end, std::less<T>());
pdqsort_branchless(begin, end, std::less<T>());
}

#undef PDQSORT_PREFER_MOVE
3 changes: 1 addition & 2 deletions be/src/util/tdigest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,7 @@ void TDigest::updateCumulative() {

void TDigest::process() {
CentroidComparator cc;
pdqsort(false, _unprocessed.begin(), _unprocessed.end(),
[&](auto& lhs, auto& rhs) { return lhs.mean() < rhs.mean(); });
pdqsort(_unprocessed.begin(), _unprocessed.end(), [&](auto& lhs, auto& rhs) { return lhs.mean() < rhs.mean(); });
auto count = _unprocessed.size();
_unprocessed.insert(_unprocessed.end(), _processed.cbegin(), _processed.cend());
std::inplace_merge(_unprocessed.begin(), _unprocessed.begin() + count, _unprocessed.end(), cc);
Expand Down

0 comments on commit 1854003

Please sign in to comment.