kalayciburak commented on code in PR #5293:
URL: https://github.com/apache/calcite/pull/5293#discussion_r4170268887
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -2623,26 +2623,30 @@ private RexNode convertOver(Blackboard bb, SqlNode
node) {
bb.convertExpression(requireNonNull(sqlLowerBound, "sqlLowerBound"));
final RexNode upperBound =
bb.convertExpression(requireNonNull(sqlUpperBound, "sqlUpperBound"));
- if (orderList.isEmpty() && !rows) {
- // A logical range requires an ORDER BY clause. Use the implicit
- // ordering of this relation. There must be one, otherwise it would
- // have failed validation.
- orderList = bb.scope.getOrderList();
- if (orderList == null) {
- throw new AssertionError(
- "Relation should have sort key for implicit ORDER BY");
- }
- }
final RexWindowExclusion exclude =
RexWindowExclusion.create(window.getExclude());
final ImmutableList.Builder<RexNode> orderKeys =
ImmutableList.builder();
- for (SqlNode order : orderList) {
- orderKeys.add(
- bb.convertSortExpression(order,
- RelFieldCollation.Direction.ASCENDING,
- RelFieldCollation.NullDirection.UNSPECIFIED,
- bb::sortToRex));
+ if (orderList.isEmpty() && !rows) {
+ // RANGE with no ORDER BY still needs a logical order. That order is
+ // the first monotonic column of the FROM item (a column the table
+ // declares as sorted, for example EMPNO). After GROUP BY that column
+ // may be gone. Its old ordinal then names a different field, and
+ // Project.isValid fails (CALCITE-7822).
+ final SqlNodeList implicit = bb.scope.getOrderList();
+ if (implicit == null) {
Review Comment:
yeah, this shouldn't be about preserving the implicit sort. dropped the name
lookup. if the refs don't match the current input the key is left off, so over
() works with or without a monotonic column. added the no-monotonic case to
winagg.iq.
also cleaned the comments, no test column name and no issue id in the
compiler notes.
--
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]
