xuzifu666 commented on code in PR #4692:
URL: https://github.com/apache/calcite/pull/4692#discussion_r2629235560


##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11462,4 +11463,196 @@ public Sql schema(CalciteAssert.SchemaSpec 
schemaSpec) {
           relFn, transforms);
     }
   }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7279";>[CALCITE-7279]
+   * ClickHouse dialect should wrap nested JOINs</a>.
+   *
+   * <p>ClickHouse does not support nested JOIN syntax directly.
+   * When a JOIN's right side is another JOIN, it must be wrapped
+   * in a SELECT * FROM (...) subquery. */
+  @Test void testClickHouseNestedJoin() {
+    final String query = "SELECT e.empno, j.dname, j.loc\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname, d2.loc\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + ") AS j ON e.deptno = j.deptno";
+
+    final String sql = sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .exec();
+
+    assertThat(sql, containsString("LEFT JOIN (SELECT *"));
+    assertThat(sql, containsString("FROM (SELECT `DEPT`.`DEPTNO`, 
`DEPT`.`DNAME`, `DEPT0`.`LOC`"));
+    assertThat(sql, containsString("INNER JOIN `SCOTT`.`DEPT` AS `DEPT0`"));
+    assertTrue(sql.matches("(?s).*AS `t\\d+`\\) AS `t\\d+`.*"));
+  }
+
+  /** Test that simple JOINs without nesting are not wrapped. */
+  @Test void testClickHouseSimpleJoinNotWrapped() {
+    final String query = "SELECT e.empno, d.dname\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN dept d ON e.deptno = d.deptno";
+    final String expected = "SELECT `EMP`.`EMPNO`, `DEPT`.`DNAME`\n"
+        + "FROM `SCOTT`.`EMP`\n"
+        + "LEFT JOIN `SCOTT`.`DEPT` ON `EMP`.`DEPTNO` = `DEPT`.`DEPTNO`";
+    sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .ok(expected);
+  }
+
+  /** Test INNER JOIN with nested right side. */
+  @Test void testClickHouseNestedInnerJoin() {
+    final String query = "SELECT e.empno, j.dname\n"
+        + "FROM emp e\n"
+        + "INNER JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + ") AS j ON e.deptno = j.deptno";
+
+    final String sql = sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .exec();

Review Comment:
   Also here, suggestion as @xiedeyantu 



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11462,4 +11463,196 @@ public Sql schema(CalciteAssert.SchemaSpec 
schemaSpec) {
           relFn, transforms);
     }
   }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7279";>[CALCITE-7279]
+   * ClickHouse dialect should wrap nested JOINs</a>.
+   *
+   * <p>ClickHouse does not support nested JOIN syntax directly.
+   * When a JOIN's right side is another JOIN, it must be wrapped
+   * in a SELECT * FROM (...) subquery. */
+  @Test void testClickHouseNestedJoin() {
+    final String query = "SELECT e.empno, j.dname, j.loc\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname, d2.loc\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + ") AS j ON e.deptno = j.deptno";
+
+    final String sql = sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .exec();
+
+    assertThat(sql, containsString("LEFT JOIN (SELECT *"));
+    assertThat(sql, containsString("FROM (SELECT `DEPT`.`DEPTNO`, 
`DEPT`.`DNAME`, `DEPT0`.`LOC`"));
+    assertThat(sql, containsString("INNER JOIN `SCOTT`.`DEPT` AS `DEPT0`"));
+    assertTrue(sql.matches("(?s).*AS `t\\d+`\\) AS `t\\d+`.*"));
+  }
+
+  /** Test that simple JOINs without nesting are not wrapped. */

Review Comment:
   comments maybe better that:```/** Test that simple JOINs for ClickHouse 
without nesting are not wrapped. */```



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11462,4 +11463,196 @@ public Sql schema(CalciteAssert.SchemaSpec 
schemaSpec) {
           relFn, transforms);
     }
   }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7279";>[CALCITE-7279]
