You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "xiedeyantu (via GitHub)" <gi...@apache.org> on 2026/01/05 10:50:27 UTC
[PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
xiedeyantu opened a new pull request, #4722:
URL: https://github.com/apache/calcite/pull/4722
See [CALCITE-5597](https://issues.apache.org/jira/browse/CALCITE-5597)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4722:
URL: https://github.com/apache/calcite/pull/4722#issuecomment-3721280917
All the above comments have been resolved.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4722:
URL: https://github.com/apache/calcite/pull/4722#issuecomment-3714263077
> select distinct deptno from emp order by random();
Yes, pgsql can not work. duckdb can support this syntax.
```
duckdb> select distinct deptno from emp order by random();
┌────────┐
│ DEPTNO │
╞════════╡
│ 20 │
│ 10 │
│ 30 │
└────────┘
```
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4722:
URL: https://github.com/apache/calcite/pull/4722#issuecomment-3718915741
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4722) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4722&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4722&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4722&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4722&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4722&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4722)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4722:
URL: https://github.com/apache/calcite/pull/4722#issuecomment-3710001691
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4722) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4722&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4722&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4722&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4722&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4722&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4722)
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on PR #4722:
URL: https://github.com/apache/calcite/pull/4722#issuecomment-3728700809
#4733 The implementation has been refactored; this PR is now closed.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu closed pull request #4722: [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result
URL: https://github.com/apache/calcite/pull/4722
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5597] SELECT DISTINCT query with ORDER BY column will get error result error result [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4722:
URL: https://github.com/apache/calcite/pull/4722#discussion_r2673346461
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -879,10 +916,23 @@ private void distinctify(
}
}
if (dupCount == 0) {
- distinctify(bb, false);
+ distinctify(bb, false, groupCount);
return;
}
+ // Calculate the number of unique grouping columns in the SELECT list.
Review Comment:
Can't this loop be combined with the one above?
It may be even easier to read.
##########
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##########
@@ -5986,4 +5986,12 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) {
+ "and t1.ename in (select t3.ename from emp t3 )";
sql(sql).ok();
}
+
+ /** Test case of
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5597">[CALCITE-5597]
+ * SELECT DISTINCT query with ORDER BY column will get error result</a>. */
+ @Test void testDistinctOrderByRand() {
Review Comment:
Please add a test where some columns have constant values; Calcite represents constants in the select list in a special way, and it may interfere with the index computations you have.
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -921,12 +971,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 Project project = rel instanceof Project ? (Project) rel : null;
+ 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 = project != null
+ ? project.getProjects().get(i) : null;
+ if (expr == null || RexUtil.isDeterministic(expr)) {
+ groupSetBuilder.set(i);
+ }
+ }
+ final ImmutableBitSet groupSet = groupSetBuilder.build();
+ ImmutableBitSet aggregateGroupSet = groupSet;
+ if (groupSet.cardinality() < totalFieldCount && project != null) {
+ // Trim the input project to remove unused non-deterministic expressions.
+ // This avoids evaluating them twice (once below the aggregate and once above).
+ final List<RexNode> newBottomProjects = new ArrayList<>();
+ final List<String> newBottomNames = new ArrayList<>();
+ for (int i = 0; i < totalFieldCount; i++) {
+ if (groupSet.get(i)) {
+ newBottomProjects.add(project.getProjects().get(i));
+ newBottomNames.add(rel.getRowType().getFieldNames().get(i));
+ }
+ }
+ RelNode inputRel =
+ LogicalProject.create(project.getInput(), project.getHints(),
+ newBottomProjects, newBottomNames, project.getVariablesSet());
+ bb.setRoot(inputRel, false);
+ aggregateGroupSet = ImmutableBitSet.range(newBottomProjects.size());
+ }
+
bb.setRoot(
- createAggregate(bb, groupSet, ImmutableList.of(groupSet),
+ createAggregate(bb, aggregateGroupSet, ImmutableList.of(aggregateGroupSet),
ImmutableList.of()),
false);
+
+ if (groupSet.cardinality() < totalFieldCount) {
+ final RelNode aggregate = bb.root();
+ final List<RexNode> newProjects = new ArrayList<>();
+ final List<String> newNames = rel.getRowType().getFieldNames();
+ final RexShuttle shuttle = new RexShuttle() {
+ @Override public RexNode visitInputRef(RexInputRef inputRef) {
+ int newIdx = groupSet.indexOf(inputRef.getIndex());
+ if (newIdx >= 0) {
+ return rexBuilder.makeInputRef(aggregate, newIdx);
+ }
+ return super.visitInputRef(inputRef);
+ }
+ };
+ for (int i = 0; i < totalFieldCount; i++) {
+ int newIdx = groupSet.indexOf(i);
+ if (newIdx >= 0) {
+ newProjects.add(rexBuilder.makeInputRef(aggregate, newIdx));
+ } else {
+ // If we are here, it means this column was excluded from the groupSet
+ // because it was non-deterministic. Such columns must come from a Project.
Review Comment:
I wonder whether it wouldn't be clearer to just create an HashMap with these columns and fill it in the first loop above.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org