tmater commented on code in PR #4844:
URL: https://github.com/apache/calcite/pull/4844#discussion_r3085805331


##########
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:
   I see your point, this is quite ambiguous. The reason I added these cases 
was mainly to show that we did not introduce regressions while touching the 
`INFIX_CAST` grammar.
   
   For `v::varchar array[1]`, the tree is something like:
   ```
     INFIX_CAST(
       v,
       ITEM(SqlDataTypeSpec(VARCHAR ARRAY), 1)
     )
   ```
   So the `[1]` is attaching on the type side, not as a subscript on the cast 
result.
   
   Also, I backported these tests onto the base of this PR as proof: 
[#4885](https://github.com/apache/calcite/pull/4885). The same behavior is 
already present there, so this is not something introduced by the 
colon-field-access change.
   
   Would you prefer fixing this ambiguity in this PR, or filing a Jira to clean 
it up separately? I am leaning towards a separate PR and polishing these tests 
a bit, maybe leaving only one ambiguous expression test just in case.



-- 
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]

Reply via email to