mihaibudiu commented on code in PR #3423:
URL: https://github.com/apache/calcite/pull/3423#discussion_r1326376733


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java:
##########
@@ -78,7 +78,13 @@ public SqlItemOperator(String name,
       SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) {
     call.operand(0).unparse(writer, leftPrec, 0);
     final SqlWriter.Frame frame = writer.startList("[", "]");
-    call.operand(1).unparse(writer, 0, 0);
+    if (offset == 0) {

Review Comment:
   The BigQuerySqlDialect already handles this correctly.
   The bug I am fixing was caused by SqlOperatorTest, where tests look like 
this:
   ```
   final SqlOperatorFixture f0 = fixture();
       f0.setFor(SqlLibraryOperators.SAFE_ORDINAL);
       final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
       f.checkScalar("ARRAY[2,4,6][SAFE_ORDINAL(3)]", "6", "INTEGER");    
   ```
   
   Notice that this test only sets up the *library*, but not the dialect. In a 
new test fixture that I am writing, which unparses and reparses the query 
before executing, this test fails because the query is not unparsed correctly. 
In this case the unparsing does not go through the dialect.
   
   I don't see a way to set the dialect in the SqlOperatorFixture. Do you have 
a suggestion?



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