Dwrite commented on code in PR #4692:
URL: https://github.com/apache/calcite/pull/4692#discussion_r2640232420
##########
core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java:
##########
@@ -404,4 +411,45 @@ private static void unparseFloor(SqlWriter writer, SqlCall
call) {
call.operand(0).unparse(writer, 0, 0);
writer.endList(frame);
}
+
+ @Override public boolean supportWrapNestedJoin(RelNode rel) {
Review Comment:
That is a great point regarding architectural consistency. The reason I
passed the RelNode to the dialect instead of using a simple boolean flag is
that not all nested JOINs require wrapping in ClickHouse.
1. Context-Specific Wrapping: ClickHouse's syntax restriction primarily
targets cases where the right side of a JOIN is another JOIN. If we used a
global supportWrapNestedJoin() boolean, RelToSqlConverter might end up wrapping
every single JOIN or every nested structure indiscriminately, which would lead
to redundant subqueries and potential performance regressions for simple linear
joins (e.g., A JOIN B JOIN C).
2. Scoping & Aliasing Complexity: As discussed, the fix isn't just about
adding a SELECT * wrapper; it's about explicitly aliasing the projected columns
based on the specific RelDataType of that join node. By passing the RelNode,
the dialect (or the converter guided by the dialect) can inspect the row type
to ensure identifiers are flattened correctly.
3. Flexibility for Future Dialects: Some dialects might only need wrapping
for specific types of JOINs (e.g., RIGHT JOIN or OUTER JOIN). Passing the
RelNode provides the necessary metadata (like JoinRelType) to make an informed
decision, whereas a simple boolean flag like requiresAliasForFromItems is too
"blunt" for this specific scoping issue.
However, I agree we should keep the common logic in RelToSqlConverter. My
implementation aims to keep the "When to wrap" decision in the Dialect (since
it's a dialect-specific quirk) while keeping the "How to wrap" logic in the
Converter.
--
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]
