You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "tanclary (via GitHub)" <gi...@apache.org> on 2023/04/19 17:24:37 UTC
[GitHub] [calcite] tanclary opened a new pull request, #3167: [CALCITE-5560] Implement BigQuery Array Subscript Operators
tanclary opened a new pull request, #3167:
URL: https://github.com/apache/calcite/pull/3167
Add support for BigQuery Array Subscript Operators `OFFSET`, `ORDINAL`, `SAFE_OFFSET`, and `SAFE_ORDINAL`.
These operators are similar to the existing `ITEM` operator used to index an array. For this PR, that implementation was parameterized to accept different offsets and out-of-bounds behavior.
[BigQuery docs](https://cloud.google.com/bigquery/docs/reference/standard-sql/operators#array_subscript_operator)
Any comments as always are appreciated, thanks!
--
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] julianhyde commented on a diff in pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1187829498
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3626,11 +3626,15 @@ public static String replace(String s, String search, String replacement) {
/** Helper for "array element reference". Caller has already ensured that
* array and index are not null. Index is 1-based, per SQL. */
- public static @Nullable Object arrayItem(List list, int item) {
- if (item < 1 || item > list.size()) {
- return null;
+ public static @Nullable Object arrayItem(List list, int item, int offset, boolean safe) {
Review Comment:
javadoc says 'index is 1-based, per SQL'. It is now lying.
You MUST revise javadoc.
--
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] tanclary commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1538831318
> @tanclary Please fix conflicts and if there aren't objections, I will merge it tomorrow.
@dssysolyatin , Just rebased and fixed conflicts, should be good now. Thanks!
--
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] tanclary commented on a diff in pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1175500937
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -678,7 +683,13 @@ Builder populate2() {
final RexCallImplementor value = new ValueConstructorImplementor();
map.put(MAP_VALUE_CONSTRUCTOR, value);
map.put(ARRAY_VALUE_CONSTRUCTOR, value);
+
+ // Each subscript operator has a corresponding offset and 'safe' value.
map.put(ITEM, new ItemImplementor());
+ map.put(OFFSET, new ItemImplementor());
Review Comment:
Thanks for the idea, I definitely agree. Should be fixed now.
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518189193
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518145701
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] tanclary commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518918378
> Should `SELECT OFFSET(1)` be valid? I don't think so, because `OFFSET` is being used outside of an array. But please check in BigQuery and test accordingly.
@julianhyde No it should not be valid. I've checked in BigQuery and added a quidem test as well.
--
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] dssysolyatin commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1538813549
@tanclary Please fix conflicts and if there aren't objections, I will merge it tomorrow.
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518157556
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5560] Implement BigQuery Array Subscript Operators
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1515209299
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1538853867
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] tanclary commented on a diff in pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1187922489
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3090,6 +3102,13 @@ private static class ItemImplementor extends AbstractRexCallImplementor {
final RexCall call, final List<Expression> argValueList) {
final MethodImplementor implementor =
getImplementor(call.getOperands().get(0).getType().getSqlTypeName());
+ // If the structure is an array, two additional arguments are added to the argValueList
+ // to denote what the offset is and what the behavior should be if the index is out of boudns.
+ if (implementor.method.getName().equals("arrayItemOptional")) {
Review Comment:
I added a commit that tries to tidy this up by checking the SqlTypeName of the collection argument instead of checking the implementor method. Let me know what you think.
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1520472729
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518202279
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Implement BigQuery Array Subscript Operators
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1515409062
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518135782
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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] tanclary commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518133174
> Change the commit message to 'Add array subscript operators OFFSET, ORDINAL (enabled for BigQuery)', just to conform to the usual format.
>
> Are people allowed to call these operators outside of an array? If so, write tests to make sure they can't, even in BigQuery.
>
> Is `array[OFFSET(2)+OFFSET(3)]` valid? Is `array[OFFSET(2 + 3)]` valid?
>
> Good work; this PR looks very close.
@julianhyde
- Commit message should be changed.
- These operators are solely for array expressions from what I can tell in the docs, so I parameterized the operand type checker as well. I added some test cases for this.
- OFFSET(2) + OFFSET(3) should not be valid (and it isn't). OFFSET(2+3) should be valid (and it is).
--
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] julianhyde commented on a diff in pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1187827849
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3090,6 +3102,13 @@ private static class ItemImplementor extends AbstractRexCallImplementor {
final RexCall call, final List<Expression> argValueList) {
final MethodImplementor implementor =
getImplementor(call.getOperands().get(0).getType().getSqlTypeName());
+ // If the structure is an array, two additional arguments are added to the argValueList
+ // to denote what the offset is and what the behavior should be if the index is out of boudns.
+ if (implementor.method.getName().equals("arrayItemOptional")) {
Review Comment:
Hard-coded method names are too fragile.
Especially strange given that getImplementor returns BuiltInMethod.ARRAY_ITEM and there's no BuiltInMethod.ARRAY_ITEM_OPTIONAL.
Reading this code, I first thought it was because of the 'optional'. But actually it's because it's an array.
So, figure out a way to make this code more robust and intuitive. It's not sufficient that it works.
--
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] asfgit closed pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
URL: https://github.com/apache/calcite/pull/3167
--
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] dssysolyatin commented on a diff in pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "dssysolyatin (via GitHub)" <gi...@apache.org>.
dssysolyatin commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1175013204
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -678,7 +683,13 @@ Builder populate2() {
final RexCallImplementor value = new ValueConstructorImplementor();
map.put(MAP_VALUE_CONSTRUCTOR, value);
map.put(ARRAY_VALUE_CONSTRUCTOR, value);
+
+ // Each subscript operator has a corresponding offset and 'safe' value.
map.put(ITEM, new ItemImplementor());
+ map.put(OFFSET, new ItemImplementor());
Review Comment:
It would be better to use a single instance of ItemImplementor instead of creating separate instances for each operator. Apart from that, everything else looks good.
--
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] julianhyde commented on a diff in pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1187828340
##########
core/src/main/java/org/apache/calcite/runtime/CalciteResource.java:
##########
@@ -885,6 +885,9 @@ ExInst<CalciteException> illegalArgumentForTableFunctionCall(String a0,
@BaseMessage("Third argument (pad pattern) for LPAD/RPAD must not be empty")
ExInst<CalciteException> illegalEmptyPadPattern();
+ @BaseMessage("Array index {0,number,#} is out of bounds.")
Review Comment:
remove the period
--
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] julianhyde commented on a diff in pull request #3167: [CALCITE-5660] Implement BigQuery Array Subscript Operators
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1173179787
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -678,7 +682,13 @@ Builder populate2() {
final RexCallImplementor value = new ValueConstructorImplementor();
map.put(MAP_VALUE_CONSTRUCTOR, value);
map.put(ARRAY_VALUE_CONSTRUCTOR, value);
- map.put(ITEM, new ItemImplementor());
+
+ // Each subscript operator has a corresponding offset and 'safe' value.
Review Comment:
Better to move `offset` and `safe` from `ItemImplementor` into `SqlItemOperator` (make them public final fields)
Right now you're leaving a little too heavily on object identity (and there's a danger that future maintainers will branch based on the name of the operator, which will be bad).
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3372,11 +3372,16 @@ public static String replace(String s, String search, String replacement) {
/** Helper for "array element reference". Caller has already ensured that
* array and index are not null. Index is 1-based, per SQL. */
- public static @Nullable Object arrayItem(List list, int item) {
- if (item < 1 || item > list.size()) {
- return null;
+ public static @Nullable Object arrayItem(List list, int item, int offset, boolean safe) {
+
Review Comment:
remove the blank line
--
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] julianhyde commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518737883
Should `SELECT OFFSET(1)` be valid? I don't think so, because `OFFSET` is being used outside of an array. But please check in BigQuery and test accordingly.
--
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] sonarcloud[bot] commented on pull request #3167: [CALCITE-5660] Add array subscript operators OFFSET, ORDINAL, SAFE_OFFSET, SAFE_ORDINAL (enabled for BigQuery)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3167:
URL: https://github.com/apache/calcite/pull/3167#issuecomment-1518921951
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3167)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3167&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3167&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list) [98.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3167&metric=new_duplicated_lines_density&view=list)
--
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