You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "xuzifu666 (via GitHub)" <gi...@apache.org> on 2026/06/23 11:31:49 UTC
[PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
xuzifu666 opened a new pull request, #5042:
URL: https://github.com/apache/calcite/pull/5042
jira: https://issues.apache.org/jira/browse/CALCITE-7615
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #5042:
URL: https://github.com/apache/calcite/pull/5042#issuecomment-4786987822
If you are happy with current changes, I would squash the commits~ @mihaibudiu
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5042:
URL: https://github.com/apache/calcite/pull/5042#issuecomment-4778934054
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3465002755
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9281,9 +9284,10 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
f1.checkType("map_concat(cast(null as map<varchar, int>), map['foo', 1])",
"(VARCHAR NOT NULL, INTEGER) MAP");
- f1.checkFails("^map_concat(map('foo', 1), null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ f1.checkNull("map_concat(map('foo', 1), null)");
Review Comment:
Can you also add a test like MAP_CONCAT(MAP_CONCAT(), MAP[1, 2]). I think this should work, and the result should be MAP<INT, INT>.
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1763,23 +1763,32 @@ private static RelDataType deriveTypeArraysZip(SqlOperatorBinding opBinding) {
OperandTypes.ARRAY.or(OperandTypes.ARRAY_BOOLEAN_LITERAL));
private static RelDataType deriveTypeMapConcat(SqlOperatorBinding opBinding) {
+ final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
if (opBinding.getOperandCount() == 0) {
- final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
- final RelDataType type = typeFactory.createSqlType(SqlTypeName.VARCHAR);
- requireNonNull(type, "type");
+ final RelDataType type = typeFactory.createSqlType(SqlTypeName.ANY);
return SqlTypeUtil.createMapType(typeFactory, type, type, true);
- } else {
- final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
- for (RelDataType operandType : operandTypes) {
- if (!SqlTypeUtil.isMap(operandType)) {
- throw opBinding.newError(
- RESOURCE.typesShouldAllBeMap(
- opBinding.getOperator().getName(),
- operandType.getFullTypeString()));
- }
+ }
+ final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
+ final List<RelDataType> mapTypes = new ArrayList<>();
+ boolean hasNull = false;
+ for (RelDataType operandType : operandTypes) {
+ if (operandType.getSqlTypeName() == SqlTypeName.NULL) {
+ hasNull = true;
+ } else if (SqlTypeUtil.isMap(operandType)) {
+ mapTypes.add(operandType);
+ } else {
+ throw opBinding.newError(
+ RESOURCE.typesShouldAllBeMap(
+ opBinding.getOperator().getName(),
+ operandType.getFullTypeString()));
}
- return requireNonNull(opBinding.getTypeFactory().leastRestrictive(operandTypes));
}
+ if (mapTypes.isEmpty() || hasNull) {
+ // All arguments are NULL literals, or at least one argument is a NULL literal;
Review Comment:
at least one null argument.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5042:
URL: https://github.com/apache/calcite/pull/5042#issuecomment-4786147180
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3464529015
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9244,16 +9244,19 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
// test only has one operand, but it is not map type.
f.checkFails("^map_concat(1)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'INTEGER NOT NULL'", false);
- f.checkFails("^map_concat(null)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL' was found", false);
+ // test operand is the NULL literal.
+ f.checkNull("map_concat(null)");
// test operands in same type family, but it is not map type.
f.checkFails("^map_concat(array[1], array[1])^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'INTEGER NOT NULL ARRAY NOT NULL'", false);
- f.checkFails("^map_concat(map['foo', 1], null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL ARRAY NOT NULL' was found", false);
+ // test map operand with NULL literal.
+ f.checkNull("map_concat(map['foo', 1], null)");
+ f.checkType("map_concat(map['foo', 1], null)",
Review Comment:
Why would the type change if you infer it to be NULL?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3464642244
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9281,9 +9284,10 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
f1.checkType("map_concat(cast(null as map<varchar, int>), map['foo', 1])",
"(VARCHAR NOT NULL, INTEGER) MAP");
- f1.checkFails("^map_concat(map('foo', 1), null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ f1.checkNull("map_concat(map('foo', 1), null)");
Review Comment:
To be honest, this is based on Spark, and I also find it odd. I changed the type here to ```ANY``` because I think it's more appropriate that way.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9244,16 +9244,19 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
// test only has one operand, but it is not map type.
f.checkFails("^map_concat(1)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'INTEGER NOT NULL'", false);
- f.checkFails("^map_concat(null)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL' was found", false);
+ // test operand is the NULL literal.
+ f.checkNull("map_concat(null)");
// test operands in same type family, but it is not map type.
f.checkFails("^map_concat(array[1], array[1])^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'INTEGER NOT NULL ARRAY NOT NULL'", false);
- f.checkFails("^map_concat(map['foo', 1], null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL ARRAY NOT NULL' was found", false);
+ // test map operand with NULL literal.
+ f.checkNull("map_concat(map['foo', 1], null)");
+ f.checkType("map_concat(map['foo', 1], null)",
Review Comment:
Upon closer examination, this should indeed be a NULL type; I have modified the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3462193882
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1763,23 +1763,35 @@ private static RelDataType deriveTypeArraysZip(SqlOperatorBinding opBinding) {
OperandTypes.ARRAY.or(OperandTypes.ARRAY_BOOLEAN_LITERAL));
private static RelDataType deriveTypeMapConcat(SqlOperatorBinding opBinding) {
+ final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
if (opBinding.getOperandCount() == 0) {
- final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
final RelDataType type = typeFactory.createSqlType(SqlTypeName.VARCHAR);
requireNonNull(type, "type");
return SqlTypeUtil.createMapType(typeFactory, type, type, true);
- } else {
- final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
- for (RelDataType operandType : operandTypes) {
- if (!SqlTypeUtil.isMap(operandType)) {
- throw opBinding.newError(
- RESOURCE.typesShouldAllBeMap(
- opBinding.getOperator().getName(),
- operandType.getFullTypeString()));
- }
+ }
+ final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
+ final List<RelDataType> mapTypes = new ArrayList<>();
+ boolean hasNull = false;
+ for (RelDataType operandType : operandTypes) {
+ if (operandType.getSqlTypeName() == SqlTypeName.NULL) {
+ hasNull = true;
+ } else if (SqlTypeUtil.isMap(operandType)) {
+ mapTypes.add(operandType);
+ } else {
+ throw opBinding.newError(
+ RESOURCE.typesShouldAllBeMap(
+ opBinding.getOperator().getName(),
+ operandType.getFullTypeString()));
}
- return requireNonNull(opBinding.getTypeFactory().leastRestrictive(operandTypes));
}
+ if (mapTypes.isEmpty()) {
Review Comment:
Is this legal?
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9244,16 +9244,19 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
// test only has one operand, but it is not map type.
f.checkFails("^map_concat(1)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'INTEGER NOT NULL'", false);
- f.checkFails("^map_concat(null)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL' was found", false);
+ // test operand is the NULL literal.
+ f.checkNull("map_concat(null)");
// test operands in same type family, but it is not map type.
f.checkFails("^map_concat(array[1], array[1])^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'INTEGER NOT NULL ARRAY NOT NULL'", false);
- f.checkFails("^map_concat(map['foo', 1], null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL ARRAY NOT NULL' was found", false);
+ // test map operand with NULL literal.
+ f.checkNull("map_concat(map['foo', 1], null)");
+ f.checkType("map_concat(map['foo', 1], null)",
Review Comment:
this type could actually be NULL too.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9281,9 +9284,10 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
f1.checkType("map_concat(cast(null as map<varchar, int>), map['foo', 1])",
"(VARCHAR NOT NULL, INTEGER) MAP");
- f1.checkFails("^map_concat(map('foo', 1), null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ f1.checkNull("map_concat(map('foo', 1), null)");
Review Comment:
Do you want to add a test with no arguments, if that's really supported?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3464332211
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9281,9 +9284,10 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
f1.checkType("map_concat(cast(null as map<varchar, int>), map['foo', 1])",
"(VARCHAR NOT NULL, INTEGER) MAP");
- f1.checkFails("^map_concat(map('foo', 1), null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ f1.checkNull("map_concat(map('foo', 1), null)");
Review Comment:
OK, I had added a test with no arguments, according to Spark behaivor this should return Map<StringType, StringType>: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L713
```
@transient override lazy val dataType: MapType = {
if (children.isEmpty) {
MapType(StringType, StringType) // default
} else {
super.dataType.asInstanceOf[MapType]
}
}
```
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1763,23 +1763,35 @@ private static RelDataType deriveTypeArraysZip(SqlOperatorBinding opBinding) {
OperandTypes.ARRAY.or(OperandTypes.ARRAY_BOOLEAN_LITERAL));
private static RelDataType deriveTypeMapConcat(SqlOperatorBinding opBinding) {
+ final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
if (opBinding.getOperandCount() == 0) {
- final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
final RelDataType type = typeFactory.createSqlType(SqlTypeName.VARCHAR);
requireNonNull(type, "type");
return SqlTypeUtil.createMapType(typeFactory, type, type, true);
- } else {
- final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
- for (RelDataType operandType : operandTypes) {
- if (!SqlTypeUtil.isMap(operandType)) {
- throw opBinding.newError(
- RESOURCE.typesShouldAllBeMap(
- opBinding.getOperator().getName(),
- operandType.getFullTypeString()));
- }
+ }
+ final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
+ final List<RelDataType> mapTypes = new ArrayList<>();
+ boolean hasNull = false;
+ for (RelDataType operandType : operandTypes) {
+ if (operandType.getSqlTypeName() == SqlTypeName.NULL) {
+ hasNull = true;
+ } else if (SqlTypeUtil.isMap(operandType)) {
+ mapTypes.add(operandType);
+ } else {
+ throw opBinding.newError(
+ RESOURCE.typesShouldAllBeMap(
+ opBinding.getOperator().getName(),
+ operandType.getFullTypeString()));
}
- return requireNonNull(opBinding.getTypeFactory().leastRestrictive(operandTypes));
}
+ if (mapTypes.isEmpty()) {
Review Comment:
Here I'm referencing Spark's behavior:
```
SELECT map_concat(map(1, 'a'), null);
-- result: NULL
SELECT map_concat(null, map(2, 'b'));
-- result: NULL
SELECT map_concat(map(1, 'a'), map(2, 'b'), null);
-- result: NULL
```
This should be reasonable according to Spark's standards.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9244,16 +9244,19 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
// test only has one operand, but it is not map type.
f.checkFails("^map_concat(1)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'INTEGER NOT NULL'", false);
- f.checkFails("^map_concat(null)^",
- "Function 'MAP_CONCAT' should all be of type map, but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL' was found", false);
+ // test operand is the NULL literal.
+ f.checkNull("map_concat(null)");
// test operands in same type family, but it is not map type.
f.checkFails("^map_concat(array[1], array[1])^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'INTEGER NOT NULL ARRAY NOT NULL'", false);
- f.checkFails("^map_concat(map['foo', 1], null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ "Arguments of function 'MAP_CONCAT' should all be of type MAP, "
+ + "but 'INTEGER NOT NULL ARRAY NOT NULL' was found", false);
+ // test map operand with NULL literal.
+ f.checkNull("map_concat(map['foo', 1], null)");
+ f.checkType("map_concat(map['foo', 1], null)",
Review Comment:
Yes, the main concern here is that the constant `map_concat(map['foo', 1], null)` might be collapsed into a NULL literal. This would cause the static type to change from MAP to NULL, making `checkType` unstable or even failing. Therefore, I've removed this test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5042:
URL: https://github.com/apache/calcite/pull/5042#issuecomment-4785711381
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5042:
URL: https://github.com/apache/calcite/pull/5042#issuecomment-4786165663
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5042:
URL: https://github.com/apache/calcite/pull/5042#issuecomment-4795201947
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3465150979
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1763,23 +1763,32 @@ private static RelDataType deriveTypeArraysZip(SqlOperatorBinding opBinding) {
OperandTypes.ARRAY.or(OperandTypes.ARRAY_BOOLEAN_LITERAL));
private static RelDataType deriveTypeMapConcat(SqlOperatorBinding opBinding) {
+ final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
if (opBinding.getOperandCount() == 0) {
- final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
- final RelDataType type = typeFactory.createSqlType(SqlTypeName.VARCHAR);
- requireNonNull(type, "type");
+ final RelDataType type = typeFactory.createSqlType(SqlTypeName.ANY);
return SqlTypeUtil.createMapType(typeFactory, type, type, true);
- } else {
- final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
- for (RelDataType operandType : operandTypes) {
- if (!SqlTypeUtil.isMap(operandType)) {
- throw opBinding.newError(
- RESOURCE.typesShouldAllBeMap(
- opBinding.getOperator().getName(),
- operandType.getFullTypeString()));
- }
+ }
+ final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
+ final List<RelDataType> mapTypes = new ArrayList<>();
+ boolean hasNull = false;
+ for (RelDataType operandType : operandTypes) {
+ if (operandType.getSqlTypeName() == SqlTypeName.NULL) {
+ hasNull = true;
+ } else if (SqlTypeUtil.isMap(operandType)) {
+ mapTypes.add(operandType);
+ } else {
+ throw opBinding.newError(
+ RESOURCE.typesShouldAllBeMap(
+ opBinding.getOperator().getName(),
+ operandType.getFullTypeString()));
}
- return requireNonNull(opBinding.getTypeFactory().leastRestrictive(operandTypes));
}
+ if (mapTypes.isEmpty() || hasNull) {
+ // All arguments are NULL literals, or at least one argument is a NULL literal;
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 merged PR #5042:
URL: https://github.com/apache/calcite/pull/5042
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3464531551
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9281,9 +9284,10 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
f1.checkType("map_concat(cast(null as map<varchar, int>), map['foo', 1])",
"(VARCHAR NOT NULL, INTEGER) MAP");
- f1.checkFails("^map_concat(map('foo', 1), null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ f1.checkNull("map_concat(map('foo', 1), null)");
Review Comment:
Maybe this should be documented. Frankly, it looks wrong to me, for empty array the type is inferred to be ANY ARRAY I think
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #5042:
URL: https://github.com/apache/calcite/pull/5042#discussion_r3465149896
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -9281,9 +9284,10 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
f1.checkType("map_concat(cast(null as map<varchar, int>), map['foo', 1])",
"(VARCHAR NOT NULL, INTEGER) MAP");
- f1.checkFails("^map_concat(map('foo', 1), null)^",
- "Function 'MAP_CONCAT' should all be of type map, "
- + "but it is 'NULL'", false);
+ f1.checkNull("map_concat(map('foo', 1), null)");
Review Comment:
OK, had added a test and result is as expected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7615] MAP_CONCAT does not accept NULL as an argument [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5042:
URL: https://github.com/apache/calcite/pull/5042#issuecomment-4786886117
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5042&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5042&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5042&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5042)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org