Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1785227201 > LGTM, sorry for the delay! Congrats on becoming a committer! thanks tanner. you helped me to a lot. :) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
tanclary merged PR #3459: URL: https://github.com/apache/calcite/pull/3459 -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1781758539 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [98.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1781572743 thank you all for patient reviewing! all comments are resolved. ci passed. @tanclary could you help to merge it? so we can add it in up-coming 1.36. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1781498518 @tanclary Thanks for the reminder. I checked sonar report and problems are roughly divided into three categories: 1. Assignment of index i in for-loop 2. The MAP of SqlFunctions does not specify a generic type 3. Reuse string constants I have modified 1, but 2 and 3 actually not applicable for us. 2: The methods of SqlFunctions are all used by reflection. It can be written as generics or not. However, the current collection types Map and List are expressed using raw, and they tend to be consistent with them. 3: is something like "CHAR(10) NOT NULL", there are multiple checkScalar, sonar wants us to define a variable and reuse it. In fact, it is not necessary. The current way of writing is more intuitive. If you agree with these, I will squash the commits. WDYT? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1780425104 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [15 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [98.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1780421895 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [15 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in `BuiltInMethod` to store all the methods.
it occupy 'MAP' name before, so I can't continue to use this name as our spark
map method name, so i changed it, then we can use 'MAP' as method name as you
suggested for consistency in naming. (I set 'MAP_FUNCTION' in `BuiltInMethod`
previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application/engine use java reflection to
call this field. But is this as expected?
I'm ok if we need to reset spark MAP method to 'MAP_FUNCTION' name in
`BuiltInMethod` and keep here unchanged. @tanclary what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372573530
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding
callBinding,
+final boolean throwOnFailure) {
+ final List argTypes =
+ SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+ // allows empty map
+ if (argTypes.size() == 0) {
+return true;
+ }
+ // the size of map arg types must be even.
+ if (argTypes.size() % 2 > 0) {
+throw
callBinding.newValidationError(RESOURCE.mapRequiresEvenArgCount());
+ }
+ final Pair<@Nullable RelDataType, @Nullable RelDataType> componentType =
+ getComponentTypes(
+ callBinding.getTypeFactory(), argTypes);
+ // check key type & value type
+ if (null == componentType.left || null == componentType.right) {
+if (throwOnFailure) {
+ throw
callBinding.newValidationError(RESOURCE.needSameTypeParameter());
+}
+return false;
+ }
+ return true;
+}
+
+/**
+ * Extract the key type and value type of arg types.
+ */
+private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+RelDataTypeFactory typeFactory,
+List argTypes) {
+ return Pair.of(
+ typeFactory.leastRestrictive(Util.quotientList(argTypes, 2, 0)),
Review Comment:
> Can we add UT for `typeFactory.leastRestrictive`?
>
> How about? `MAP('k1', 1, 'k2', 2.0, 'k3', '3')`
yes, added it. thanks for reviewing.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
JiajunBernoulli commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372495642
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding
callBinding,
+final boolean throwOnFailure) {
+ final List argTypes =
+ SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+ // allows empty map
+ if (argTypes.size() == 0) {
+return true;
+ }
+ // the size of map arg types must be even.
+ if (argTypes.size() % 2 > 0) {
+throw
callBinding.newValidationError(RESOURCE.mapRequiresEvenArgCount());
+ }
+ final Pair<@Nullable RelDataType, @Nullable RelDataType> componentType =
+ getComponentTypes(
+ callBinding.getTypeFactory(), argTypes);
+ // check key type & value type
+ if (null == componentType.left || null == componentType.right) {
+if (throwOnFailure) {
+ throw
callBinding.newValidationError(RESOURCE.needSameTypeParameter());
+}
+return false;
+ }
+ return true;
+}
+
+/**
+ * Extract the key type and value type of arg types.
+ */
+private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+RelDataTypeFactory typeFactory,
+List argTypes) {
+ return Pair.of(
+ typeFactory.leastRestrictive(Util.quotientList(argTypes, 2, 0)),
Review Comment:
Can we add UT for `typeFactory.leastRestrictive`?
How about?
`MAP('k1', 1, 'k2', 2.0, 'k3', '3')`
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in `BuiltInMethod` to store all the methods.
it occupy 'MAP' name before, so I can't continue to use this name as our spark
map method name, so i changed it, then we can use 'MAP' as method name as you
suggested for consistency in naming. (I set MAP_FUNCTION in `BuiltInMethod`
previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application/engine use java reflection to
call this field. But is this as expected?
I'm ok if we need to reset spark MAP method to 'MAP_FUNCTION' name in
`BuiltInMethod` and keep here unchanged. @tanclary what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in `BuiltInMethod` to store all the methods.
it occupy 'MAP' name before, so I can't continue to use this name as our spark
map method name, so i changed it, then we can use 'MAP' as method name as you
suggested for consistency in naming. (I set MAP_FUNCTION in `BuiltInMethod`
previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application/engine use java reflection to
call this field. But is this as expected?
I'm ok if we need to reset spark MAP function to 'MAP_FUNCTION' name in
`BuiltInMethod` and keep here unchanged. what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in `BuiltInMethod` to store all the
functions. it occupy 'MAP' name before, so I can't continue to use this name as
our spark map method name, so i changed it, then we can use 'MAP' as method
name as you suggested for consistency in naming. (I set MAP_FUNCTION in
`BuiltInMethod` previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application/engine use java reflection to
call this field. But is this as expected?
I'm ok if we need to reset spark MAP function to 'MAP_FUNCTION' name in
`BuiltInMethod` and keep here unchanged. what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in `BuiltInMethod` to store all the
functions. it occupy 'MAP' name before, so I can't continue to use this name as
our spark map method name, so i changed it, then we can use 'MAP' as method
name as you suggested for consistency in naming. (I set MAP_FUNCTION in
`BuiltInMethod` previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application/engine use java reflection to
call this field. But is this as expected?
or should i reset spark MAP function to 'MAP_FUNCTION' name in
`BuiltInMethod` and keep here unchanged? what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in `BuiltInMethod` to store all the
functions. it occupy 'MAP' name before, so I can't continue to use this name as
our spark map method name, so i changed it, then we can use 'MAP' as method
name as you suggested for consistency in naming. (I set MAP_FUNCTION in
`BuiltInMethod` previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application/engine use java reflection to
call this field. But is this as expected? i'm not sure of it.
or should i reset spark MAP function to 'MAP_FUNCTION' name in
`BuiltInMethod` and keep here unchanged? what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in BuiltInMethod to store all the functions.
it use 'MAP' name before, so I can't continue to use this name as our spark map
method name as you suggested. (I set MAP_FUNCTION previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application that reflects the use of this
field.
or should i reset spark MAP function to 'MAP_FUNCTION' name and keep here
unchanged? what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
because this is a static field in BuiltInMethod to store all the functions.
it use 'MAP' name before, so I can't continue to use this name as our spark map
method name as you suggested. (I set MAP_FUNCTION previously).
one disadvantage I can think of here is that it may cause compatibility
issues? If there is an upper-layer application that reflects the use of this
field.
or should i reset spark MAP function to 'MAP_FUNCTION' name? what do you
think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
tanclary commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1371982252
##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public RelNode translate(Queryable queryable) {
public RelNode translate(Expression expression) {
if (expression instanceof MethodCallExpression) {
final MethodCallExpression call = (MethodCallExpression) expression;
- BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+ BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);
Review Comment:
why call it `FUNCTIONS_MAPS`?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1779211043 hi, @tanclary Sorry to pin you, I have solved the comments. could you help to re-check 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1766408159 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [15 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1766388440 I have rebased to the main to use [CALCITE-5570](https://issues.apache.org/jira/browse/CALCITE-5570), it supports the form like 'cast(null as map)' -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358161626
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6703,11 +6704,53 @@ private static void checkIf(SqlOperatorFixture f) {
// test operands not in same type family.
f.checkFails("^map_concat(map[1, null], array[1])^",
"Parameters must be of the same type", false);
+
+// 2. check with map function, map(k, v ...)
+final SqlOperatorFixture f1 = fixture()
+.setFor(SqlLibraryOperators.MAP_CONCAT)
+.withLibrary(SqlLibrary.SPARK);
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2))", "{foo=1,
bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2), map('foo', 2))",
"{foo=2, bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(null, 1), map(null, 2))", "{null=2}",
+"(NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(1, 2), map(1, null))", "{1=null}",
+"(INTEGER NOT NULL, INTEGER) MAP NOT NULL");
+// test zero arg, but it should return empty map.
+f1.checkScalar("map_concat()", "{}",
+"(VARCHAR NOT NULL, VARCHAR NOT NULL) MAP");
+
+// test concat with empty map, in fact, spark will return MAP(CHAR,
INTEGER), because
+// it will add a cast 'cast(map() as map)' to convert UNKNOWN
type,
+// but currently calcite not support cast to map type.
+f1.checkScalar("map_concat(map('foo', 1), map())", "{foo=1}",
+"(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+
+// after calcite supports cast(null as map), cast(map() as
map)
+// it should add these tests.
+if (TODO) {
Review Comment:
I get your point. but I have noticed that CALCITE-5570 has supported cast to
map. So these cases can work now. I have removed the IF(TODO).
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358161626
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6703,11 +6704,53 @@ private static void checkIf(SqlOperatorFixture f) {
// test operands not in same type family.
f.checkFails("^map_concat(map[1, null], array[1])^",
"Parameters must be of the same type", false);
+
+// 2. check with map function, map(k, v ...)
+final SqlOperatorFixture f1 = fixture()
+.setFor(SqlLibraryOperators.MAP_CONCAT)
+.withLibrary(SqlLibrary.SPARK);
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2))", "{foo=1,
bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2), map('foo', 2))",
"{foo=2, bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(null, 1), map(null, 2))", "{null=2}",
+"(NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(1, 2), map(1, null))", "{1=null}",
+"(INTEGER NOT NULL, INTEGER) MAP NOT NULL");
+// test zero arg, but it should return empty map.
+f1.checkScalar("map_concat()", "{}",
+"(VARCHAR NOT NULL, VARCHAR NOT NULL) MAP");
+
+// test concat with empty map, in fact, spark will return MAP(CHAR,
INTEGER), because
+// it will add a cast 'cast(map() as map)' to convert UNKNOWN
type,
+// but currently calcite not support cast to map type.
+f1.checkScalar("map_concat(map('foo', 1), map())", "{foo=1}",
+"(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+
+// after calcite supports cast(null as map), cast(map() as
map)
+// it should add these tests.
+if (TODO) {
Review Comment:
I get your point. but the form such as `cast(map() as map)` may
not necessarily be a bug, and there is currently no JIRA to track it, so it is
kept as a TODO for the time being. WDYT?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358158316
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding
callBinding,
+final boolean throwOnFailure) {
+ final List argTypes =
+ SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+ // allows empty map
+ if (argTypes.size() == 0) {
+return true;
+ }
+ // the size of map arg types must be even.
+ if (argTypes.size() % 2 > 0) {
+throw
callBinding.newValidationError(RESOURCE.mapRequiresEvenArgCount());
+ }
+ final Pair<@Nullable RelDataType, @Nullable RelDataType> componentType =
+ getComponentTypes(
+ callBinding.getTypeFactory(), argTypes);
+ // check key type & value type
+ if (null == componentType.left || null == componentType.right) {
+if (throwOnFailure) {
+ throw
callBinding.newValidationError(RESOURCE.needSameTypeParameter());
+}
+return false;
+ }
+ return true;
+}
+
+/**
+ * Extract the key type and value type of arg types.
+ */
+private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+RelDataTypeFactory typeFactory,
+List argTypes) {
+ return Pair.of(
+ typeFactory.leastRestrictive(Util.quotientList(argTypes, 2, 0)),
Review Comment:
Util.quotientList(argTypes, 2, 0):
This extracts all elements at even indices from argTypes.
It represents the types of keys in the map as they are placed at even
positions
e.g. 0, 2, 4, etc. details please see Util.quotientList.
std MapValueConstructor has same logic. see:
https://github.com/apache/calcite/blob/5151168e9a9035595939c2ae0f21a06984229209/core/src/main/java/org/apache/calcite/sql/fun/SqlMapValueConstructor.java#L90
I've added some comments here for better readability.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358246934
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1082,6 +1084,38 @@ private static RelDataType
arrayReturnType(SqlOperatorBinding opBinding) {
SqlLibraryOperators::arrayReturnType,
OperandTypes.SAME_VARIADIC);
+ private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+Pair<@Nullable RelDataType, @Nullable RelDataType> type =
+getComponentTypes(
+opBinding.getTypeFactory(), opBinding.collectOperandTypes());
+return SqlTypeUtil.createMapType(
+opBinding.getTypeFactory(),
+requireNonNull(type.left, "inferred key type"),
+requireNonNull(type.right, "inferred value type"),
+false);
+ }
+
+ private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+ RelDataTypeFactory typeFactory,
+ List argTypes) {
+// special case, allows empty map
+if (argTypes.size() == 0) {
Review Comment:
yes, fixed
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358161626
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6703,11 +6704,53 @@ private static void checkIf(SqlOperatorFixture f) {
// test operands not in same type family.
f.checkFails("^map_concat(map[1, null], array[1])^",
"Parameters must be of the same type", false);
+
+// 2. check with map function, map(k, v ...)
+final SqlOperatorFixture f1 = fixture()
+.setFor(SqlLibraryOperators.MAP_CONCAT)
+.withLibrary(SqlLibrary.SPARK);
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2))", "{foo=1,
bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2), map('foo', 2))",
"{foo=2, bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(null, 1), map(null, 2))", "{null=2}",
+"(NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(1, 2), map(1, null))", "{1=null}",
+"(INTEGER NOT NULL, INTEGER) MAP NOT NULL");
+// test zero arg, but it should return empty map.
+f1.checkScalar("map_concat()", "{}",
+"(VARCHAR NOT NULL, VARCHAR NOT NULL) MAP");
+
+// test concat with empty map, in fact, spark will return MAP(CHAR,
INTEGER), because
+// it will add a cast 'cast(map() as map)' to convert UNKNOWN
type,
+// but currently calcite not support cast to map type.
+f1.checkScalar("map_concat(map('foo', 1), map())", "{foo=1}",
+"(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+
+// after calcite supports cast(null as map), cast(map() as
map)
+// it should add these tests.
+if (TODO) {
Review Comment:
I get your point. but the form such as `cast(map() as map)` may
not necessarily be a bug, and there is currently no JIRA to track it, calcite
is not ready to support it yet, and may not support it, so it is kept as a TODO
for the time being. WDYT?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1761405658 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1761402938 hi, @tanclary I have addressed all your comments except `instead of if (TODO) would it be better to add an enum to the Bug class?` this one. I have rebased the main accidentally, I hope it won’t cause any trouble to your review. If you have time, pls help me to review 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459: URL: https://github.com/apache/calcite/pull/3459#discussion_r1358166406 ## core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java: ## @@ -568,6 +569,9 @@ public static SqlOperandTypeChecker variadic( public static final SqlSingleOperandTypeChecker MAP_FROM_ENTRIES = new MapFromEntriesOperandTypeChecker(); + public static final SqlSingleOperandTypeChecker MAP_FUNCTION = Review Comment: fixed. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358166726
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding
callBinding,
+final boolean throwOnFailure) {
+ final List argTypes =
+ SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+ // allows empty map
+ if (argTypes.size() == 0) {
+return true;
+ }
+ // the size of map arg types must be even.
+ if (argTypes.size() % 2 > 0) {
Review Comment:
fixed.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358164236
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -873,6 +874,7 @@ Builder populate2() {
map.put(MAP_VALUE_CONSTRUCTOR, value);
map.put(ARRAY_VALUE_CONSTRUCTOR, value);
defineMethod(ARRAY, BuiltInMethod.ARRAYS_AS_LIST.method,
NullPolicy.NONE);
+ defineMethod(MAP, BuiltInMethod.MAP_FUNCTION.method, NullPolicy.NONE);
Review Comment:
In fact, there is a MAP object in BuiltInMethod, occupy this name. but yes,
if we rename that, we can use MAP. I have updated 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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358164736
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5251,6 +5251,17 @@ public static Map mapFromArrays(List keysArray, List
valuesArray) {
return map;
}
+ /** Support the MAP function. */
+ public static Map mapFunction(Object... args) {
Review Comment:
fixed.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358164236
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -873,6 +874,7 @@ Builder populate2() {
map.put(MAP_VALUE_CONSTRUCTOR, value);
map.put(ARRAY_VALUE_CONSTRUCTOR, value);
defineMethod(ARRAY, BuiltInMethod.ARRAYS_AS_LIST.method,
NullPolicy.NONE);
+ defineMethod(MAP, BuiltInMethod.MAP_FUNCTION.method, NullPolicy.NONE);
Review Comment:
In fact, there is a MAP object in BuiltInMethod, occupy this name. but yes,
if we rename that, we can use MAP. I have updated it. WDYT?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459: URL: https://github.com/apache/calcite/pull/3459#discussion_r1358162190 ## site/_docs/reference.md: ## @@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions: | b | TO_HEX(binary) | Converts *binary* into a hexadecimal varchar | b | FROM_HEX(varchar) | Converts a hexadecimal-encoded *varchar* into bytes | b o | LTRIM(string)| Returns *string* with all blanks removed from the start +| s | MAP(key, value [, key, value]*)| Returns a map with the given key/value pairs Review Comment: yes, fixed. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358161626
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6703,11 +6704,53 @@ private static void checkIf(SqlOperatorFixture f) {
// test operands not in same type family.
f.checkFails("^map_concat(map[1, null], array[1])^",
"Parameters must be of the same type", false);
+
+// 2. check with map function, map(k, v ...)
+final SqlOperatorFixture f1 = fixture()
+.setFor(SqlLibraryOperators.MAP_CONCAT)
+.withLibrary(SqlLibrary.SPARK);
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2))", "{foo=1,
bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map('foo', 1), map('bar', 2), map('foo', 2))",
"{foo=2, bar=2}",
+"(CHAR(3) NOT NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(null, 1), map(null, 2))", "{null=2}",
+"(NULL, INTEGER NOT NULL) MAP NOT NULL");
+f1.checkScalar("map_concat(map(1, 2), map(1, null))", "{1=null}",
+"(INTEGER NOT NULL, INTEGER) MAP NOT NULL");
+// test zero arg, but it should return empty map.
+f1.checkScalar("map_concat()", "{}",
+"(VARCHAR NOT NULL, VARCHAR NOT NULL) MAP");
+
+// test concat with empty map, in fact, spark will return MAP(CHAR,
INTEGER), because
+// it will add a cast 'cast(map() as map)' to convert UNKNOWN
type,
+// but currently calcite not support cast to map type.
+f1.checkScalar("map_concat(map('foo', 1), map())", "{foo=1}",
+"(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+
+// after calcite supports cast(null as map), cast(map() as
map)
+// it should add these tests.
+if (TODO) {
Review Comment:
I get your point. but the form such as `cast(map() as map)` may
not necessarily be a bug, and there is currently no JIRA to track it, calcite
is not ready to support it yet, and may not support it, so it is kept as a TODO
for the time being.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358157015
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
Review Comment:
The args of map are non-fixed, so we set to -1 here. then `operandCount`
can dynamically set according to the number of input args.
I've added some comments here for better readability.
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding
callBinding,
+final boolean throwOnFailure) {
+ final List argTypes =
+ SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+ // allows empty map
+ if (argTypes.size() == 0) {
+return true;
+ }
+ // the size of map arg types must be even.
+ if (argTypes.size() % 2 > 0) {
+throw
callBinding.newValidationError(RESOURCE.mapRequiresEvenArgCount());
+ }
+ final Pair<@Nullable RelDataType, @Nullable RelDataType> componentType =
+ getComponentTypes(
+ callBinding.getTypeFactory(), argTypes);
+ // check key type & value type
+ if (null == componentType.left || null == componentType.right) {
+if (throwOnFailure) {
+ throw
callBinding.newValidationError(RESOURCE.needSameTypeParameter());
+}
+return false;
+ }
+ return true;
+}
+
+/**
+ * Extract the key type and value type of arg types.
+ */
+private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+RelDataTypeFactory typeFactory,
+List argTypes) {
+ return Pair.of(
+ typeFactory.leastRestrictive(Util.quotientList(argTypes, 2, 0)),
Review Comment:
Util.quotientList(argTypes, 2, 0):
This extracts all elements at even indices from argTypes.
It represents the types of keys in the map as they are placed at even
positions
e.g. 0, 2, 4, etc. details please see Util.quotientList.
I've added some comments here for better readability.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358157015
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
Review Comment:
The args of map are non-fixed, so we set to -1 here. then `operandCount`
can dynamically set according to the number of input args. I've added some
comments for better readability.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1358155535
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5251,6 +5251,17 @@ public static Map mapFromArrays(List keysArray, List
valuesArray) {
return map;
}
+ /** Support the MAP function. */
+ public static Map mapFunction(Object... args) {
+final Map map = new LinkedHashMap<>();
+for (int i = 0; i < args.length; i++) {
+ Object key = args[i++];
Review Comment:
yes, updated.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
tanclary commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1357038522
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5251,6 +5251,17 @@ public static Map mapFromArrays(List keysArray, List
valuesArray) {
return map;
}
+ /** Support the MAP function. */
+ public static Map mapFunction(Object... args) {
Review Comment:
same comment as above
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -873,6 +874,7 @@ Builder populate2() {
map.put(MAP_VALUE_CONSTRUCTOR, value);
map.put(ARRAY_VALUE_CONSTRUCTOR, value);
defineMethod(ARRAY, BuiltInMethod.ARRAYS_AS_LIST.method,
NullPolicy.NONE);
+ defineMethod(MAP, BuiltInMethod.MAP_FUNCTION.method, NullPolicy.NONE);
Review Comment:
should we call it just MAP to be consistent with other functions?
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
Review Comment:
where does the -1 come from?
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding
callBinding,
+final boolean throwOnFailure) {
+ final List argTypes =
+ SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+ // allows empty map
+ if (argTypes.size() == 0) {
Review Comment:
`isEmpty()`
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -568,6 +569,9 @@ public static SqlOperandTypeChecker variadic(
public static final SqlSingleOperandTypeChecker MAP_FROM_ENTRIES =
new MapFromEntriesOperandTypeChecker();
+ public static final SqlSingleOperandTypeChecker MAP_FUNCTION =
Review Comment:
same naming comment as above
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5251,6 +5251,17 @@ public static Map mapFromArrays(List keysArray, List
valuesArray) {
return map;
}
+ /** Support the MAP function. */
+ public static Map mapFunction(Object... args) {
+final Map map = new LinkedHashMap<>();
+for (int i = 0; i < args.length; i++) {
+ Object key = args[i++];
Review Comment:
Could be helpful to add a comment explaining the odd elements are keys and
the even elements are values.
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1082,6 +1084,38 @@ private static RelDataType
arrayReturnType(SqlOperatorBinding opBinding) {
SqlLibraryOperators::arrayReturnType,
OperandTypes.SAME_VARIADIC);
+ private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+Pair<@Nullable RelDataType, @Nullable RelDataType> type =
+getComponentTypes(
+opBinding.getTypeFactory(), opBinding.collectOperandTypes());
+return SqlTypeUtil.createMapType(
+opBinding.getTypeFactory(),
+requireNonNull(type.left, "inferred key type"),
+requireNonNull(type.right, "inferred value type"),
+false);
+ }
+
+ private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+ RelDataTypeFactory typeFactory,
+ List argTypes) {
+// special case, allows empty map
+if (argTypes.size() == 0) {
Review Comment:
should we use `isEmpty()` instead?
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
}
}
+ /**
+ * Operand type-checking strategy for a MAP function, it allows empty map.
+ */
+ private static class MapFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+ super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding
callBinding,
+final boolean throwOnFailure) {
+ final List argTypes =
+ SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+ // allows empty map
+ if (argTypes.size() == 0) {
+return true;
+ }
+ // the size of map arg types must be even.
+ if (argTypes.size() % 2 > 0) {
Review Comment:
is the condition
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1757045249 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [18 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [10.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1752373444 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [20 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [10.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1752017470 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [18 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [10.3% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1752015129 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [18 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [10.3% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349687321
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6604,11 +6605,44 @@ private static void checkIf(SqlOperatorFixture f) {
// test operands not in same type family.
f.checkFails("^map_concat(map[1, null], array[1])^",
"Parameters must be of the same type", false);
+
+// 2. check with map function, map(k, v ...)
Review Comment:
updated, but currently calcite not support 'cast(map() as map)', so I have added the case in IF block until we support 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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646120
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -4886,14 +4886,21 @@ SqlNode MapConstructor() :
{
{ s = span(); }
(
-LOOKAHEAD(1)
-
-// by sub query "MAP (SELECT empno, deptno FROM emp)"
-e = LeafQueryOrExpr(ExprContext.ACCEPT_QUERY)
-
+(
+// empty map function call: "map()"
+LOOKAHEAD(2)
+ { args = SqlNodeList.EMPTY; }
+|
+args = ParenthesizedQueryOrCommaList(ExprContext.ACCEPT_ALL)
+)
{
-return SqlStdOperatorTable.MAP_QUERY.createCall(
-s.end(this), e);
+if (args.size() == 1 && args.get(0).isA(SqlKind.QUERY)) {
+// MAP query constructor e.g. "MAP (SELECT empno, deptno FROM
emps)"
+return SqlStdOperatorTable.MAP_QUERY.createCall(s.end(this),
args.get(0));
+} else {
+// MAP function e.g. "MAP(1, 2)" equivalent to standard
"MAP[1, 2]"
+return SqlLibraryOperators.MAP.createCall(s.end(this),
args.getList());
Review Comment:
I hope to retain the original parser logic here, because it will be detected
later on OperandTypeChecker. And here is similar with spark's array handling
and calcite array constructor. pls see:
https://github.com/apache/calcite/pull/3141/files
Another benefit is that we can use some more human-readable error messages
in predefined CalciteResource.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646120
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -4886,14 +4886,21 @@ SqlNode MapConstructor() :
{
{ s = span(); }
(
-LOOKAHEAD(1)
-
-// by sub query "MAP (SELECT empno, deptno FROM emp)"
-e = LeafQueryOrExpr(ExprContext.ACCEPT_QUERY)
-
+(
+// empty map function call: "map()"
+LOOKAHEAD(2)
+ { args = SqlNodeList.EMPTY; }
+|
+args = ParenthesizedQueryOrCommaList(ExprContext.ACCEPT_ALL)
+)
{
-return SqlStdOperatorTable.MAP_QUERY.createCall(
-s.end(this), e);
+if (args.size() == 1 && args.get(0).isA(SqlKind.QUERY)) {
+// MAP query constructor e.g. "MAP (SELECT empno, deptno FROM
emps)"
+return SqlStdOperatorTable.MAP_QUERY.createCall(s.end(this),
args.get(0));
+} else {
+// MAP function e.g. "MAP(1, 2)" equivalent to standard
"MAP[1, 2]"
+return SqlLibraryOperators.MAP.createCall(s.end(this),
args.getList());
Review Comment:
I hope to retain the original parser logic here, because it will be detected
later on OperandTypeChecker. And here is similar with Array handling and
calcite array constructor. pls see:
https://github.com/apache/calcite/pull/3141/files
Another benefit is that we can use some more human-readable error messages
in predefined CalciteResource.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459: URL: https://github.com/apache/calcite/pull/3459#discussion_r1349685796 ## site/_docs/reference.md: ## @@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions: | b | TO_HEX(binary) | Converts *binary* into a hexadecimal varchar | b | FROM_HEX(varchar) | Converts a hexadecimal-encoded *varchar* into bytes | b o | LTRIM(string)| Returns *string* with all blanks removed from the start +| s | MAP(key, value [, key, value]*)| Returns a map with the given key/value pairs Review Comment: Fixed. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646490
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1077,6 +1079,38 @@ private static RelDataType
arrayReturnType(SqlOperatorBinding opBinding) {
SqlLibraryOperators::arrayReturnType,
OperandTypes.SAME_VARIADIC);
+ private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+Pair<@Nullable RelDataType, @Nullable RelDataType> type =
+getComponentTypes(
+opBinding.getTypeFactory(), opBinding.collectOperandTypes());
+return SqlTypeUtil.createMapType(
+opBinding.getTypeFactory(),
+requireNonNull(type.left, "inferred key type"),
+requireNonNull(type.right, "inferred value type"),
+false);
+ }
+
+ private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+ RelDataTypeFactory typeFactory,
+ List argTypes) {
+// special case, allows empty map
+if (argTypes.size() == 0) {
+ return Pair.of(typeFactory.createUnknownType(),
typeFactory.createUnknownType());
Review Comment:
because Spark use VOID type when map is empty, the Spark VOID type are
equivalent in some degree with calcite UNKNOWN.
And it's similar with current spark ARRAY handling. pls see:
https://github.com/apache/calcite/pull/3141/files
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646490
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1077,6 +1079,38 @@ private static RelDataType
arrayReturnType(SqlOperatorBinding opBinding) {
SqlLibraryOperators::arrayReturnType,
OperandTypes.SAME_VARIADIC);
+ private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+Pair<@Nullable RelDataType, @Nullable RelDataType> type =
+getComponentTypes(
+opBinding.getTypeFactory(), opBinding.collectOperandTypes());
+return SqlTypeUtil.createMapType(
+opBinding.getTypeFactory(),
+requireNonNull(type.left, "inferred key type"),
+requireNonNull(type.right, "inferred value type"),
+false);
+ }
+
+ private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+ RelDataTypeFactory typeFactory,
+ List argTypes) {
+// special case, allows empty map
+if (argTypes.size() == 0) {
+ return Pair.of(typeFactory.createUnknownType(),
typeFactory.createUnknownType());
Review Comment:
because Spark use a NullType class(VOID type) when map is empty, the Spark
VOID type are equivalent in some degree with calcite UNKNOWN.
https://github.com/apache/spark/blob/24b82dfd6cfb9a658af615446be5423695830dd9/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala#L184C2-L184C2
And it's similar with current spark ARRAY handling. pls see:
https://github.com/apache/calcite/pull/3141/files
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646120
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -4886,14 +4886,21 @@ SqlNode MapConstructor() :
{
{ s = span(); }
(
-LOOKAHEAD(1)
-
-// by sub query "MAP (SELECT empno, deptno FROM emp)"
-e = LeafQueryOrExpr(ExprContext.ACCEPT_QUERY)
-
+(
+// empty map function call: "map()"
+LOOKAHEAD(2)
+ { args = SqlNodeList.EMPTY; }
+|
+args = ParenthesizedQueryOrCommaList(ExprContext.ACCEPT_ALL)
+)
{
-return SqlStdOperatorTable.MAP_QUERY.createCall(
-s.end(this), e);
+if (args.size() == 1 && args.get(0).isA(SqlKind.QUERY)) {
+// MAP query constructor e.g. "MAP (SELECT empno, deptno FROM
emps)"
+return SqlStdOperatorTable.MAP_QUERY.createCall(s.end(this),
args.get(0));
+} else {
+// MAP function e.g. "MAP(1, 2)" equivalent to standard
"MAP[1, 2]"
+return SqlLibraryOperators.MAP.createCall(s.end(this),
args.getList());
Review Comment:
I hope to retain the original parser logic here, because it will be detected
later on OperandTypeChecker. And here is similar with Array handling. pls see:
https://github.com/apache/calcite/pull/3141/files
And even calcite MapValueConstructor keep this checking in
OperandTypeChecker either.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646490
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1077,6 +1079,38 @@ private static RelDataType
arrayReturnType(SqlOperatorBinding opBinding) {
SqlLibraryOperators::arrayReturnType,
OperandTypes.SAME_VARIADIC);
+ private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+Pair<@Nullable RelDataType, @Nullable RelDataType> type =
+getComponentTypes(
+opBinding.getTypeFactory(), opBinding.collectOperandTypes());
+return SqlTypeUtil.createMapType(
+opBinding.getTypeFactory(),
+requireNonNull(type.left, "inferred key type"),
+requireNonNull(type.right, "inferred value type"),
+false);
+ }
+
+ private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+ RelDataTypeFactory typeFactory,
+ List argTypes) {
+// special case, allows empty map
+if (argTypes.size() == 0) {
+ return Pair.of(typeFactory.createUnknownType(),
typeFactory.createUnknownType());
Review Comment:
because Spark use a VOID type when map is empty, the Spark VOID type are
equivalent in some degree with calcite UNKNOWN.
And it's similar with current spark ARRAY handling. pls see:
https://github.com/apache/calcite/pull/3141/files
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1751953719 @macroguo-ghy hi, thanks for reviewing. Because we have implemented Spark's ARRAY function, their processing logic on parser and empty array/map is similar, if compare with 2 PRs to review may be more clearer. https://github.com/apache/calcite/pull/3141/files -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646490
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1077,6 +1079,38 @@ private static RelDataType
arrayReturnType(SqlOperatorBinding opBinding) {
SqlLibraryOperators::arrayReturnType,
OperandTypes.SAME_VARIADIC);
+ private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+Pair<@Nullable RelDataType, @Nullable RelDataType> type =
+getComponentTypes(
+opBinding.getTypeFactory(), opBinding.collectOperandTypes());
+return SqlTypeUtil.createMapType(
+opBinding.getTypeFactory(),
+requireNonNull(type.left, "inferred key type"),
+requireNonNull(type.right, "inferred value type"),
+false);
+ }
+
+ private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+ RelDataTypeFactory typeFactory,
+ List argTypes) {
+// special case, allows empty map
+if (argTypes.size() == 0) {
+ return Pair.of(typeFactory.createUnknownType(),
typeFactory.createUnknownType());
Review Comment:
because Spark use UNKNOWN type when it's empty, it's similar with current
spark ARRAY handling. pls see: https://github.com/apache/calcite/pull/3141/files
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646220
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6604,11 +6605,44 @@ private static void checkIf(SqlOperatorFixture f) {
// test operands not in same type family.
f.checkFails("^map_concat(map[1, null], array[1])^",
"Parameters must be of the same type", false);
+
+// 2. check with map function, map(k, v ...)
Review Comment:
yes, good point. I will fix 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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349646120
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -4886,14 +4886,21 @@ SqlNode MapConstructor() :
{
{ s = span(); }
(
-LOOKAHEAD(1)
-
-// by sub query "MAP (SELECT empno, deptno FROM emp)"
-e = LeafQueryOrExpr(ExprContext.ACCEPT_QUERY)
-
+(
+// empty map function call: "map()"
+LOOKAHEAD(2)
+ { args = SqlNodeList.EMPTY; }
+|
+args = ParenthesizedQueryOrCommaList(ExprContext.ACCEPT_ALL)
+)
{
-return SqlStdOperatorTable.MAP_QUERY.createCall(
-s.end(this), e);
+if (args.size() == 1 && args.get(0).isA(SqlKind.QUERY)) {
+// MAP query constructor e.g. "MAP (SELECT empno, deptno FROM
emps)"
+return SqlStdOperatorTable.MAP_QUERY.createCall(s.end(this),
args.get(0));
+} else {
+// MAP function e.g. "MAP(1, 2)" equivalent to standard
"MAP[1, 2]"
+return SqlLibraryOperators.MAP.createCall(s.end(this),
args.getList());
Review Comment:
I hope to retain the original parser logic here, because it will be detected
later on OperandTypeChecker. And here is similar with Array handling. pls see:
https://github.com/apache/calcite/pull/3141/files
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
chucheng92 commented on code in PR #3459: URL: https://github.com/apache/calcite/pull/3459#discussion_r1349645838 ## site/_docs/reference.md: ## @@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions: | b | TO_HEX(binary) | Converts *binary* into a hexadecimal varchar | b | FROM_HEX(varchar) | Converts a hexadecimal-encoded *varchar* into bytes | b o | LTRIM(string)| Returns *string* with all blanks removed from the start +| s | MAP(key, value [, key, value]*)| Returns a map with the given key/value pairs Review Comment: yes, good point. I will fix 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
macroguo-ghy commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1349635823
##
site/_docs/reference.md:
##
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names
for types and functions:
| b | TO_HEX(binary) | Converts *binary* into
a hexadecimal varchar
| b | FROM_HEX(varchar) | Converts a
hexadecimal-encoded *varchar* into bytes
| b o | LTRIM(string)| Returns *string* with
all blanks removed from the start
+| s | MAP(key, value [, key, value]*)| Returns a map with the
given key/value pairs
Review Comment:
Empty map is a legal functon. Please add `MAP()` to the documentation.
##
core/src/main/codegen/templates/Parser.jj:
##
@@ -4886,14 +4886,21 @@ SqlNode MapConstructor() :
{
{ s = span(); }
(
-LOOKAHEAD(1)
-
-// by sub query "MAP (SELECT empno, deptno FROM emp)"
-e = LeafQueryOrExpr(ExprContext.ACCEPT_QUERY)
-
+(
+// empty map function call: "map()"
+LOOKAHEAD(2)
+ { args = SqlNodeList.EMPTY; }
+|
+args = ParenthesizedQueryOrCommaList(ExprContext.ACCEPT_ALL)
+)
{
-return SqlStdOperatorTable.MAP_QUERY.createCall(
-s.end(this), e);
+if (args.size() == 1 && args.get(0).isA(SqlKind.QUERY)) {
+// MAP query constructor e.g. "MAP (SELECT empno, deptno FROM
emps)"
+return SqlStdOperatorTable.MAP_QUERY.createCall(s.end(this),
args.get(0));
+} else {
+// MAP function e.g. "MAP(1, 2)" equivalent to standard
"MAP[1, 2]"
+return SqlLibraryOperators.MAP.createCall(s.end(this),
args.getList());
Review Comment:
I think we can check if the size of args is an even number.
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1077,6 +1079,38 @@ private static RelDataType
arrayReturnType(SqlOperatorBinding opBinding) {
SqlLibraryOperators::arrayReturnType,
OperandTypes.SAME_VARIADIC);
+ private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+Pair<@Nullable RelDataType, @Nullable RelDataType> type =
+getComponentTypes(
+opBinding.getTypeFactory(), opBinding.collectOperandTypes());
+return SqlTypeUtil.createMapType(
+opBinding.getTypeFactory(),
+requireNonNull(type.left, "inferred key type"),
+requireNonNull(type.right, "inferred value type"),
+false);
+ }
+
+ private static Pair<@Nullable RelDataType, @Nullable RelDataType>
getComponentTypes(
+ RelDataTypeFactory typeFactory,
+ List argTypes) {
+// special case, allows empty map
+if (argTypes.size() == 0) {
+ return Pair.of(typeFactory.createUnknownType(),
typeFactory.createUnknownType());
Review Comment:
There is a subtle difference between `unknown` and `any`, can you please
explain why `unknown` is used?
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6604,11 +6605,44 @@ private static void checkIf(SqlOperatorFixture f) {
// test operands not in same type family.
f.checkFails("^map_concat(map[1, null], array[1])^",
"Parameters must be of the same type", false);
+
+// 2. check with map function, map(k, v ...)
Review Comment:
Can you please add some tests with `map()`, like `map_concat(map('foo', 1),
map())`, `map_keys(map())` or `cast(map() as MAP)`.
Because the type of `map()` is `MAP`, I am not sure if
unknown component type will cause an error.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1751917666 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [17 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [97.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [11.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1751910686 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [92.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [11.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3459: URL: https://github.com/apache/calcite/pull/3459#issuecomment-1751731258 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3459) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3459=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3459=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [92.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=list) [10.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3459=new_duplicated_lines_density=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