+   * ClickHouse dialect should wrap nested JOINs</a>.
+   *
+   * <p>ClickHouse does not support nested JOIN syntax directly.
+   * When a JOIN's right side is another JOIN, it must be wrapped
+   * in a SELECT * FROM (...) subquery. */
+  @Test void testClickHouseNestedJoin() {
+    final String query = "SELECT e.empno, j.dname, j.loc\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname, d2.loc\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + ") AS j ON e.deptno = j.deptno";
+
+    final String sql = sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .exec();
+
+    assertThat(sql, containsString("LEFT JOIN (SELECT *"));
+    assertThat(sql, containsString("FROM (SELECT `DEPT`.`DEPTNO`, 
`DEPT`.`DNAME`, `DEPT0`.`LOC`"));
+    assertThat(sql, containsString("INNER JOIN `SCOTT`.`DEPT` AS `DEPT0`"));
+    assertTrue(sql.matches("(?s).*AS `t\\d+`\\) AS `t\\d+`.*"));
+  }
+
+  /** Test that simple JOINs without nesting are not wrapped. */
+  @Test void testClickHouseSimpleJoinNotWrapped() {
+    final String query = "SELECT e.empno, d.dname\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN dept d ON e.deptno = d.deptno";
+    final String expected = "SELECT `EMP`.`EMPNO`, `DEPT`.`DNAME`\n"
+        + "FROM `SCOTT`.`EMP`\n"
+        + "LEFT JOIN `SCOTT`.`DEPT` ON `EMP`.`DEPTNO` = `DEPT`.`DEPTNO`";
+    sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .ok(expected);
+  }
+
+  /** Test INNER JOIN with nested right side. */

Review Comment:
   Same as above



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -223,6 +224,69 @@ private static class AliasReplacementShuttle extends 
SqlShuttle {
     }
   }
 
+  /**
+   * Wraps a nested join Result into a subquery with a new alias.
+   * Required for dialects like ClickHouse that don't support nested JOIN 
syntax.
+   *
+   * @param input the Result from the nested join subtree
+   * @param outerAlias the alias to assign to the wrapped subquery
+   * @return a new Result with the wrapped SQL node
+   */
+  protected Result wrapNestedJoin(Result input, RelNode inputRel, String 
outerAlias) {
+    // Get the original SQL node from the input Result
+    final SqlNode original = input.asSelect();
+    // Generate inner alias (different from outer)
+    String innerAlias = "t" + inputRel.getId();
+    // Wrap: original → (SELECT * FROM (original) AS newAlias)
+    SqlNode wrapped = wrapAsSelectStar(original, innerAlias);
+
+    Map<String, RelDataType> newAliases = new LinkedHashMap<>();

Review Comment:
   Is there any consideration for sorting efficiency when using LinkedHashMap 
here?



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11491,4 +11463,196 @@ public Sql schema(CalciteAssert.SchemaSpec 
schemaSpec) {
           relFn, transforms);
     }
   }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7279";>[CALCITE-7279]
+   * ClickHouse dialect should wrap nested JOINs</a>.
+   *
+   * <p>ClickHouse does not support nested JOIN syntax directly.
+   * When a JOIN's right side is another JOIN, it must be wrapped
+   * in a SELECT * FROM (...) subquery. */
+  @Test void testClickHouseNestedJoin() {
+    final String query = "SELECT e.empno, j.dname, j.loc\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname, d2.loc\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + ") AS j ON e.deptno = j.deptno";
+
+    final String sql = sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .exec();
+
+    assertThat(sql, containsString("LEFT JOIN (SELECT *"));

Review Comment:
   +1



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -11462,4 +11463,196 @@ public Sql schema(CalciteAssert.SchemaSpec 
schemaSpec) {
           relFn, transforms);
     }
   }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7279";>[CALCITE-7279]
