opt: fix some aggregate scoping issues #46227
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
|
This change is |
|
gentle ping |
Prior to this commit, some aggregate functions were either incorrectly rejected or incorrectly accepted when they were scoped at a higher level than their position in the query. For example, aggregate functions are not normally allowed in WHERE, but if the aggregate is actually scoped at a higher level, then the aggregate should be allowed. Prior to this commit, these aggregate functions were rejected and caused an error. This commit fixes the issue by validating the context of the aggregate's scope rather than the aggregate's position in the query. In order to avoid adding another field to the scope struct, this commit re-uses the existing `context` field which was previously only used for error messages. To make comparisons more efficient, the field is now an enum rather than a string. Fixes cockroachdb#44724 Fixes cockroachdb#45838 Fixes cockroachdb#30652 Release justification: This bug fix is a low risk, high benefit change to existing functionality, since it fixes internal errors and increases compatibility with Postgres. Release note (bug fix): Fixed an internal error that could occur when an aggregate inside the right-hand side of a LATERAL join was scoped at the level of the left-hand side. Release note (bug fix): Fixed an error that incorrectly occurred when an aggregate was used inside the WHERE or ON clause of a subquery but was scoped at an outer level of the query.
RaduBerinde
reviewed
Mar 20, 2020
There was a problem hiding this comment.
Oops, sorry, this fell through the cracks. 
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @andy-kimball and @RaduBerinde)
Build succeeded |
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.
Prior to this commit, some aggregate functions were either incorrectly
rejected or incorrectly accepted when they were scoped at a higher
level than their position in the query. For example, aggregate functions
are not normally allowed in
WHERE, but if the aggregate is actually scopedat a higher level, then the aggregate should be allowed. Prior to this
commit, these aggregate functions were rejected and caused an error.
This commit fixes the issue by validating the context of the aggregate's
scope rather than the aggregate's position in the query. In order to
avoid adding another field to the scope struct, this commit re-uses
the existing
contextfield which was previously only used for errormessages. To make comparisons more efficient, the field is now an enum
rather than a string.
Fixes #44724
Fixes #45838
Fixes #30652
Release justification: This bug fix is a low risk, high benefit change
to existing functionality, since it fixes internal errors and increases
compatibility with Postgres.
Release note (bug fix): Fixed an internal error that could occur when
an aggregate inside the right-hand side of a LATERAL join was scoped at
the level of the left-hand side.
Release note (bug fix): Fixed an error that incorrectly occurred when
an aggregate was used inside the WHERE or ON clause of a subquery but
was scoped at an outer level of the query.