[CALCITE-4491] Aggregations of nested window functions produce invalid SQL #2346
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
|
Sorry, working on rebase on top of master now. |
Dialects which do not support nested aggregations now also don't support nested window functions. The added test covers both cases: - a dialect which supports nested aggregations -> OracleSqlDialect - a dialect which does not support nested aggregations -> PostgreSqlDialect
Dominick1993
force-pushed
the
CALCITE-4491
branch
from
d45691e
to
6c5156d
Compare
Dominick1993
changed the title
Dominick1993
commented
Feb 10, 2021
| @@ -1803,9 +1805,14 @@ private boolean needNewSubQuery( | |||
|
|
|||
| if (rel instanceof Aggregate) { | |||
| final Aggregate agg = (Aggregate) rel; | |||
| final boolean hasNestedAgg = hasNestedAggregations(agg); | |||
| final boolean hasNestedAgg = hasNested(agg, operand -> | |||
| operand instanceof SqlCall | |||
There was a problem hiding this comment.
@julianhyde I wasn't sure whether this check should be a part of hasNested method or a part of the predicate. I've chosen to go with the more generic predicate approach here, but please let me know your preference.
Dominick1993
changed the title
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Feb 10, 2021
…r PostgreSQL (Dominik Labuda) Close apache#2346
julianhyde
pushed a commit
to julianhyde/calcite
that referenced
this pull request
Feb 10, 2021
…r PostgreSQL (Dominik Labuda) Dialects that do not support nested aggregations now also don't support nested window functions. (If there are dialects that support one but not the other, we can revisit.) The added test covers both cases: - a dialect which supports nested aggregations -> OracleSqlDialect - a dialect which does not support nested aggregations -> PostgreSqlDialect Close apache#2346
MalteBellmann
pushed a commit
to caespes/calcite
that referenced
this pull request
Feb 21, 2021
…r PostgreSQL (Dominik Labuda) Dialects that do not support nested aggregations now also don't support nested window functions. (If there are dialects that support one but not the other, we can revisit.) The added test covers both cases: - a dialect which supports nested aggregations -> OracleSqlDialect - a dialect which does not support nested aggregations -> PostgreSqlDialect Close apache#2346
XuQianJin-Stars
pushed a commit
to XuQianJin-Stars/calcite
that referenced
this pull request
Jul 14, 2021
…r PostgreSQL (Dominik Labuda) Dialects that do not support nested aggregations now also don't support nested window functions. (If there are dialects that support one but not the other, we can revisit.) The added test covers both cases: - a dialect which supports nested aggregations -> OracleSqlDialect - a dialect which does not support nested aggregations -> PostgreSqlDialect Close apache#2346
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.
Dialects which do not support nested aggregations now also don't support
nested window functions.
The added test covers both cases: