fix: fix regressions in tests #38
Closed
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
wiedld
reviewed
Sep 9, 2024
Comment on lines
+267
to
+272
| 12 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) | ||
| 2 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) | ||
| 0 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) | ||
| 14 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) | ||
| 0 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) | ||
| 0 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) | ||
| 5 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) | ||
| 1 2014-08-27T14:00:00Z Timestamp(Millisecond, Some("UTC")) |
There was a problem hiding this comment.
So it's not just the ordering. The values in the first count column is incorrect.
itsjunetime
pushed a commit
that referenced
this pull request
Oct 22, 2024
…regation (apache#12946) * Improve unparsing for ORDER BY with Aggregation functions (#38) * Improve UNION unparsing (#39) * Scalar functions in ORDER BY unparsing support (#41) * Improve unparsing for complex Window functions with Aggregation (#42) * WindowFunction order_by should respect `supports_nulls_first_in_sort` dialect setting (#43) * Fix plan_to_sql * Improve
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.
Specifically, the regressions caused by 02eab80
This should let us get by this blocker and continue DF upgrades.
I'm just putting it here for now so I don't forget abt it and others can look at it. It needs more comments and more descriptive commit names.
Also, it still seems to affect ordering of results that are equivalent - so our tests don't pass without changes after this, but I think they'll be correct after this PR.
This also might prevent some of the optimizations that the linked commit was trying to get? I'm not certain.