xuzifu666 commented on code in PR #4933:
URL: https://github.com/apache/calcite/pull/4933#discussion_r3437424722
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -720,6 +720,9 @@ private static RelCollation requiredCollation(RelNode r) {
if (r instanceof Project) {
return requiredCollation(((Project) r).getInput());
}
+ if (r instanceof Filter) {
Review Comment:
It is necessary, without this change would error:
```
java.lang.AssertionError
at org.apache.calcite.sql2rel.SqlToRelConverter
.requiredCollation(SqlToRelConverter.java:729)
at org.apache.calcite.sql2rel.SqlToRelConverter
.requiredCollation(SqlToRelConverter.java:721)
at org.apache.calcite.sql2rel.SqlToRelConverter
.convertOrder(SqlToRelConverter.java:...)
at org.apache.calcite.sql2rel.SqlToRelConverter
.convertSelectImpl(SqlToRelConverter.java:...)
```
Any SQL statement like this will fail:
```
SELECT DISTINCT ON (deptno) empno, ename
FROM emp
WHERE deptno > 10 ← The WHERE clause produces a Filter
ORDER BY deptno, empno
```
Failure workflow
1. `convertWhere` adds a Filter node.
2. `convertOrder` calls `requiredCollation(result)`.
3. `result` is a Project → enters the Project branch, recurses downwards.
4. `project.getInput()` is a Filter → no Filter check!
5. Skips the Delta check.
6. Reaches the final `throw new AssertionError()`.
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6627,7 +6507,122 @@ void testReturnsCorrectRowTypeOnCombinedJoin() {
f.withSql(qualifyOnMultipleWindowFunctions).ok();
}
- /** Negative tests for the {@code QUALIFY} clause. */
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-5406";>[CALCITE-5406]
+ * Support the SELECT DISTINCT ON statement for PostgreSQL dialect</a>. */
+ @Test void testDistinctOnNotAllowed() {
+ // DISTINCT ON is not allowed under default SQL conformance
+ sql("^SELECT DISTINCT ON (deptno) empno FROM emp^ ORDER BY deptno")
+ .fails("SELECT DISTINCT ON is not supported under the current SQL
conformance level");
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-5406";>[CALCITE-5406]
+ * Support the SELECT DISTINCT ON statement for PostgreSQL dialect</a>. */
+ @Test void testDistinctOnPositive() {
+ final SqlValidatorFixture f =
+ fixture().withConformance(SqlConformanceEnum.LENIENT);
+
+ f.withSql("SELECT DISTINCT ON (deptno) empno, ename FROM emp ORDER BY
deptno, empno")
+ .ok();
+
+ f.withSql("SELECT DISTINCT ON (deptno, job) empno FROM emp ORDER BY
deptno, job, hiredate")
+ .ok();
+
+ f.withSql("SELECT DISTINCT ON (deptno) empno FROM emp ORDER BY deptno
DESC")
+ .ok();
+
+ f.withSql("SELECT DISTINCT ON (deptno) empno FROM emp ORDER BY deptno
NULLS FIRST")
+ .ok();
+ }
+
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-5406";>[CALCITE-5406]
+ * Support the SELECT DISTINCT ON statement for PostgreSQL dialect</a>. */
+ @Test void testDistinctOnNegative() {
+ final SqlValidatorFixture f =
+ fixture().withConformance(SqlConformanceEnum.LENIENT);
+
+ // DISTINCT ON requires ORDER BY
+ f.withSql("^SELECT DISTINCT ON (deptno) empno FROM emp^")
+ .fails("SELECT DISTINCT ON requires an ORDER BY clause");
+
+ // ORDER BY must contain all DISTINCT ON expressions as prefix
+ f.withSql("SELECT DISTINCT ON (deptno, job) empno FROM emp ORDER BY
^deptno^")
+ .fails("SELECT DISTINCT ON expressions must match initial ORDER BY
expressions");
+
+ // ORDER BY prefix must match exactly
+ f.withSql("SELECT DISTINCT ON (deptno, job) empno FROM emp ORDER BY ^job^,
deptno")
+ .fails("SELECT DISTINCT ON expressions must match initial ORDER BY
expressions");
+
+ // DISTINCT ON with extra ORDER BY is ok
+ f.withSql("SELECT DISTINCT ON (deptno) empno FROM emp ORDER BY deptno,
job")
+ .ok();
+
+ // Duplicate expressions in DISTINCT ON are allowed but must be matched in
ORDER BY
+ f.withSql("SELECT DISTINCT ON (deptno, deptno) deptno, empno FROM emp
ORDER BY deptno, deptno")
+ .ok();
+
+ // DISTINCT ON with expression
+ f.withSql("SELECT DISTINCT ON (empno % 2) empno, ename FROM emp ORDER BY
empno % 2")
+ .ok();
+
+ // Empty DISTINCT ON is not allowed (parse error)
+ f.withSql("SELECT DISTINCT ON ((^)^) deptno FROM emp")
+ .fails("(?s)Encountered \"\\)\" at .*");
+
+ // DISTINCT ON can reference alias (like ORDER BY)
+ f.withSql("SELECT DISTINCT ON (x) empno AS x, deptno FROM emp ORDER BY x")
+ .ok();
+
+ // DISTINCT ON with alias-column name clash: alias in SELECT takes
precedence
+ f.withSql("SELECT DISTINCT ON (deptno) empno AS deptno, deptno AS d FROM
emp ORDER BY deptno")
+ .ok();
+
+ // DISTINCT ON with qualified column reference
+ f.withSql("SELECT DISTINCT ON (e.deptno) e.deptno AS x, e.empno "
Review Comment:
It work well, I also added related test in last commit.
--
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]
