FrankChen021 commented on code in PR #19976:
URL: https://github.com/apache/druid/pull/19976#discussion_r3968849649
##########
sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java:
##########
@@ -475,6 +475,14 @@ public static void validateClusteredByColumns(final
SqlNodeList clusteredByNodes
);
}
+ // Aliases like "CLUSTERED BY col AS alias" are not allowed.
+ if (clusteredByNode.isA(ImmutableSet.of(SqlKind.AS))) {
Review Comment:
[P2] Reject aliases hidden by NULLS FIRST
`AddOrderItem` now permits an alias before the NULLS modifier, so `CLUSTERED
BY col AS alias NULLS FIRST` is represented as `NULLS_FIRST(AS(col, alias))`.
This check only recognizes a top-level `AS`; Druid's validator allows
`NULLS_FIRST` around a non-DESC operand, so the unsupported alias passes
validation and reaches the sort/explain path. The downstream clustered-by
resolver only extracts an alias when `AS` is the outer call, so this form is
not resolved to the output column name. Unwrap the order wrappers before
checking for `AS` (or reject aliases anywhere in the item) and add a regression
test for the wrapped form.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
