mihaibudiu commented on code in PR #4671:
URL: https://github.com/apache/calcite/pull/4671#discussion_r2609108423
##########
core/src/main/java/org/apache/calcite/sql/SqlUtil.java:
##########
@@ -1294,6 +1294,46 @@ public static boolean containsCall(SqlNode node,
}
}
+ /**
+ * Strips sort modifiers (DESC, NULLS FIRST/LAST) from a node.
Review Comment:
This does not modify the node, as you would think from the JavaDoc; it
returns another node without these modifiers.
Maybe call the method withoutOrderModifiers?
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -2969,26 +2971,62 @@ SqlNodeList OrderBy(boolean accept) :
throw SqlUtil.newContextException(s.pos(),
RESOURCE.illegalOrderBy());
}
}
- <BY> AddOrderItem(list)
- (
- // NOTE jvs 6-Feb-2004: See comments at top of file for why
- // hint is necessary here.
- LOOKAHEAD(2) <COMMA> AddOrderItem(list)
- )*
+ <BY> OrderItemList(list)
+ {
+ return new SqlNodeList(list, s.addAll(list).pos());
+ }
+}
+
+/**
+ * Parses a BY clause for SELECT (syntactic sugar for GROUP BY ... ORDER BY).
+ */
+SqlNodeList SqlSelectBy() :
Review Comment:
This is non-standard, so it may cause problems by accepting programs that
others think are illegal.
So the question is whether it should be behind a configuration flag, like
the * EXCEPT () feature you have introduced recently.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3035,19 +3093,20 @@ private void registerQuery(
clauseScopes.put(IdPair.of(select, Clause.SELECT), selectScope2);
clauseScopes.put(IdPair.of(select, Clause.MEASURE),
new MeasureScope(selectScope, select));
- if (select.getGroup() != null) {
+ SqlNodeList groupList = first(select.getBy(), select.getGroup());
Review Comment:
If this was done after rewriteSelectBy you would not need these changes
anymore.
I don't know if it's possible.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -550,11 +558,15 @@ private boolean isNonAggregatedNonGroupedColumn(SqlNode
node, SqlSelect select)
}
return groupList.getList().stream()
.noneMatch(groupItem -> groupItem != null
- && node.equalsDeep(groupItem, Litmus.IGNORE));
+ && node.equalsDeep(stripAs(groupItem), Litmus.IGNORE));
Review Comment:
doesn't this change the behavior for existing programs?
##########
core/src/main/java/org/apache/calcite/sql/SqlUtil.java:
##########
@@ -1294,6 +1294,46 @@ public static boolean containsCall(SqlNode node,
}
}
+ /**
+ * Strips sort modifiers (DESC, NULLS FIRST/LAST) from a node.
+ */
+ public static SqlNode stripOrderModifiers(SqlNode node) {
+ SqlNode expr = node;
+ while (expr instanceof SqlCall) {
+ final SqlCall call = (SqlCall) expr;
+ final SqlKind kind = call.getKind();
+ if (kind == SqlKind.DESCENDING
+ || kind == SqlKind.NULLS_FIRST
+ || kind == SqlKind.NULLS_LAST) {
+ expr = call.operand(0);
+ } else {
+ break;
+ }
+ }
+ return expr;
+ }
+
+ /**
+ * Strips AS from an ORDER BY item, but keeps sort modifiers.
+ */
+ public static SqlNode stripAsFromOrder(SqlNode node) {
+ if (node instanceof SqlCall) {
Review Comment:
Similar comments apply here
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -1538,6 +1550,52 @@ private void handleOffsetFetch(@Nullable SqlNode offset,
@Nullable SqlNode fetch
}
}
+ /**
+ * Rewrites SELECT ... BY syntax into SELECT, GROUP BY, ORDER BY.
+ */
+ private void rewriteSelectBy(SqlSelect select) {
+ final SqlNodeList by = select.getBy();
+ if (by == null) {
+ return;
+ }
+ if (select.getGroup() != null && !select.getGroup().isEmpty()) {
+ throw newValidationError(select.getGroup(),
RESOURCE.selectByAndGroupBy());
+ }
+ if (select.getOrderList() != null && !select.getOrderList().isEmpty()) {
+ throw newValidationError(select.getOrderList(),
RESOURCE.selectByAndOrderBy());
+ }
+ // Mark this select as having a BY clause, so that we can enable
+ // non-strict GROUP BY behavior (wrapping non-grouped columns in
ANY_VALUE).
+ selectsWithBy.add(select);
+
+ final SqlNodeList selectList = select.getSelectList();
+ final SqlNodeList groupBy = new SqlNodeList(by.getParserPosition());
+ final SqlNodeList orderBy = new SqlNodeList(by.getParserPosition());
+
+ // Process BY items
+ List<SqlNode> extraSelectItems = new ArrayList<>();
Review Comment:
My question was: why can't this be done directly during parsing?
--
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]
