rongrong commented on code in PR #4439:
URL: https://github.com/apache/calcite/pull/4439#discussion_r2167440378
##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##########
@@ -213,7 +214,11 @@ protected boolean coerceColumnType(
}
RelDataType targetType3 = syncAttributes(validator.deriveType(scope,
node), targetType);
SqlNode node3 = castTo(node, targetType3);
- if (node.getKind() == SqlKind.IDENTIFIER) {
+ // Although this function is called coerceColumnType, it is not always
invoked on a "column".
+ // Preserve the original column name only if this expression is an item in
a select list.
+ boolean isSelectItem = (scope instanceof SelectScope)
Review Comment:
I mean, if I understood this correctly, the original change is just for
readability of the plan. Personally, adding an alias here with special checks
so you can improve plan readability is a bit strange. I'd probably look into
how to make the automatically assigning alias logic to be smarter (instead of
generating `EXPR$n`, maybe take a prefix?) which would improve plan readability
for a lot of other places as well. Though I haven't looked into that so I have
no idea whether it's easy to achieve. Personality I was also not bothered by
the not so readable plan. But these are very personal opinions, please ignore.
--
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]
