cjj2010 commented on code in PR #4912:
URL: https://github.com/apache/calcite/pull/4912#discussion_r3271902243
##########
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:
Thanks for the careful review — these are good questions.
On the SqlParserPos.ZERO change:
The SqlIdentifier nodes created inside expandExprFromJoin (i.e., the t1.col
and t2.col sub-expressions that form the COALESCE) are not parsed from user
input — they are constructed programmatically by the expansion logic. The user
only writes col; the framework builds the qualified forms internally. Since
validation is now performed before expansion, directly on the original
user-written identifiers (which carry proper source positions), these
internally generated nodes no longer need to carry a meaningful position. Using
identifier.getParserPosition() on them was misleading — it implied they were
the same node as the original identifier, which they are not. SqlParserPos.ZERO
is the standard convention in Calcite for internally generated nodes (see also
expandStar, validateFrom, etc.).
On "synthetic identifier":
I used "synthetic" informally to mean "created programmatically, not parsed
from user SQL." I agree the term is not ideal — I'll remove it from the comment
to avoid confusion.
On the SqlAdvisor concern:
The concern was hypothetical, not based on a concrete failing case.
SqlAdvisorValidator overrides expand, expandSelectExpr, and expandOrderExpr to
return expressions unchanged, so expandExprFromJoin is not reachable from that
path. More importantly, error reporting quality is not degraded: validation
errors are now thrown on the original user-written identifiers (before
expansion), which always carry real source positions. So users will still see
accurate error locations.
--
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]
