cjj2010 commented on code in PR #4912:
URL: https://github.com/apache/calcite/pull/4912#discussion_r3265530082
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -7530,7 +7531,7 @@ private SqlNode expandExprFromJoin(SqlJoin join,
SqlIdentifier identifier, Selec
final SqlIdentifier exp =
new SqlIdentifier(
ImmutableList.of(child.name, name),
- identifier.getParserPosition());
+ SqlParserPos.ZERO);
Review Comment:
> why remove the position from the identifier?
Thank you for reviewing this pull request (PR). The identifier here is
synthetic — the validator fabricates EMP.DEPTNO / DEPT.DEPTNO when expanding
JOIN ... USING(deptno) into COALESCE(EMP.DEPTNO, DEPT.DEPTNO) AS DEPTNO. The
user never typed it.
Three reasons to use SqlParserPos.ZERO:
1. SqlParserPos.ZERO is documented as "use this if the node doesn't
correspond to a position in piece of SQL text." The surrounding COALESCE and AS
in this same expansion already use ZERO; the inner identifiers should match.
2. With identifier.getParserPosition(), the synthetic EMP.DEPTNO was
indistinguishable from a user-written one. When it re-entered
expandCommonColumn, conformances with allowQualifyingCommonColumn() == false
(Oracle, Presto, …) raised a spurious Cannot qualify common column
'EMP.DEPTNO'. Pairing this change with the new pre-expansion check
validateNoQualifiedCommonColumns cleanly separates "user wrote it" (real
position, rejected) from "validator synthesized it" (ZERO, allowed).
3. Any future error raised on this node would otherwise underline the user's
deptno and report EMP.DEPTNO — misleading for SqlAdvisor.
--
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]
