Skip to content

Conversation

@ygerzhedovich
Copy link
Contributor

No description provided.

@ygerzhedovich ygerzhedovich force-pushed the ignite-18479 branch 2 times, most recently from b10ff68 to d7157e1 Compare February 22, 2023 10:06
Comment on lines +49 to +50
/** Reverse-ordered rows in case of limited sort. */
private List<RowT> reversed;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/** Reverse-ordered rows in case of limited sort. */
private List<RowT> reversed;
/** Reverse-ordered rows in case of limited sort. */
private List<RowT> stack;

Also, I'd use fastutil Stack interface here, or even add Stack backed by ArrayList to core module.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think we can reuse the same array for both structures: PriorityQueue and Stack.

E.g. using ObjectHeapPriorityQueue and write trivial Stack implementation.
Actually, PriorityQueue stores data in heap structure that is partially sorted.
It looks easy to implement heap-sort algorithm to get the array sorted instantly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think most optimizations and refactoring should be done separately. Here just port from AI2 with fixes real bugs

# Conflicts:
#	modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/agg/Accumulators.java
@AMashenkov AMashenkov merged commit f18c63e into apache:main Feb 23, 2023
@AMashenkov AMashenkov deleted the ignite-18479 branch February 23, 2023 12:13
lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Mar 18, 2023
lowka pushed a commit to gridgain/apache-ignite-3 that referenced this pull request Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants