kalayciburak commented on code in PR #5293:
URL: https://github.com/apache/calcite/pull/5293#discussion_r4163103727


##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -5180,6 +5180,56 @@ private void startOfGroupStep3(String startOfGroup) {
             "EXPR$0=4");
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7822";>[CALCITE-7822]
+   * AssertionError "type mismatch" in SqlToRelConverter for OVER () in a
+   * GROUP BY query</a>. */
+  @Test void testEmptyOverInAggregateQuery() {

Review Comment:
   yep, they do. moved them to winagg.iq and dropped the java assertions.



##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -2674,6 +2678,79 @@ private RexNode convertOver(Blackboard bb, SqlNode node) 
{
     }
   }
 
+  /**
+   * Adds implicit RANGE order keys that still refer to the current input.
+   * A monotonic column of the FROM item is not an order key once GROUP BY
+   * has removed it; binding it by its original FROM ordinal then points at
+   * a different field (CALCITE-7822).
+   */
+  private void addImplicitOrderKeys(Blackboard bb, SqlNodeList implicit,
+      ImmutableList.Builder<RexNode> orderKeys) {
+    final RelDataType rowType = bb.root().getRowType();
+    final SqlNameMatcher nameMatcher =
+        bb.scope.getValidator().getCatalogReader().nameMatcher();
+    for (SqlNode order : implicit) {
+      final RexNode converted =
+          bb.convertSortExpression(order,
+              RelFieldCollation.Direction.ASCENDING,
+              RelFieldCollation.NullDirection.UNSPECIFIED,
+              bb::sortToRex);
+      if (orderKeyMatchesInput(rowType, converted)) {
+        orderKeys.add(converted);
+        continue;
+      }
+      final RexNode rebound = rebindImplicitOrderKey(nameMatcher, rowType, 
order);
+      if (rebound != null) {
+        orderKeys.add(rebound);
+      }
+    }
+  }
+
+  /** Whether every input ref in {@code orderKey} matches the current input. */
+  private static boolean orderKeyMatchesInput(RelDataType rowType, RexNode 
orderKey) {
+    final List<RexInputRef> refs = new ArrayList<>();
+    orderKey.accept(new RexShuttle() {
+      @Override public RexNode visitInputRef(RexInputRef inputRef) {
+        refs.add(inputRef);
+        return inputRef;
+      }
+    });
+    if (refs.isEmpty()) {
+      return false;
+    }
+    for (RexInputRef ref : refs) {
+      if (ref.getIndex() < 0 || ref.getIndex() >= rowType.getFieldCount()) {
+        return false;
+      }
+      final RelDataTypeField field = 
rowType.getFieldList().get(ref.getIndex());
+      if (!equalSansNullability(field.getType(), ref.getType())) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  /** Rebinds a simple column order key onto the current input, or null. */
+  private @Nullable RexNode rebindImplicitOrderKey(SqlNameMatcher nameMatcher,

Review Comment:
   it meant look the column up by name on the current row, if GROUP BY didn't 
remove it. renamed the helper so it says that.



##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -2674,6 +2678,79 @@ private RexNode convertOver(Blackboard bb, SqlNode node) 
{
     }
   }
 
+  /**
+   * Adds implicit RANGE order keys that still refer to the current input.
+   * A monotonic column of the FROM item is not an order key once GROUP BY
+   * has removed it; binding it by its original FROM ordinal then points at
+   * a different field (CALCITE-7822).
+   */
+  private void addImplicitOrderKeys(Blackboard bb, SqlNodeList implicit,
+      ImmutableList.Builder<RexNode> orderKeys) {
+    final RelDataType rowType = bb.root().getRowType();
+    final SqlNameMatcher nameMatcher =
+        bb.scope.getValidator().getCatalogReader().nameMatcher();
+    for (SqlNode order : implicit) {
+      final RexNode converted =
+          bb.convertSortExpression(order,
+              RelFieldCollation.Direction.ASCENDING,
+              RelFieldCollation.NullDirection.UNSPECIFIED,
+              bb::sortToRex);
+      if (orderKeyMatchesInput(rowType, converted)) {
+        orderKeys.add(converted);
+        continue;
+      }
+      final RexNode rebound = rebindImplicitOrderKey(nameMatcher, rowType, 
order);
+      if (rebound != null) {
+        orderKeys.add(rebound);
+      }
+    }
+  }
+
+  /** Whether every input ref in {@code orderKey} matches the current input. */

Review Comment:
   the ref index is in range, and its type equals the field at that index 
(nullability aside). if the types differ, the ordinal now names a different 
column.



-- 
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]

Reply via email to