refactor!: move ordering-related code to order_by_util.rs && fix an issue with decimals
#388
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
iajoiner
force-pushed
the
refactor/order-by-utils
branch
from
02b9700 to
454f77e
Compare
iajoiner
force-pushed
the
refactor/order-by-utils
branch
from
454f77e to
69c17eb
Compare
iajoiner
requested review from
JayWhite2357
and removed request for
JayWhite2357
iajoiner
force-pushed
the
refactor/order-by-utils
branch
from
69c17eb to
d7caa22
Compare
JayWhite2357
previously approved these changes
Nov 24, 2024
iajoiner
force-pushed
the
refactor/order-by-utils
branch
from
d7caa22 to
7b3443e
Compare
…irection` to fix it for decimals and refactor the code
iajoiner
force-pushed
the
refactor/order-by-utils
branch
from
7b3443e to
3c96dd1
Compare
JayWhite2357
approved these changes
Nov 24, 2024
|
🎉 This PR is included in version 0.50.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Please be sure to look over the pull request guidelines here: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#submit-pr.
Please go through the following checklist
!is used if and only if at least one breaking change has been introduced.source scripts/run_ci_checks.sh.Rationale for this change
OrderByExec(to be added),GroupByExecandSortMergeJoinExec(to be added) all depend on correct sorting of rows. Hence we move them together. Meanwhile a bug was found incompare_indexes_by_owned_columns_with_directioncausing negative decimals to be considered larger than positive ones. Hence we fixed it and modified a test to check for that.What changes are included in this PR?
group_by_util.rsandowned_column.rstoorder_by_util.rsAre these changes tested?
Yes.