Dwrite commented on code in PR #4692:
URL: https://github.com/apache/calcite/pull/4692#discussion_r2649749418
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -233,7 +300,13 @@ public Result visit(Join e) {
break;
}
final Result leftResult = visitInput(e, 0).resetAlias();
- final Result rightResult = visitInput(e, 1).resetAlias();
+ Result rightResult = visitInput(e, 1).resetAlias();
+
+ if (dialect.shouldWrapNestedJoin(e)) {
+ String newAlias = "t" + e.getId(); // alias
Review Comment:
You are absolutely right. I agree that even a statistically low probability
of collision is unacceptable for a SQL generator, and a compiler must be robust
for all possible user programs.
I have updated the implementation to replace the t + id logic with a more
rigorous alias generation approach:
Unique Alias Generation: I now use SqlValidatorUtil.uniquify to generate the
subquery alias. To ensure 100% safety, I collect all field names from the
Join's rowType and the neededAlias into a usedNames set. This ensures the
generated identifier (e.g., t, t0, t1...) will never collide with existing
table or column names in the current scope.
In-place Aliasing: The wrapNestedJoin method now manipulates the SqlSelect
directly. It forces an explicit AS projection for every column using the
standardized field names from RelDataType. This "flattens" the identifiers and
makes them resolvable by the outer query block in ClickHouse, while avoiding
redundant subquery nesting.
This change ensures that the generated SQL is both ClickHouse-compatible and
formally correct, preventing any potential identifier shadowing as you pointed
out. Thank you for the guidance on maintaining compiler-level robustness!
--
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]
