feat(array): Add Presto function array_top_n #12105
Open
+596
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D68031372 |
✅ Deploy Preview for meta-velox canceled.
|
| }); | ||
|
|
||
| auto result = evaluate<ArrayVector>( | ||
| "if(c0 % 2 = 0, array_top_n(c1, 2), array_top_n(c2, 2))", |
There was a problem hiding this comment.
can the second param be a column? if yes, can you add a test for that ?
There was a problem hiding this comment.
What's the best way to confirm that?
|
|
||
| // Heap sort k values. | ||
| std::partial_sort( | ||
| heap.begin(), heap.begin() + n, heap.end(), std::greater<>{}); |
There was a problem hiding this comment.
we would need special handling for floating points (use a different comparator), see #9772 for reference
There was a problem hiding this comment.
Looks like tests pass for floating points, can you point to a test case I can adopt to confirm?
peterenescu
force-pushed
the
export-D68031372
branch
from
d25d98d to
80e2065
Compare
peterenescu
added a commit
to peterenescu/velox
that referenced
this pull request
Jan 22, 2025
Summary: Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function). Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp Differential Revision: D68031372
|
This pull request was exported from Phabricator. Differential Revision: D68031372 |
peterenescu
added a commit
to peterenescu/velox
that referenced
this pull request
Jan 24, 2025
Summary: Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function). Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp Differential Revision: D68031372
peterenescu
added a commit
to peterenescu/velox
that referenced
this pull request
Jan 28, 2025
Summary: Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function). Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp Differential Revision: D68031372
peterenescu
force-pushed
the
export-D68031372
branch
from
80e2065 to
c84b345
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68031372 |
peterenescu
added a commit
to peterenescu/velox
that referenced
this pull request
Jan 28, 2025
Summary: Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function). Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp Differential Revision: D68031372
peterenescu
force-pushed
the
export-D68031372
branch
from
c84b345 to
89f4853
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68031372 |
peterenescu
added a commit
to peterenescu/velox
that referenced
this pull request
Jan 29, 2025
Summary: Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function). Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp Differential Revision: D68031372
peterenescu
force-pushed
the
export-D68031372
branch
from
89f4853 to
3c0a4e3
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68031372 |
peterenescu
added a commit
to peterenescu/velox
that referenced
this pull request
Jan 29, 2025
Summary: Pull Request resolved: facebookincubator#12105 Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function). Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp Differential Revision: D68031372
| int numNull = 0; | ||
| for (const auto& item : array) { | ||
| if (item.has_value()) { | ||
| minHeap.push(item.value()); |
There was a problem hiding this comment.
can you do a check on the value against the top() in the minHeap before pushing it on the heap (both here and in the generic implementation). In the current implementation if you always add it then it would end up doing many more comparisons and switches to balance the heap which can be wasteful
Summary: Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function). Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp Differential Revision: D68031372
peterenescu
force-pushed
the
export-D68031372
branch
from
3c0a4e3 to
5930e97
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68031372 |
peterenescu
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Adds Presto function array_top_n as a simple function in Velox. Function uses a temporary vector to store inputted values and heap sorts them up to k values (second input to function).
Updates ArrayFunction.h with struct ArrayTopNFunction and adds new tester function ArrayTopNTest.cpp
Differential Revision: D68031372