You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2022/08/25 16:34:42 UTC
[GitHub] [calcite] goldmedal opened a new pull request, #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
goldmedal opened a new pull request, #2883:
URL: https://github.com/apache/calcite/pull/2883
Invoke `ORDINAL` operator when accessing the array elements.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
Posted by GitBox <gi...@apache.org>.
libenchao commented on code in PR #2883:
URL: https://github.com/apache/calcite/pull/2883#discussion_r956529675
##########
core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java:
##########
@@ -183,6 +183,15 @@ public BigQuerySqlDialect(SqlDialect.Context context) {
case TRIM:
unparseTrim(writer, call, leftPrec, rightPrec);
break;
+ case ITEM:
Review Comment:
`ITEM` is not only used for `array`, but also for `map`, `struct`. So it's better to check whether the `operand(0)` is an `array`.
Besides, does `ORDINAL` only work in the array accessing in BigQuery? Are there any other places that also need to be handled like this?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] goldmedal commented on a diff in pull request #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
Posted by GitBox <gi...@apache.org>.
goldmedal commented on code in PR #2883:
URL: https://github.com/apache/calcite/pull/2883#discussion_r956537645
##########
core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java:
##########
@@ -183,6 +183,15 @@ public BigQuerySqlDialect(SqlDialect.Context context) {
case TRIM:
unparseTrim(writer, call, leftPrec, rightPrec);
break;
+ case ITEM:
Review Comment:
`ORIDANL` is an [Array subscript operator](https://cloud.google.com/bigquery/docs/reference/standard-sql/operators#array_subscript_operator) in BigQuery. I didn't see any other places needs this operator.
BTW, because `arrray subscript operator` has `ORDIANL`, `SAFE_ORDINAL`, `OFFST` and `SAFE_OFFSET`, should we offer a config for the user to switch which one is preferred?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
Posted by GitBox <gi...@apache.org>.
libenchao commented on code in PR #2883:
URL: https://github.com/apache/calcite/pull/2883#discussion_r956656282
##########
core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java:
##########
@@ -183,6 +187,21 @@ public BigQuerySqlDialect(SqlDialect.Context context) {
case TRIM:
unparseTrim(writer, call, leftPrec, rightPrec);
break;
+ case ITEM:
+ if (call.operand(0) instanceof SqlBasicCall
Review Comment:
Sorry that I didn't realized that in `SqlDialect` we do not have validated types. It's indeed not easy to know the operand's type here.
I would suggest that we go back to the JIRA to discuss the design first, I'll post my concerns there.
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] goldmedal commented on a diff in pull request #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
Posted by GitBox <gi...@apache.org>.
goldmedal commented on code in PR #2883:
URL: https://github.com/apache/calcite/pull/2883#discussion_r956537645
##########
core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java:
##########
@@ -183,6 +183,15 @@ public BigQuerySqlDialect(SqlDialect.Context context) {
case TRIM:
unparseTrim(writer, call, leftPrec, rightPrec);
break;
+ case ITEM:
Review Comment:
> Besides, does `ORDINAL` only work in the array accessing in BigQuery? Are there any other places that also need to be handled like this?
`ORIDANL` is an [Array subscript operator](https://cloud.google.com/bigquery/docs/reference/standard-sql/operators#array_subscript_operator) in BigQuery. I didn't see any other places needs this operator.
BTW, because `arrray subscript operator` has `ORDIANL`, `SAFE_ORDINAL`, `OFFST` and `SAFE_OFFSET`, should we offer a config for the user to switch which one is preferred?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator [calcite]
Posted by "goldmedal (via GitHub)" <gi...@apache.org>.
goldmedal closed pull request #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
URL: https://github.com/apache/calcite/pull/2883
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] goldmedal commented on a diff in pull request #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
Posted by GitBox <gi...@apache.org>.
goldmedal commented on code in PR #2883:
URL: https://github.com/apache/calcite/pull/2883#discussion_r956556202
##########
core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java:
##########
@@ -183,6 +187,21 @@ public BigQuerySqlDialect(SqlDialect.Context context) {
case TRIM:
unparseTrim(writer, call, leftPrec, rightPrec);
break;
+ case ITEM:
+ if (call.operand(0) instanceof SqlBasicCall
Review Comment:
I tried to handle the SQL which access the element of a table column like
```sql
WITH POST.DEPT_GBY_EMP_NAME AS {
select * from (values
(10, array['Jane', 'Bob']),
(20, array['Eric']),
(30, array['Susan', 'Alice']),
(40, array[null])) as t(deptno, employees)
}
SELECT D.EMPLOYEES[1] FROM POST.DEPT_GBY_EMP_NAME D
```
I found it's hard to recognize which type the column is here. It could be a `ARRAY`, `MAP` or `STRUCT`. Is there any better way to do this here?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] goldmedal commented on a diff in pull request #2883: [CALCITE-5245] Array element accessing in BigQuery should invoke `ORDINAL` operator
Posted by GitBox <gi...@apache.org>.
goldmedal commented on code in PR #2883:
URL: https://github.com/apache/calcite/pull/2883#discussion_r956554074
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -6466,6 +6466,32 @@ private void checkLiteral2(String expression, String expected) {
.withBigQuery().ok(expected);
}
+ @Test void testBigQueryArrayElementAccessing() {
+ final String arrayCtorSql = ""
+ + "SELECT ARRAY[1, 2, 3][1]";
+ final String expectedArrayCtorSql = ""
+ + "SELECT ARRAY[1, 2, 3][ORDINAL(1)]";
+ sql(arrayCtorSql)
+ .withBigQuery().ok(expectedArrayCtorSql);
+
+ // TODO: Got error from
+ // `SqlImplementor`: Need to implement org.apache.calcite.rel.core.Collect
Review Comment:
I tried to add test for `ARRAY_QUERY` but got this error. Should we create another PR for it?
--
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org