Fix sorting with multiple SortByClauses #15
Merged
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
eemhu
approved these changes
Jun 7, 2024
Added more tests and all datatypes are now utilized, including the AUTOMATIC type in SortByClause. Forgot to mention earlier, all tests in PTH-10 are passing when using this branch locally! There is just a small need for fixes in PTH-10 because I refactored BatchCollect and SortByClause to not accept null anymore and there was a few calls that passed null to constructors in PTH-10. Otherwise all good. |
eemhu
approved these changes
Jun 7, 2024
StrongestNumber9
requested changes
Jun 10, 2024
StrongestNumber9
requested changes
Jun 10, 2024
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.
Related to PTH-10 issue #256.
The SortOperation takes a List of SortByClauses as parameter to sort with. However, the List was used in a for loop in a way that it always overrode the previous sorting that had happened, ultimately only applying sorting to the dataset with the last SortByClause of the List. Not likely that this was intentional.
I changed it so that it now executes just one
orderBy()call for all of the SortByClauses.So it now sorts first with the column that is first on the list, then inside that sorting it sorts again with the second column and so on. This means it preserves the sorting of the column that was in the list before.