mihaibudiu commented on code in PR #4722:
URL: https://github.com/apache/calcite/pull/4722#discussion_r2666378989
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -864,6 +864,31 @@ private void distinctify(
throw new IllegalArgumentException("rel must not be null");
}
final RelNode rel = bb.root;
+ int groupCount = rel.getRowType().getFieldCount();
+ if (bb.scope != null && bb.scope.getNode() instanceof SqlSelect) {
+ groupCount =
validator().getValidatedNodeType(bb.scope.getNode()).getFieldCount();
+ }
+ distinctify(bb, checkForDupExprs, groupCount);
+ }
+
+ /**
+ * Having translated 'SELECT ... FROM ... [GROUP BY ...] [HAVING ...]', adds
+ * a relational expression to make the results unique.
Review Comment:
is the relational expression an Aggregate? Then say so explicitly.
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -921,12 +959,78 @@ private void distinctify(
}
// Usual case: all expressions in the SELECT clause are different.
- final ImmutableBitSet groupSet =
- ImmutableBitSet.range(rel.getRowType().getFieldCount());
+ final int totalFieldCount = rel.getRowType().getFieldCount();
+ final ImmutableBitSet.Builder groupSetBuilder = ImmutableBitSet.builder();
+ // The first groupCount columns are the columns in the user's SELECT list
+ // and must be part of the group set.
+ for (int i = 0; i < groupCount; i++) {
+ groupSetBuilder.set(i);
+ }
+ // For ORDER BY columns, only deterministic expressions can be in the
group set.
+ // Non-deterministic functions like RAND() would make every row unique,
+ // breaking DISTINCT semantics (e.g., SELECT DISTINCT deptno ... ORDER BY
RAND()).
+ for (int i = groupCount; i < totalFieldCount; i++) {
+ RexNode expr = (rel instanceof Project)
Review Comment:
can you pull this instanceof out of the loop and use one variable to store
it?
you make this check many times later too
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -864,6 +864,31 @@ private void distinctify(
throw new IllegalArgumentException("rel must not be null");
}
final RelNode rel = bb.root;
+ int groupCount = rel.getRowType().getFieldCount();
+ if (bb.scope != null && bb.scope.getNode() instanceof SqlSelect) {
+ groupCount =
validator().getValidatedNodeType(bb.scope.getNode()).getFieldCount();
+ }
+ distinctify(bb, checkForDupExprs, groupCount);
+ }
+
+ /**
+ * Having translated 'SELECT ... FROM ... [GROUP BY ...] [HAVING ...]', adds
+ * a relational expression to make the results unique.
Review Comment:
maybe having here the example from the test would make it easier to
understand what is going on
--
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]
