You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "caicancai (via GitHub)" <gi...@apache.org> on 2024/02/26 15:15:04 UTC
[PR] [CALCITE-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
caicancai opened a new pull request, #3705:
URL: https://github.com/apache/calcite/pull/3705
(no comment)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507356528
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6297,6 +6297,54 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
"INTEGER is not comparable to BOOLEAN", false);
}
+ /** Test case for
+ * <a href="https://github.com/apache/calcite/pull/3705">[CALCITE-5976]
+ * Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert
+ * does not equal derived component type)</a>. */
+ @Test void testArrayAppendFuncByCast() {
Review Comment:
no need to add a new testCase. put it into the `testArrayAppendFunc`.
--
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-5976] Function ArrayPrepend/ArrayAppend/ArrayInsert gives exception when inserted element type not equals array component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1978012072
> @caicancai please change your commit/pr name.
Thank you
--
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-5976] Function ArrayPrepend/ArrayAppend/ArrayInsert gives exception when inserted element type not equals array component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1980297620
@caicancai looks good to me now. could you change commit/jira name by using 'ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT'. These function names are usually capitalized in commit.
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1980992743
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [99.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [56.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1964395139
I still have some details to finish, I just want to make sure the method is feasible
--
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-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1977841757
@caicancai please change your commit/pr name.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1972595364
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [9 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [56.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970486374
> @caicancai A suggestion, don’t directly @/pin committers to review for you. This will affect other committers or contributors’ participation in your reviews.
Thanks for your reminder
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507078891
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
Review Comment:
There is no need to implement this. For the form of `array_append/array_prepend/array_insert`, the array of the first arg has been cast. the component type has been derived. The only thing you need to do is to cast the element sqlnode into the component type.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970786153
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [7.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1973145979
> @caicancai I have created a commit based on your PR. you can pick and take a look. https://github.com/chucheng92/calcite/tree/CALCITE-5976
Thank you, very much appreciated
--
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-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1977709437
no comment. squash
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1972525578
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [9 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [58.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1969285761
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [8.4% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1980361982
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [53.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1964476690
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [96.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 merged PR #3705:
URL: https://github.com/apache/calcite/pull/3705
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970329621
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [99.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [4.7% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970333055
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [99.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [5.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507247094
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1187,6 +1187,9 @@ private static RelDataType arrayAppendPrependReturnType(SqlOperatorBinding opBin
if (elementType.isNullable()) {
type = opBinding.getTypeFactory().createTypeWithNullability(type, true);
}
+ SqlValidatorUtil.
+ adjustTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
Review Comment:
thank you
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507357141
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6538,6 +6586,54 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
"INTEGER is not comparable to BOOLEAN", false);
}
+ /** Test case for
+ * <a href="https://github.com/apache/calcite/pull/3705">[CALCITE-5976]
+ * Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert
+ * does not equal derived component type)</a>. */
+ @Test void testArrayPrependFuncByCast() {
Review Comment:
put it into the `testArrayPrependFunc`
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6810,6 +6904,31 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
"(INTEGER NOT NULL, CHAR(1) NOT NULL) MAP NOT NULL ARRAY NOT NULL");
}
+ /** Test case for
+ * <a href="https://github.com/apache/calcite/pull/3705">[CALCITE-5976]
+ * Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert
+ * does not equal derived component type)</a>. */
+ @Test void testArrayInsertFuncByCast() {
Review Comment:
same problem.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1375,6 +1376,14 @@ private static void adjustTypeForMultisetConstructor(
} else {
elementType = oddType;
}
+ if (call.getOperator().getName().equals("ARRAY_INSERT") && i == 1) {
Review Comment:
do not hard-code 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
Re: [PR] [CALCITE-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507403331
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1266,10 +1271,17 @@ private static RelDataType arrayInsertReturnType(SqlOperatorBinding opBinding) {
// we don't need to do leastRestrictive on componentType and elementType,
// because in operand checker we limit the elementType must equals array component type.
// So we use componentType directly.
- RelDataType type = componentType;
+ RelDataType type =
+ opBinding.getTypeFactory().leastRestrictive(
+ ImmutableList.of(componentType, elementType));
if (elementType.isNullable()) {
type = opBinding.getTypeFactory().createTypeWithNullability(type, true);
}
+
+ if (componentType != elementType) {
Review Comment:
we can't use != to compare directly. try to use `equalsSansFieldNames`.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1972526319
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [9 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [58.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1990746285
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [99.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [56.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1964454871
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [97.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970484336
@caicancai A suggestion, don’t directly @/pin committers to review for you. This will affect other committers or contributors’ participation in your reviews.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507108833
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
I think here you need also to pass leastRestrictiveType. the cast array and element to this leastRestrictiveType
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507112865
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
1.about method name, maybe adjustTypeForArrayFunction is better. here is not Constructor but array_append/array_prepend/array_insert ...
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
1.about method name, maybe adjustTypeForArrayFunction is better. here is not Constructor but array_append/array_prepend/array_insert ...
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507112865
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
1.about method name, maybe adjustTypeForArrayFunction is better. here is not Constructor
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507353662
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1375,6 +1376,14 @@ private static void adjustTypeForMultisetConstructor(
} else {
elementType = oddType;
}
+ if (call.getOperator().getName().equals("ARRAY_INSERT") && i == 1) {
Review Comment:
pls don't modify `adjustTypeForMultisetConstructor` directly. just add a static method to cast inserted element to array componentType.
--
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-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1972607712
@caicancai please use correct/proper commit name when you fix code-reivews to add a new commit.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507103203
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ caseTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
+ }
+ }
+
+ /**
+ * Implicit conversion of data types in array_append and array_prepend based on the types
+ * of componentType and elementType.
+ *
+ * @param componentType derived array component type
+ * @param elementType derived array elementType type
+ * @param opBinding description of call
+ */
+ public static void caseTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
Review Comment:
This is too vulnerable.
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1187,6 +1187,9 @@ private static RelDataType arrayAppendPrependReturnType(SqlOperatorBinding opBin
if (elementType.isNullable()) {
type = opBinding.getTypeFactory().createTypeWithNullability(type, true);
}
+ SqlValidatorUtil.
+ adjustTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
Review Comment:
why pass elementType? the leastRestrictive type is `type` here
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ caseTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
+ }
+ }
+
+ /**
+ * Implicit conversion of data types in array_append and array_prepend based on the types
+ * of componentType and elementType.
+ *
+ * @param componentType derived array component type
+ * @param elementType derived array elementType type
+ * @param opBinding description of call
+ */
+ public static void caseTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
Review Comment:
This is too vulnerable.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507066839
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6538,6 +6586,54 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
"INTEGER is not comparable to BOOLEAN", false);
}
+ /** Test case for
+ * <a href="https://github.com/apache/calcite/pull/3705">[CALCITE-5976]
+ * Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert
+ * does not equal derived component type)</a>. */
+ @Test void testArrayPrependFuncByCast() {
Review Comment:
This is all the tests I can think of, if anything is missing please let me know and I can make additional verifications
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970501010
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [7 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [99.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [5.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507108833
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
I think here you need also to pass leastRestrictiveType. then cast array and element to this leastRestrictiveType
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507208559
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6538,6 +6586,54 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
"INTEGER is not comparable to BOOLEAN", false);
}
+ /** Test case for
+ * <a href="https://github.com/apache/calcite/pull/3705">[CALCITE-5976]
+ * Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert
+ * does not equal derived component type)</a>. */
+ @Test void testArrayPrependFuncByCast() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_PREPEND);
+ f0.checkFails("^array_prepend(array[1], 2)^",
+ "No match found for function signature ARRAY_PREPEND\\("
+ + "<INTEGER ARRAY>, <NUMERIC>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+ f.checkScalar("array_prepend(array[1], cast(3 as float))", "[3.0, 1.0]",
+ "FLOAT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1], cast(3 as bigint))", "[3, 1]",
+ "BIGINT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[2], cast(3 as double))", "[3.0, 2.0]",
+ "DOUBLE NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as float))", "[3.0, 1.0, 2.0]",
+ "FLOAT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[2, 1], cast(3 as double))", "[3.0, 2.0, 1.0]",
+ "DOUBLE NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as tinyint))", "[3, 1, 2]",
+ "INTEGER NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as bigint))", "[3, 1, 2]",
+ "BIGINT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array(1, 2), cast(null as double))", "[null, 1.0, 2.0]",
Review Comment:
use spark array is enough. we have a issue: https://issues.apache.org/jira/browse/CALCITE-6042
If we have a better solution, then we can use more clean way to support std array & spark array function.
The current way is a bit bloated.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
1.about method name, maybe adjustTypeForArrayRelatedFunction is better. here is not Constructor
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1506166899
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,34 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayAppendPrependConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ switch (elementType.getSqlTypeName().getName()) {
Review Comment:
Why is it written like this?
Because in the case of array_append(array(int), double), the int type in the array in the array_append function will be converted to the same double type as elementType, for example
```scala
scala> val df = spark.sql("select array_append(array(2), cast(2 as double))")
df: org.apache.spark.sql.DataFrame = [array_append(array(2), CAST(2 AS DOUBLE)): array<double>]scala> df.show()
+-----------------------------------------+
|array_append(array(2), CAST(2 AS DOUBLE))|
+-----------------------------------------+
| [2.0, 2.0]|
+-----------------------------------------+
```
In the case of array_append(array(int), tinyint), the tinyint type in the array_append function is converted into the same int type as comentType.
```scala
scala> val df = spark.sql("select array_append(array(2), cast(2 as tinyint))")
df: org.apache.spark.sql.DataFrame = [array_append(array(2), CAST(2 AS TINYINT)): array<int>]scala> df.show()
+------------------------------------------+
|array_append(array(2), CAST(2 AS TINYINT))|
+------------------------------------------+
| [2, 2]|
+------------------------------------------+
```
--
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-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1977750417
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [53.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1968735142
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [11.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970335443
I am not very satisfied with the code structure, but the general idea has been determined. The test codes are all tested in spark. At present, they are consistent with spark.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507078891
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
Review Comment:
There is no need to implement this. For the form of `array_append/array_prepend/array_insert`, the array of the first arg has been cast. the component type has been derived. The only thing you need to do is to cast the elementType into the component type.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507078891
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
Review Comment:
There is no need to implement this. For the form of `array_append/array_prepend/array_insert`, the array of the first arg has been cast. The only thing you need to do is to cast the elementType into the component type.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1968718072
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [11.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1968722925
Please retry analysis of this Pull-Request directly on SonarCloud
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1506166899
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,34 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayAppendPrependConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ switch (elementType.getSqlTypeName().getName()) {
Review Comment:
Why is it written like this?
Because in the case of array_append(array(int), double), the int type in the array in the array_append function will be converted to the same double type as elementType, for example
```scala
scala> val df = spark.sql("select array_append(array(2), cast(2 as double))")
df: org.apache.spark.sql.DataFrame = [array_append(array(2), CAST(2 AS DOUBLE)): array<double>]scala> df.show()
+-----------------------------------------+
|array_append(array(2), CAST(2 AS DOUBLE))|
+-----------------------------------------+
| [2.0, 2.0]|
+-----------------------------------------+
```
In the case of array_append(array(int), tinyint), the tinyint type in the array_append function is converted into the same int type as comentType.
```scala
scala> val df = spark.sql("select array_append(array(2), cast(2 as tinyint))")
df: org.apache.spark.sql.DataFrame = [array_append(array(2), CAST(2 AS TINYINT)): array<int>]scala> df.show()
+------------------------------------------+
|array_append(array(2), CAST(2 AS TINYINT))|
+------------------------------------------+
| [2, 2]|
+------------------------------------------+
```
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,34 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayAppendPrependConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
+ adjustTypeForMultisetConstructor3(
+ elementType, elementType, (SqlCallBinding) opBinding);
+ break;
+ default:
+ if (!componentType.getSqlTypeName().getName().equals("UNKNOWN")) {
Review Comment:
If it is an unknown type, no conversion is required.
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1514073661
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1181,13 +1181,26 @@ private static RelDataType arrayAppendPrependReturnType(SqlOperatorBinding opBin
final RelDataType arrayType = opBinding.collectOperandTypes().get(0);
final RelDataType componentType = arrayType.getComponentType();
final RelDataType elementType = opBinding.collectOperandTypes().get(1);
+ requireNonNull(componentType, () -> "componentType of " + arrayType);
+
RelDataType type =
opBinding.getTypeFactory().leastRestrictive(
ImmutableList.of(componentType, elementType));
+ requireNonNull(type, "inferred array element type");
+
if (elementType.isNullable()) {
type = opBinding.getTypeFactory().createTypeWithNullability(type, true);
}
- requireNonNull(type, "inferred array element type");
+
+ // make explicit CAST for array elements and inserted element to the biggest type
+ // if array component type not equals to inserted element type
+ if (!componentType.equalsSansFieldNames(elementType)) {
+ // 0, 1 is the operand index to be CAST
Review Comment:
here can be optimized. If `arrayComponentType` or inserted element type it is already the same as `leastRestrictiveType`, there is no need to cast 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
Re: [PR] [CALCITE-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1993206124
merging ...
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1989998470
> @chucheng92 I fixed a similar problem. If you have time, can you help me take a look at this PR? Thank you. #3721
looks good to me now, good job. @caicancai
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1508483368
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1375,6 +1376,14 @@ private static void adjustTypeForMultisetConstructor(
} else {
elementType = oddType;
}
+ if (call.getOperator().getName().equals("ARRAY_INSERT") && i == 1) {
Review Comment:
done
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1266,10 +1271,17 @@ private static RelDataType arrayInsertReturnType(SqlOperatorBinding opBinding) {
// we don't need to do leastRestrictive on componentType and elementType,
// because in operand checker we limit the elementType must equals array component type.
// So we use componentType directly.
- RelDataType type = componentType;
+ RelDataType type =
+ opBinding.getTypeFactory().leastRestrictive(
+ ImmutableList.of(componentType, elementType));
if (elementType.isNullable()) {
type = opBinding.getTypeFactory().createTypeWithNullability(type, true);
}
+
+ if (componentType != elementType) {
Review Comment:
done
--
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-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1973169933
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [53.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Function ArrayPrepend/ArrayAppend/ArrayInsert gives exception when inserted element type not equals array component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1977903293
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [53.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507223643
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
removeit
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
remove 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
Re: [PR] [CALCITE-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970319145
> The code is very ugly, please do not review it, I will refactor it later.
you can create a DRAFT PR when PR is not ready.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1970761814
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [7.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1987040158
@chucheng92 I fixed a similar problem. If you have time, can you help me take a look at this PR? Thank you.
https://github.com/apache/calcite/pull/3721
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507110136
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ caseTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
+ }
+ }
+
+ /**
+ * Implicit conversion of data types in array_append and array_prepend based on the types
+ * of componentType and elementType.
+ *
+ * @param componentType derived array component type
+ * @param elementType derived array elementType type
+ * @param opBinding description of call
+ */
+ public static void caseTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
+ case "DECIMAL":
+ adjustTypeForMultisetConstructor3(
+ elementType, elementType, (SqlCallBinding) opBinding);
+ break;
+ default:
+ if (!componentType.getSqlTypeName().getName().equals("UNKNOWN")) {
Review Comment:
if you pass leastRestrictiveType, just cast array to this target type and cast element to this targetType is enough. no need to list these types. It's vulnerable.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507110136
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ caseTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
+ }
+ }
+
+ /**
+ * Implicit conversion of data types in array_append and array_prepend based on the types
+ * of componentType and elementType.
+ *
+ * @param componentType derived array component type
+ * @param elementType derived array elementType type
+ * @param opBinding description of call
+ */
+ public static void caseTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
+ case "DECIMAL":
+ adjustTypeForMultisetConstructor3(
+ elementType, elementType, (SqlCallBinding) opBinding);
+ break;
+ default:
+ if (!componentType.getSqlTypeName().getName().equals("UNKNOWN")) {
Review Comment:
if you pass leastRestrictiveType, just cast array to this target type and cast element to this targetType is enough. no need to list these types. It's vulnerable and a bit ugly.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507110136
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ caseTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
+ }
+ }
+
+ /**
+ * Implicit conversion of data types in array_append and array_prepend based on the types
+ * of componentType and elementType.
+ *
+ * @param componentType derived array component type
+ * @param elementType derived array elementType type
+ * @param opBinding description of call
+ */
+ public static void caseTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
+ case "DECIMAL":
+ adjustTypeForMultisetConstructor3(
+ elementType, elementType, (SqlCallBinding) opBinding);
+ break;
+ default:
+ if (!componentType.getSqlTypeName().getName().equals("UNKNOWN")) {
Review Comment:
if you pass targetType, just cast array to this target type and cast element to this targetType is enough. no need to list these types. It's vulnerable.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507108833
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
Review Comment:
I think here you need to pass 3 args.
arrayComponentType/elementType/targetType.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507208559
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6538,6 +6586,54 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
"INTEGER is not comparable to BOOLEAN", false);
}
+ /** Test case for
+ * <a href="https://github.com/apache/calcite/pull/3705">[CALCITE-5976]
+ * Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert
+ * does not equal derived component type)</a>. */
+ @Test void testArrayPrependFuncByCast() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_PREPEND);
+ f0.checkFails("^array_prepend(array[1], 2)^",
+ "No match found for function signature ARRAY_PREPEND\\("
+ + "<INTEGER ARRAY>, <NUMERIC>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+ f.checkScalar("array_prepend(array[1], cast(3 as float))", "[3.0, 1.0]",
+ "FLOAT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1], cast(3 as bigint))", "[3, 1]",
+ "BIGINT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[2], cast(3 as double))", "[3.0, 2.0]",
+ "DOUBLE NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as float))", "[3.0, 1.0, 2.0]",
+ "FLOAT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[2, 1], cast(3 as double))", "[3.0, 2.0, 1.0]",
+ "DOUBLE NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as tinyint))", "[3, 1, 2]",
+ "INTEGER NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as bigint))", "[3, 1, 2]",
+ "BIGINT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array(1, 2), cast(null as double))", "[null, 1.0, 2.0]",
Review Comment:
use spark array is enough. we have a issue: https://issues.apache.org/jira/browse/CALCITE-6042
If we have a better solution, then we can use more clean way to support std array & spark array function.
The current way is a bit bloated/duplicated.
--
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-5976] Use explicit CAST if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1972932906
@caicancai I have created a commit based on your PR. you can pick and take a look.
https://github.com/chucheng92/calcite/tree/CALCITE-5976
--
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-5976] Function ArrayPrepend/ArrayAppend/ArrayInsert gives exception when inserted element type not equals array component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1980278527
@chucheng92 I've added you as Co-authored, thank you very much
--
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-5976] Function ARRAY_PREPEND/ARRAY_APPEND/ARRAY_INSERT gives exception when inserted element type not equals array component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1980306228
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [98.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [53.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1968662775
The code is very ugly, please do not review it, I will refactor it later.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1969268631
@mihaibudiu @chucheng92
If you have time, can you help me see if my approach is suitable? This PR has not been completed yet. I still need to test a few more samples in spark testing to ensure that this PR can be adapted as much as possible.
Of course, I also try to optimize the code as much as possible.
Thank you
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1964388685
@chucheng92 Hello, I'm not done yet, I want to make sure the way I'm fixing it is acceptable
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3705:
URL: https://github.com/apache/calcite/pull/3705#issuecomment-1972346304
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3705&resolved=false&inNewCodePeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_coverage&view=list)
 [64.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3705&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3705)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1506171264
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,34 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayAppendPrependConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
+ adjustTypeForMultisetConstructor3(
+ elementType, elementType, (SqlCallBinding) opBinding);
+ break;
+ default:
+ if (!componentType.getSqlTypeName().getName().equals("UNKNOWN")) {
Review Comment:
If it is an unknown type, no conversion is required.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1506940282
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ caseTypeForArrayFunctionConstructor(componentType, elementType, opBinding);
+ }
+ }
+
+ /**
+ * Implicit conversion of data types in array_append and array_prepend based on the types
+ * of componentType and elementType.
+ *
+ * @param componentType derived array component type
+ * @param elementType derived array elementType type
+ * @param opBinding description of call
+ */
+ public static void caseTypeForArrayFunctionConstructor(
+ RelDataType componentType, RelDataType elementType, SqlOperatorBinding opBinding) {
+ switch (elementType.getSqlTypeName().getName()) {
+ case "DOUBLE":
+ case "FLOAT":
+ case "BIGINT":
+ case "DECIMAL":
+ adjustTypeForMultisetConstructor3(
+ elementType, elementType, (SqlCallBinding) opBinding);
+ break;
+ default:
+ if (!componentType.getSqlTypeName().getName().equals("UNKNOWN")) {
Review Comment:
Are there other expressions? I only think of this representation
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507078891
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
Review Comment:
There is no need to implement this. For the form of `array_append/array_prepend/array_insert`, the array of the first arg has been cast. the derived type has been calculated (leastRestrictive of componentType & elementType). The only thing you need to do is to cast the element sqlnode into the derived type.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
Review Comment:
There is no need to implement this. For the form of `array_append/array_prepend/array_insert`, the array of the first arg has been cast. the derived type has been calculated (leastRestrictive about componentType & elementType). The only thing you need to do is to cast the element sqlnode into the derived type.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507078891
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1326,6 +1327,48 @@ public static void adjustTypeForArrayConstructor(
}
}
+ /**
+ * When the array_append and array_prepend element does not equal the array component type
+ * make explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
Review Comment:
There is no need to implement this. For the form of `array_append/array_prepend/array_insert`, the array of the first arg has been cast. the derived type has been calculated (leastRestrictive about componentType & elementType). The only thing you need to do is to cast the element sqlnode into the derived type.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507248799
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6538,6 +6586,54 @@ void checkRegexpExtract(SqlOperatorFixture f0, FunctionAlias functionAlias) {
"INTEGER is not comparable to BOOLEAN", false);
}
+ /** Test case for
+ * <a href="https://github.com/apache/calcite/pull/3705">[CALCITE-5976]
+ * Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert
+ * does not equal derived component type)</a>. */
+ @Test void testArrayPrependFuncByCast() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_PREPEND);
+ f0.checkFails("^array_prepend(array[1], 2)^",
+ "No match found for function signature ARRAY_PREPEND\\("
+ + "<INTEGER ARRAY>, <NUMERIC>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+ f.checkScalar("array_prepend(array[1], cast(3 as float))", "[3.0, 1.0]",
+ "FLOAT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1], cast(3 as bigint))", "[3, 1]",
+ "BIGINT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[2], cast(3 as double))", "[3.0, 2.0]",
+ "DOUBLE NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as float))", "[3.0, 1.0, 2.0]",
+ "FLOAT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[2, 1], cast(3 as double))", "[3.0, 2.0, 1.0]",
+ "DOUBLE NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as tinyint))", "[3, 1, 2]",
+ "INTEGER NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array[1, 2], cast(3 as bigint))", "[3, 1, 2]",
+ "BIGINT NOT NULL ARRAY NOT NULL");
+ f.checkScalar("array_prepend(array(1, 2), cast(null as double))", "[null, 1.0, 2.0]",
Review Comment:
thank you
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1506173209
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1398,6 +1471,28 @@ private static SqlNode castTo(SqlNode node, RelDataType type) {
SqlTypeUtil.convertTypeToSpec(type).withNullable(type.isNullable()));
}
+ /**
+ * Creates a CAST operation that converts the given Array in {@link SqlNode} to the specified {@link RelDataType}.
+ * This method uses the {@link SqlStdOperatorTable#CAST} operator to create a new {@link SqlCall}
+ * node representing a CAST operation. The original 'node' is cast to the desired 'type',
+ * preserving the nullability of the 'type'.
+ *
+ * @param node the {@link SqlNode} which is to be cast
+ * @param type the target {@link RelDataType} to which 'node' should be cast
+ * @return a new {@link SqlNode} representing the CAST operation
+ */
+ private static SqlNode arrayToCast(SqlNode node, RelDataType type) {
Review Comment:
In order to convert the types in the array to the same type as elementType
e.g:
array(1, 2) => array(1.0, 2.0)
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1506173209
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1398,6 +1471,28 @@ private static SqlNode castTo(SqlNode node, RelDataType type) {
SqlTypeUtil.convertTypeToSpec(type).withNullable(type.isNullable()));
}
+ /**
+ * Creates a CAST operation that converts the given Array in {@link SqlNode} to the specified {@link RelDataType}.
+ * This method uses the {@link SqlStdOperatorTable#CAST} operator to create a new {@link SqlCall}
+ * node representing a CAST operation. The original 'node' is cast to the desired 'type',
+ * preserving the nullability of the 'type'.
+ *
+ * @param node the {@link SqlNode} which is to be cast
+ * @param type the target {@link RelDataType} to which 'node' should be cast
+ * @return a new {@link SqlNode} representing the CAST operation
+ */
+ private static SqlNode arrayToCast(SqlNode node, RelDataType type) {
Review Comment:
In order to convert the types in the array to the same type as elementType
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507043922
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1381,6 +1424,58 @@ private static void adjustTypeForMultisetConstructor(
}
}
+ /**
+ * Adjusts the types for operands in a SqlCallBinding during the construction of a sql collection
+ * type such as array_append and array_prepend, but cannot change the data type in the array.
+ *
+ * @param evenType the {@link RelDataType} to which the operands at even positions should be cast
+ * @param oddType the {@link RelDataType} to which the operands at odd positions should be cast
+ * @param sqlCallBinding the {@link SqlCallBinding} containing the operands to be adjusted
+ */
+ private static void adjustTypeForMultisetConstructor2(
Review Comment:
I'm thinking about whether adjustTypeForMultisetConstructor2 and adjustTypeForMultisetConstructor3 can be combined into one function. At present, I don't have a good solution, so I separate the two functions.
--
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-5976] Use explicit casting if inserted element type in ArrayPrepend/ArrayAppend/ArrayInsert does not equal derived component type [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3705:
URL: https://github.com/apache/calcite/pull/3705#discussion_r1507043922
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##########
@@ -1381,6 +1424,58 @@ private static void adjustTypeForMultisetConstructor(
}
}
+ /**
+ * Adjusts the types for operands in a SqlCallBinding during the construction of a sql collection
+ * type such as array_append and array_prepend, but cannot change the data type in the array.
+ *
+ * @param evenType the {@link RelDataType} to which the operands at even positions should be cast
+ * @param oddType the {@link RelDataType} to which the operands at odd positions should be cast
+ * @param sqlCallBinding the {@link SqlCallBinding} containing the operands to be adjusted
+ */
+ private static void adjustTypeForMultisetConstructor2(
Review Comment:
I'm thinking about whether adjustTypeForMultisetConstructor2 and adjustTypeForMultisetConstructor3 can be combined into one function. At present, I don't have a good solution, so I separate the two functions.
--
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