Dwrite commented on code in PR #5036:
URL: https://github.com/apache/calcite/pull/5036#discussion_r3601851538
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##########
@@ -546,26 +546,55 @@ public Result visit(Correlate e) {
null);
return result(join, leftResult, rightResult);
}
-
/** Visits a Filter; called by {@link #dispatch} via reflection. */
public Result visit(Filter e) {
final RelNode input = e.getInput();
+ final Set<CorrelationId> definedHere = e.getVariablesSet();
+
if (input instanceof Aggregate) {
final Aggregate aggregate = (Aggregate) input;
final boolean ignoreClauses = aggregate.getInput() instanceof Project;
- final Result x =
+ Result x =
visitInput(e, 0, isAnon(), ignoreClauses,
ImmutableSet.of(Clause.HAVING));
+ final boolean pushed = !definedHere.isEmpty()
+ && e.getInput().getInputs().size() <= 1;
+ if (pushed) {
+ String alias = x.neededAlias;
+ if (alias != null) {
+ x = x.resetAliasForCorrelation(alias, e.getInput().getRowType());
+ } else {
+ alias = unqualifiedName(x.node);
+ if (alias == null) {
+ alias = "t";
+ }
+ x = x.resetAliasForCorrelation
Review Comment:
That's a fair concern, and I don't currently have a verified answer for it.
This "t" fallback isn't new to this PR — it mirrors the exact same
pattern already in visit(Project) from CALCITE-7343. It only fires when
neededAlias is null AND unqualifiedName(node) is also null, i.e. when
e.getInput() renders as a derived SqlSelect rather than a bare table
reference (e.g. a Project/Aggregate sitting over a Join).
I haven't yet tested the specific scenario you're pointing at: two
independent correlated scopes that both fall into this exact fallback
and both land on the hardcoded "t". If that can happen within nested
scopes, it would reproduce the original aliasing problem this PR is
meant to fix, just one level removed.
I'll write a test for that (nested correlated sub-queries where both
the outer and inner Filter/Project have non-table-scan inputs) and
check whether it collides. If it does, SqlValidatorUtil.uniquify(...)
is already used elsewhere in this file (visit(Join)'s wrapNestedJoin
path) for exactly this kind of collision avoidance, and I can apply
the same approach here.
--
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]
