Skip to content

Commit 6bcd55e

Browse files
authored
[Chore](sort) remove SortingQueueStrategy::Default (#59279)
### What problem does this PR solve? This pull request refactors the sorting queue implementation in `be/src/vec/core/sort_cursor.h` to simplify the codebase by removing the `SortingQueueStrategy` enum and the related template specialization logic. The new approach introduces a dedicated `SortingQueueBatch` class, streamlining the sorting queue logic and reducing code complexity. **Refactoring and simplification of sorting queue logic:** * Removed the `SortingQueueStrategy` enum and the `SortingQueueImpl` template specialization, replacing them with a single `SortingQueueBatch` class that handles batch sorting logic directly. * Consolidated the `current()` and `next()` methods to remove conditional compilation and strategy checks, simplifying their interfaces for batch processing. * Removed unnecessary `if constexpr` blocks and strategy-dependent code, making batch size updates unconditional where appropriate. [[1]](diffhunk://#diff-6e09b43a2f90fe07546a69275b5e29af2e1f35e86be173b662b12192996b54f7L337-L354) [[2]](diffhunk://#diff-6e09b43a2f90fe07546a69275b5e29af2e1f35e86be173b662b12192996b54f7L393-L395) [[3]](diffhunk://#diff-6e09b43a2f90fe07546a69275b5e29af2e1f35e86be173b662b12192996b54f7L427-L430) * Updated type aliases to remove references to the old `SortingQueueImpl` and strategy-based specializations, leaving only the new `SortingQueueBatch`. **Minor cleanup:** * Removed redundant default initialization of `_block_supplier` in `BlockSupplierSortCursorImpl`. ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
1 parent d102aee commit 6bcd55e

File tree

1 file changed

+17
-57
lines changed

1 file changed

+17
-57
lines changed

be/src/vec/core/sort_cursor.h

Lines changed: 17 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ struct BlockSupplierSortCursorImpl : public MergeSortCursorImpl {
164164
bool eof() const override { return is_last(0) && _is_eof; }
165165

166166
VExprContextSPtrs _ordering_expr;
167-
BlockSupplier _block_supplier {};
167+
BlockSupplier _block_supplier;
168168
bool _is_eof = false;
169169
};
170170

@@ -251,16 +251,14 @@ struct MergeSortBlockCursor {
251251
}
252252
};
253253

254-
enum class SortingQueueStrategy : uint8_t { Default, Batch };
255-
256254
/// Allows to fetch data from multiple sort cursors in sorted order (merging sorted data streams).
257-
template <typename Cursor, SortingQueueStrategy strategy>
258-
class SortingQueueImpl {
255+
template <typename Cursor>
256+
class SortingQueueBatch {
259257
public:
260-
SortingQueueImpl() = default;
258+
SortingQueueBatch() = default;
261259

262260
template <typename Cursors>
263-
explicit SortingQueueImpl(Cursors& cursors) {
261+
explicit SortingQueueBatch(Cursors& cursors) {
264262
size_t size = cursors.size();
265263
_queue.reserve(size);
266264

@@ -270,47 +268,20 @@ class SortingQueueImpl {
270268

271269
std::make_heap(_queue.begin(), _queue.end());
272270

273-
if constexpr (strategy == SortingQueueStrategy::Batch) {
274-
if (!_queue.empty()) {
275-
update_batch_size();
276-
}
271+
if (!_queue.empty()) {
272+
update_batch_size();
277273
}
278274
}
279275

280276
bool is_valid() const { return !_queue.empty(); }
281277

282-
Cursor& current()
283-
requires(strategy == SortingQueueStrategy::Default)
284-
{
285-
return &_queue.front();
286-
}
287-
288-
std::pair<Cursor*, size_t> current()
289-
requires(strategy == SortingQueueStrategy::Batch)
290-
{
291-
return {&_queue.front(), batch_size};
292-
}
278+
std::pair<Cursor*, size_t> current() { return {&_queue.front(), batch_size}; }
293279

294280
size_t size() { return _queue.size(); }
295281

296282
Cursor& next_child() { return _queue[next_child_index()]; }
297283

298-
void ALWAYS_INLINE next()
299-
requires(strategy == SortingQueueStrategy::Default)
300-
{
301-
assert(is_valid());
302-
303-
if (!_queue.front()->is_last()) {
304-
_queue.front()->next();
305-
update_top(true);
306-
} else {
307-
remove_top();
308-
}
309-
}
310-
311-
void ALWAYS_INLINE next(size_t batch_size_value)
312-
requires(strategy == SortingQueueStrategy::Batch)
313-
{
284+
void ALWAYS_INLINE next(size_t batch_size_value) {
314285
assert(is_valid());
315286
assert(batch_size_value <= batch_size);
316287
assert(batch_size_value > 0);
@@ -334,12 +305,10 @@ class SortingQueueImpl {
334305
_queue.pop_back();
335306
next_child_idx = 0;
336307

337-
if constexpr (strategy == SortingQueueStrategy::Batch) {
338-
if (_queue.empty()) {
339-
batch_size = 0;
340-
} else {
341-
update_batch_size();
342-
}
308+
if (_queue.empty()) {
309+
batch_size = 0;
310+
} else {
311+
update_batch_size();
343312
}
344313
}
345314

@@ -348,9 +317,7 @@ class SortingQueueImpl {
348317
std::push_heap(_queue.begin(), _queue.end());
349318
next_child_idx = 0;
350319

351-
if constexpr (strategy == SortingQueueStrategy::Batch) {
352-
update_batch_size();
353-
}
320+
update_batch_size();
354321
}
355322

356323
private:
@@ -390,9 +357,7 @@ class SortingQueueImpl {
390357

391358
/// Check if we are in order.
392359
if (check_in_order && (*child_it).greater(*begin)) {
393-
if constexpr (strategy == SortingQueueStrategy::Batch) {
394-
update_batch_size();
395-
}
360+
update_batch_size();
396361
return;
397362
}
398363

@@ -424,9 +389,7 @@ class SortingQueueImpl {
424389
} while (!((*child_it).greater(top)));
425390
*curr_it = std::move(top);
426391

427-
if constexpr (strategy == SortingQueueStrategy::Batch) {
428-
update_batch_size();
429-
}
392+
update_batch_size();
430393
}
431394

432395
/// Update batch size of elements that client can extract from current cursor
@@ -471,9 +434,6 @@ class SortingQueueImpl {
471434
}
472435
}
473436
};
474-
template <typename Cursor>
475-
using SortingQueue = SortingQueueImpl<Cursor, SortingQueueStrategy::Default>;
476-
template <typename Cursor>
477-
using SortingQueueBatch = SortingQueueImpl<Cursor, SortingQueueStrategy::Batch>;
437+
478438
#include "common/compile_check_end.h"
479439
} // namespace doris::vectorized

0 commit comments

Comments
 (0)