+   * ClickHouse dialect should wrap nested JOINs</a>.
+   *
+   * <p>ClickHouse does not support nested JOIN syntax directly.
+   * When a JOIN's right side is another JOIN, it must be wrapped
+   * in a SELECT * FROM (...) subquery. */
+  @Test void testClickHouseNestedJoin() {
+    final String query = "SELECT e.empno, j.dname, j.loc\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname, d2.loc\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + ") AS j ON e.deptno = j.deptno";
+
+    final String sql = sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .exec();
+
+    assertThat(sql, containsString("LEFT JOIN (SELECT *"));
+    assertThat(sql, containsString("FROM (SELECT `DEPT`.`DEPTNO`, 
`DEPT`.`DNAME`, `DEPT0`.`LOC`"));
+    assertThat(sql, containsString("INNER JOIN `SCOTT`.`DEPT` AS `DEPT0`"));
+    assertTrue(sql.matches("(?s).*AS `t\\d+`\\) AS `t\\d+`.*"));
+  }
+
+  /** Test that simple JOINs without nesting are not wrapped. */
+  @Test void testClickHouseSimpleJoinNotWrapped() {
+    final String query = "SELECT e.empno, d.dname\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN dept d ON e.deptno = d.deptno";
+    final String expected = "SELECT `EMP`.`EMPNO`, `DEPT`.`DNAME`\n"
+        + "FROM `SCOTT`.`EMP`\n"
+        + "LEFT JOIN `SCOTT`.`DEPT` ON `EMP`.`DEPTNO` = `DEPT`.`DEPTNO`";
+    sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .ok(expected);
+  }
+
+  /** Test INNER JOIN with nested right side. */
+  @Test void testClickHouseNestedInnerJoin() {
+    final String query = "SELECT e.empno, j.dname\n"
+        + "FROM emp e\n"
+        + "INNER JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + ") AS j ON e.deptno = j.deptno";
+
+    final String sql = sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .exec();
+
+    assertThat(sql, containsString("INNER JOIN (SELECT *"));
+    assertThat(sql, containsString("FROM (SELECT `DEPT`.`DEPTNO`, 
`DEPT`.`DNAME`"));
+    assertThat(sql, containsString("INNER JOIN `SCOTT`.`DEPT` AS `DEPT0`"));
+    assertTrue(sql.matches("(?s).*AS `t\\d+`\\) AS `t\\d+`.*"));
+  }
+
+  /** Test three-way JOIN where optimization may create nested structure. */
+  @Test void testClickHouseThreeWayJoin() {
+    final String query = "SELECT e.empno, d1.dname, d2.loc\n"
+        + "FROM emp e\n"
+        + "INNER JOIN dept d1 ON e.deptno = d1.deptno\n"
+        + "INNER JOIN dept d2 ON d1.deptno = d2.deptno";
+    final String expected = "SELECT `EMP`.`EMPNO`, `DEPT`.`DNAME`, 
`DEPT0`.`LOC`\n"
+        + "FROM `SCOTT`.`EMP`\n"
+        + "INNER JOIN `SCOTT`.`DEPT` ON `EMP`.`DEPTNO` = `DEPT`.`DEPTNO`\n"
+        + "INNER JOIN `SCOTT`.`DEPT` AS `DEPT0` "
+        + "ON `DEPT`.`DEPTNO` = `DEPT0`.`DEPTNO`";
+    sql(query)
+        .schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
+        .withClickHouse()
+        .ok(expected);
+  }
+
+  /** Test nested JOIN with WHERE clause in subquery. */
+  @Test void testClickHouseNestedJoinWithWhere() {
+    final String query = "SELECT e.empno, j.dname\n"
+        + "FROM emp e\n"
+        + "LEFT JOIN (\n"
+        + "  SELECT d1.deptno, d1.dname\n"
+        + "  FROM dept d1\n"
+        + "  INNER JOIN dept d2 ON d1.deptno = d2.deptno\n"
+        + "  WHERE d1.deptno < 30\n"
+        + ") AS j ON e.deptno = j.deptno";
+

Review Comment:
   Please try to maintain a consistent style when using blank lines in your 
test code.



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