tmater commented on code in PR #4844:
URL: https://github.com/apache/calcite/pull/4844#discussion_r3100545931
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -3774,6 +3774,24 @@ SqlNode Expression(ExprContext exprContext) :
list = Expression2(exprContext) { return SqlParserUtil.toTree(list); }
}
+void AddRegularPostfixes(List<Object> list) :
+{
+ SqlNode ext;
+}
+{
+ (
+ LOOKAHEAD(2) <DOT>
+ ext = RowExpressionExtension() {
+ list.add(
+ new SqlParserUtil.ToTreeListItem(
+ SqlStdOperatorTable.DOT, getPos()));
Review Comment:
I ran into this downstream while building on top of this work. It gets hard
later to tell whether the original syntax was `:` or `.`, since : is rewritten
to DOT / ITEM immediately, so I think a dedicated follow-up makes sense there.
I’d still prefer to keep this PR scoped to the parser change, and I opened
CALCITE-7476 for the next step.
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -3774,6 +3774,48 @@ SqlNode Expression(ExprContext exprContext) :
list = Expression2(exprContext) { return SqlParserUtil.toTree(list); }
}
+/** Parses regular postfixes such as {@code a.b}, {@code a[1]} and
+ * {@code a[1].b}. Shared by the main expression grammar and Babel's
+ * infix cast. */
+void AddRegularPostfixes(List<Object> list) :
+{
+ SqlNode ext;
+}
+{
+ (
+ LOOKAHEAD(2) <DOT>
+ ext = RowExpressionExtension() {
+ list.add(
+ new SqlParserUtil.ToTreeListItem(
+ SqlStdOperatorTable.DOT, getPos()));
+ list.add(ext);
+ }
+ |
+ AddBracketPostfix(list)
+ )*
+}
+
+/** Parses postfixes that are allowed after colon field access, for example
+ * {@code v:field.nested} or {@code v:[1]['x']}. Unlike regular postfixes,
+ * this path does not allow member-style calls such as {@code v:field.func()}.
*/
+void AddColonPostfixes(List<Object> list) :
Review Comment:
I added a separate postfix method for colon access because this came up
during downstream review. Reusing the regular postfix path was too permissive,
since it would also allow the same member-style calls as dot access, while for
`:` we only want path continuation with `.` and `[]`; Spark/Databricks blocks
that broader dot-style behavior as well.
##########
babel/src/test/java/org/apache/calcite/test/BabelParserTest.java:
##########
@@ -301,6 +306,101 @@ private void checkParseInfixCast(String sqlType) {
sql(sql).ok(expected);
}
+ @Test void testColonFieldAccessWithInfixCast() {
+ final SqlParserFixture f =
+ fixture().withConformance(new SqlAbstractConformance() {
+ @Override public boolean isColonFieldAccessAllowed() {
+ return true;
+ }
+ });
+
+ // Bracket after :: binds to the type, not as subscript on the cast result
+ sql("select v::varchar array[1] from t")
+ .ok("SELECT `V` :: VARCHAR ARRAY[1]\nFROM `T`");
+ f.sql("select v::varchar array[1] from t")
Review Comment:
Filed [CALCITE-7475](https://issues.apache.org/jira/browse/CALCITE-7475) to
cover this part and narrowed down the test surface.
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -3791,13 +3809,61 @@ void AddExpression2b(List<Object> list, ExprContext
exprContext) :
e = Expression3(exprContext) {
list.add(e);
}
+ AddRegularPostfixes(list)
+ [
+ LOOKAHEAD(2, <COLON> SimpleIdentifier(),
+ { this.conformance.isColonFieldAccessAllowed() })
+ <COLON>
+ ext = SimpleIdentifier() {
+ list.add(
+ new SqlParserUtil.ToTreeListItem(
+ SqlStdOperatorTable.DOT, getPos()));
+ list.add(ext);
+ }
+ AddRegularPostfixes(list)
+ |
+ LOOKAHEAD(2, <COLON> <LBRACKET>,
+ { this.conformance.isColonFieldAccessAllowed() })
+ <COLON>
+ AddBracketAccess(list)
+ AddRegularPostfixes(list)
+ ]
+}
+
+void AddBracketAccess(List<Object> list) :
Review Comment:
Done
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -3791,13 +3809,61 @@ void AddExpression2b(List<Object> list, ExprContext
exprContext) :
e = Expression3(exprContext) {
list.add(e);
}
+ AddRegularPostfixes(list)
+ [
+ LOOKAHEAD(2, <COLON> SimpleIdentifier(),
+ { this.conformance.isColonFieldAccessAllowed() })
+ <COLON>
Review Comment:
Yes, for the subset this patch is targeting, it covers the common
Snowflake/Databricks colon-path surface.
Snowflake is proprietary, I could only reverse engineer/check docs. Examples
in the docs are `src:salesperson.name`, `src:vehicle[0].price`, and
`src:vehicle[0].price::NUMBER`, which matches the syntax this change adds. But
experimented with it and `s:payload.score::INT` works too.
For Databricks, the public docs and the open Spark grammar match this
implementation well. Spark treats `:` as semi-structured field access and `::`
as a separate cast operator. That matches the forms added here, such as
`v:field`, `v:['field']`, `arr[1]:field`, `v:field[1]`, and `v:field::int`.
Reference:
[SqlBaseParser.g4#L1316](https://github.com/apache/spark/blob/master/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4#L1316)
--
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]
