You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "chucheng92 (via GitHub)" <gi...@apache.org> on 2023/10/06 07:34:31 UTC
[PR] [CALCITE-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
chucheng92 opened a new pull request, #3458:
URL: https://github.com/apache/calcite/pull/3458
https://issues.apache.org/jira/browse/CALCITE-6037
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350418693
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
Hi, @mihaibudiu. thanks for reviewing. good question.
Frankly speaking, `SqlFunctionCategory` is a very loose field now. In fact, there is no strict verification during lookup, so no test case covers it. However, it is currently widely used and cannot be deleted for subsequent expansion. We can refer to the comments of the current `lookupOperatorOverloads` and use it immediately when we need it. WDYT?
https://github.com/apache/calcite/blob/597b53dbb075ab72700d40f66db81d2ac9c182b8/core/src/main/java/org/apache/calcite/sql/util/ReflectiveSqlOperatorTable.java#L94
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3458:
URL: https://github.com/apache/calcite/pull/3458#issuecomment-1752272115
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3458)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349538846
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type).
more details pls see: ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
The string function category can return string & numeric values. And this function only can divide into String category.
Generally speaking, these functions(in this PR ) process XML text and conform to the definition of String category rather than Numeric.
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type).
more details pls see: ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
The string function category can return string & numeric values. And this function only can divide into String category.
Generally speaking, these functions(in this PR) process XML text and conform to the definition of String category rather than Numeric.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349539324
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1075,7 +1079,8 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC);
+ OperandTypes.SAME_VARIADIC,
+ SqlFunctionCategory.SYSTEM);
Review Comment:
current the function type of ARRAY is Numeric currently. Obviously not right. The closest type in calcite is System. it means built-in or system-in. we can see many other functions if not belongs to timedate/string/numeric will let it be System. more details please see ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
I believe the calcite System category equals ANSI-sql built-in category.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "LakeShen (via GitHub)" <gi...@apache.org>.
LakeShen commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350493820
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1075,7 +1079,8 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC);
+ OperandTypes.SAME_VARIADIC,
+ SqlFunctionCategory.SYSTEM);
Review Comment:
Get your point,I'm ok for that.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "LakeShen (via GitHub)" <gi...@apache.org>.
LakeShen commented on PR #3458:
URL: https://github.com/apache/calcite/pull/3458#issuecomment-1751738779
Hi @chucheng92 ,Thank you for opening this PR, left a couple of comments!
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on PR #3458:
URL: https://github.com/apache/calcite/pull/3458#issuecomment-1783777113
> For me, this is an improvement.
>
> If there is no objection, I will merge it.
thanks for approve + 1.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350418693
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
Hi, @mihaibudiu. thanks for reviewing. good point.
Frankly speaking, `SqlFunctionCategory` is a very loose field now. In fact, there is no strict verification during lookup, so no test cases cover it. However, it is currently widely used and cannot be deleted for subsequent expansion. We can refer to the comments of the current `lookupOperatorOverloads` and use it immediately when we need it. WDYT?
https://github.com/apache/calcite/blob/597b53dbb075ab72700d40f66db81d2ac9c182b8/core/src/main/java/org/apache/calcite/sql/util/ReflectiveSqlOperatorTable.java#L94
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349538846
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type), which is not clearly specified by calcite. So we need to take a comprehensive view. These functions actually process XML. In database systems such as MySql or Oracle, they are divided into XML functions. The closest type in calcite is String.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "LakeShen (via GitHub)" <gi...@apache.org>.
LakeShen commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349534541
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1075,7 +1079,8 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC);
+ OperandTypes.SAME_VARIADIC,
+ SqlFunctionCategory.SYSTEM);
Review Comment:
what is meaning of System function?And why array function belong to system function.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349538846
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type).
more details pls see: ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
The string function category can return numeric values.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349538846
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type).
more details pls see: ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
The string function category can return string & numeric values. And this function only can divide into String category.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349538846
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type).
more details pls see: ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
The string function category can return string & numeric values. Generally speaking, these functions(in this PR) process XML text and conform to the definition of String category rather than Numeric.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect [calcite]
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli merged PR #3458:
URL: https://github.com/apache/calcite/pull/3458
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349538846
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type), which is not clearly specified by calcite. So we need to take a comprehensive view. These functions actually process XML. In database systems such as MySql or Oracle, they are divided into XML functions. The closest type in calcite is String.
please see ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
The string function category can return numeric values.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349539324
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1075,7 +1079,8 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC);
+ OperandTypes.SAME_VARIADIC,
+ SqlFunctionCategory.SYSTEM);
Review Comment:
current the function type of ARRAY is Numeric currently. Obviously not right. The closest type in calcite is System. it means built-in or system-in. we can see many other functions if not belongs to timedate/string/numeric will let it be System.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349538846
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
@LakeShen I get your point. Usually we classify functions according to the operands (in some cases, they can also be determined according to the return value type).
more details pls see: ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
The string function category can return string & numeric values.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350418693
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
Hi, @mihaibudiu. thanks for reviewing. good point.
Frankly speaking, `SqlFunctionCategory` is a very loose field now. In fact, there is no strict verification during lookup, so no test cases cover it. However, it is currently widely used and cannot be deleted now (It is also for the sake of future scalability).
We can refer to the comments of the current `lookupOperatorOverloads`, it can immediately works if we need to use it. WDYT?
https://github.com/apache/calcite/blob/597b53dbb075ab72700d40f66db81d2ac9c182b8/core/src/main/java/org/apache/calcite/sql/util/ReflectiveSqlOperatorTable.java#L94
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349539324
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1075,7 +1079,8 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC);
+ OperandTypes.SAME_VARIADIC,
+ SqlFunctionCategory.SYSTEM);
Review Comment:
current the function type of ARRAY is Numeric currently. Obviously not right. The closest type in calcite is System. it means built-in or system-in. we can see many other functions if not belongs to timedate/string/numeric will let it be System. more details please see ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
It looks like that the calcite System category equals built-in category.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3458:
URL: https://github.com/apache/calcite/pull/3458#issuecomment-1770804752
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3458)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect [calcite]
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on PR #3458:
URL: https://github.com/apache/calcite/pull/3458#issuecomment-1783725412
For me, this is an improvement.
If there is no objection, I will merge 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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349539324
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1075,7 +1079,8 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC);
+ OperandTypes.SAME_VARIADIC,
+ SqlFunctionCategory.SYSTEM);
Review Comment:
current the function type of ARRAY is Numeric currently. Obviously not right. The closest type in calcite is System. it means built-in or system-in. we can see many other functions if not belongs to timedate/string/numeric will let it be System. more details please see ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
It looks like that the calcite System category equals ANSI-sql built-in category.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350418693
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
Hi, @mihaibudiu. thanks for reviewing. good point.
Frankly speaking, `SqlFunctionCategory` is a very loose field now. In fact, there is no strict verification during lookup, so no test cases cover it. However, it is currently widely used and cannot be deleted now (It is also for the sake of subsequent scalability).
We can refer to the comments of the current `lookupOperatorOverloads`, it can immediately works if we need to use it. WDYT?
https://github.com/apache/calcite/blob/597b53dbb075ab72700d40f66db81d2ac9c182b8/core/src/main/java/org/apache/calcite/sql/util/ReflectiveSqlOperatorTable.java#L94
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350418693
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
Hi, @mihaibudiu. thanks for reviewing. good point.
Frankly speaking, `SqlFunctionCategory` is a very loose field now. In fact, there is no strict verification during lookup, so no test case covers it. However, it is currently widely used and cannot be deleted for subsequent expansion. We can refer to the comments of the current `lookupOperatorOverloads` and use it immediately when we need it. WDYT?
https://github.com/apache/calcite/blob/597b53dbb075ab72700d40f66db81d2ac9c182b8/core/src/main/java/org/apache/calcite/sql/util/ReflectiveSqlOperatorTable.java#L94
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350418693
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
Hi, @mihaibudiu. thanks for reviewing. good question.
Frankly speaking, `SqlFunctionCategory` is a very loose field now. In fact, there is no strict verification during lookup, so no test case covers it. However, it is currently widely used and cannot be deleted for subsequent expansion. We can refer to the comments of the current `lookupOperatorOverloads` and use it immediately when we need it.
https://github.com/apache/calcite/blob/597b53dbb075ab72700d40f66db81d2ac9c182b8/core/src/main/java/org/apache/calcite/sql/util/ReflectiveSqlOperatorTable.java#L94
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "LakeShen (via GitHub)" <gi...@apache.org>.
LakeShen commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349533907
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
I find that EXTRACT_VALUE function would return the numeric value in some Scene.
I don't know whether it is appropriate to divide it into a String type function. I saw that it was divided into numeric function before.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "chucheng92 (via GitHub)" <gi...@apache.org>.
chucheng92 commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1349539324
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1075,7 +1079,8 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC);
+ OperandTypes.SAME_VARIADIC,
+ SqlFunctionCategory.SYSTEM);
Review Comment:
current the function type of ARRAY is Numeric currently. Obviously not right. The closest type in calcite is System. it means built-in or system-in. we can see many other functions if not belongs to timedate/string/numeric will let it be System. more details please see ANSI sql function category: https://www.oreilly.com/library/view/sql-in-a/9780596155322/ch04s04.html.
I believe the calcite System equals ANSI-sql built-in.
--
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-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT/EXISTSNODE is incorrect [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3458:
URL: https://github.com/apache/calcite/pull/3458#issuecomment-1750167828
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3458)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3458&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3458&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3458&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6037] The function category of ARRAY/EXTRACT_VALUE/XML_TRANSFORM/EXTRACT_XML/EXISTSNODE is incorrect (Spark/MySQL/Oracle Library) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #3458:
URL: https://github.com/apache/calcite/pull/3458#discussion_r1350375131
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -532,26 +532,30 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction EXTRACT_VALUE =
SqlBasicFunction.create("EXTRACTVALUE",
ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.STRING_STRING);
+ OperandTypes.STRING_STRING,
Review Comment:
I have a meta-question: what is this field used for?
It doesn't seem to affect any of the tests.
Clearly, the meaning of this field is ambiguous too - it's not clear what the right choice is in all cases.
Can we just remove it completely?
--
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