Dwrite commented on code in PR #4916:
URL: https://github.com/apache/calcite/pull/4916#discussion_r3227073887
##########
babel/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -197,17 +197,31 @@ SqlCreate SqlCreateTable(Span s, boolean replace) :
void InfixCast(List<Object> list, ExprContext exprContext, Span s) :
{
final SqlDataTypeSpec dt;
+ SqlNode e, p;
}
{
<INFIX_CAST> {
checkNonQueryExpression(exprContext);
}
dt = DataType() {
- list.add(
- new SqlParserUtil.ToTreeListItem(SqlLibraryOperators.INFIX_CAST,
- s.pos()));
- list.add(dt);
+ SqlNode leftOperand = (SqlNode) list.remove(list.size() - 1);
Review Comment:
I totally understand the concern, but the logic here is strictly 'Cast
First, then Access' （DOT(ITEM(CAST(v, VARCHAR ARRAY), 1), FIELD).
The reason we handle ITEM and DOT within this rule is that the :: operator
has a specific precedence relationship with them. In our parser, if we return
to the main expression loop immediately after parsing the type, the trailing
[n] or .field often gets 'lost' or incorrectly bound because the type
specification itself is greedy.
By handling them here, we ensure:
The CAST node is built first using the left operand and the type.
The resulting CAST node is then wrapped by ITEM or DOT calls.
Effectively, this rule defines the scope of the infix cast result. Renaming
it to something like InfixCastOrFieldAccess might be misleading, as this rule
is never the entry point for a standalone field access—it only handles field
access specifically on the result of a :: cast.
For these reasons, I believe InfixCast remains the most accurate name as it
describes the operator that triggers this entire logic.
--
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]
