Provide query library functions for computing differences and sort order #5151
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
chipkent
added
query engine
DocumentationNeeded
ReleaseNotesNeeded
kosak
reviewed
Feb 14, 2024
| } | ||
|
|
||
| return IntStream.range(0, values.intSize("sortIndex")) | ||
| .boxed().sorted((i, j) -> ${pt.boxed}.compare(values.get(i),values.get(j)) ) |
There was a problem hiding this comment.
Two issues:
- add one space, delete one:
compare(values.get(i), values.get(j))) - Here and everywhere in this file I have a concern.
the concern is that when applied to Float (or Double) this template will use Float.compare (or Double.compare) and in particular will not use a NaN-aware comparison.
this would make the sort inconsistent with the one used for object arrays, which does use the NullNanAwareComparator
The same NaN issue comes up when we use Arrays.sort() in this file on float[] and double[]
so I guess the first question is: do we care about NaN
There was a problem hiding this comment.
Resolved, but it came with boxing and copies.
rbasralian
reviewed
Feb 14, 2024
…phaven-core into 5101_3413_diff_rank
rbasralian
previously approved these changes
Mar 13, 2024
kosak
reviewed
Mar 14, 2024
kosak
approved these changes
Mar 19, 2024
rbasralian
approved these changes
Mar 19, 2024
|
Labels indicate documentation is required. Issues for documentation have been opened: Community: deephaven/deephaven-docs-community#173 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Query library functions for:
Resolves #5101
Resolves #3413