You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@calcite.apache.org by "liuyongvs (via GitHub)" <gi...@apache.org> on 2023/06/02 11:23:57 UTC

[GitHub] [calcite] liuyongvs opened a new pull request, #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

liuyongvs opened a new pull request, #3238:
URL: https://github.com/apache/calcite/pull/3238

    STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]])
   
   Returns a map after splitting the string into key/value pairs using delimiters. Default delimiters are ',' for stringDelimiter and ':' for keyValueDelimiter Both pairDelim and keyValueDelim are treated as regular expressions.
   
   ```
   Examples:
   
   > SELECT str_to_map('a:1,b:2,c:3', ',', ':');
   
   - {"a":"1","b":"2","c":"3"}
   
   > SELECT str_to_map('a');
    {"a":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

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241087753


##########
core/src/main/java/org/apache/calcite/runtime/CalciteResource.java:
##########
@@ -897,6 +897,9 @@ ExInst<CalciteException> illegalArgumentForTableFunctionCall(String a0,
   @BaseMessage("Substring error: negative substring length not allowed")
   ExInst<CalciteException> illegalNegativeSubstringLength();
 
+  @BaseMessage("Illegal arguments: The length of the keys array {0,number,#} is not equal to the length of the values array {1,number,#} in MAP_FROM_ARRAYS function")

Review Comment:
   
   @NobiGo 
   
   1) this info is more  concrete, it will make user know what is the problem?
   2) the error info aligns with flink. because i think it is better than spark's info.
   
   ```
   spark error info
     "_LEGACY_ERROR_TEMP_2128" : {
       "message" : [
         "The key array and value array of MapData must have the same length."
       ]
   ```
   
   ```
   fink error info
           if (keysArray.size() != valuesArray.size()) {
               throw new FlinkRuntimeException(
                       "Invalid function MAP_FROM_ARRAYS call:\n"
                               + "The length of the keys array "
                               + keysArray.size()
                               + " is not equal to the length of the values array "
                               + valuesArray.size());
   
   
   https://github.com/apache/flink/blob/65217c35e7c8f19acb1c7761d2a513ec9c0d316d/flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/MapFromArraysFunction.java#L30
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241090465


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5688,6 +5688,79 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("^map_from_arrays(2, array[1, 2])^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER>,"
+            + " <INTEGER ARRAY>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(^array[1, '1', true]^, array['a', 'b', 'c'])",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array['a', 'b', 'c'], ^array[1, '1', true]^)",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }
+
+  /** Tests {@code STR_TO_MAP} function from Spark. */
+  @Test void testStrToMapFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.STR_TO_MAP);
+    f0.checkFails("^str_to_map('a=1,b=2', ',', '=')^",
+        "No match found for function signature STR_TO_MAP\\("
+            + "<CHARACTER>, <CHARACTER>, <CHARACTER>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("str_to_map('a=1,b=2', ',', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2', ',')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a=1&b=2', '&', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k#2%v#3', '%', '#')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1&b:2', '&')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k:2%v:3', '%')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a')", "{a=null}",
+        "(CHAR(1) NOT NULL, CHAR(1) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a,b,c')", "{a=null, b=null, c=null}",
+        "(CHAR(5) NOT NULL, CHAR(5) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a-b--c', '--')", "{a-b=null, c=null}",
+        "(CHAR(6) NOT NULL, CHAR(6) NOT NULL) MAP NOT NULL");
+    f.checkType("str_to_map(cast(null as varchar))",
+        "(VARCHAR, VARCHAR) MAP");
+    f.checkNull("str_to_map(cast(null as varchar))");

Review Comment:
   @NobiGo added the unit tests 
   
   ```
       f.checkNull("str_to_map(null, ',', ':')");
       f.checkNull("str_to_map('a:1,b:2,c:3', null, ':')");
       f.checkNull("str_to_map('a:1,b:2,c:3', ',',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

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1575639833

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.1%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1578980614

   hi @tanclary do you have other suggestions, if not , i will squash the commits. thanks so much!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1577872882

   hi @tanclary thanks for your review and your several valuable suggestion +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1579635937

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.2%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] MasseGuillaume commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "MasseGuillaume (via GitHub)" <gi...@apache.org>.

MasseGuillaume commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1222041507


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",

Review Comment:
   I would check both ways:`f.checkFails("^map_from_arrays(2, array[1, 2])^",`



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }
+
+  /** Tests {@code STR_TO_MAP} function from Spark. */
+  @Test void testStrToMapFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.STR_TO_MAP);
+    f0.checkFails("^str_to_map('a=1,b=2', ',', '=')^",
+        "No match found for function signature STR_TO_MAP\\("
+            + "<CHARACTER>, <CHARACTER>, <CHARACTER>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("str_to_map('a=1,b=2', ',', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2', ',')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a=1&b=2', '&', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k#2%v#3', '%', '#')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1&b:2', '&')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k:2%v:3', '%')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a')", "{a=null}",
+        "(CHAR(1) NOT NULL, CHAR(1) NOT NULL) MAP NOT NULL");
+    f.checkType("str_to_map(cast(null as varchar))",
+        "(VARCHAR, VARCHAR) MAP");
+    f.checkNull("str_to_map(cast(null as varchar))");
+  }

Review Comment:
   ```
   spark.sql("""select str_to_map("a,b,c")""").show(false)
   +---------------------------------+
   |str_to_map(a,b,c, ,, :)          |
   +---------------------------------+
   |{a -> null, b -> null, c -> null}|
   +---------------------------------+
   
   spark.sql("""select str_to_map("a-b--c", "--")""").show(false)
   +-------------------------+
   |str_to_map(a-b--c, --, :)|
   +-------------------------+
   |{a-b -> null, c -> null} |
   +-------------------------+
   ```



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }

Review Comment:
   ```
   spark.sql("""select map_from_arrays(array(null), array(1))""").show
   java.lang.RuntimeException: Cannot use null as map key!
   
   spark.sql("""select map_from_arrays(array("a"), null)""").show
   +-------------------------------+
   |map_from_arrays(array(a), NULL)|
   +-------------------------------+
   |                           null|
   +-------------------------------+
   ```



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",

Review Comment:
   ```
   spark.sql("""select map_from_arrays(array(1, "1", true), array("a", "b", "c"))""").show
   org.apache.spark.sql.AnalysisException: cannot resolve 'array(1, '1', true)' due to data type mismatch: input to function array should all be the same type, but it's [int, string, boolean]; line 1 pos 23;
   
   spark.sql("""select map_from_arrays(array("a", "b", "c"), array(1, "1", true))""").show
   org.apache.spark.sql.AnalysisException: cannot resolve 'array(1, '1', true)' due to data type mismatch: input to function array should all be the same type, but it's [int, string, boolean]; line 1 pos 45;
   ```



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");

Review Comment:
   ```
   spark.sql("""select map_from_arrays(array(1, 1, null), array('foo', 'bar', 'name'))""").show
   java.lang.RuntimeException: Duplicate map key 1 was found, please check the input data. If you want to remove the duplicated keys, you can set spark.sql.mapKeyDedupPolicy to LAST_WIN so that the key inserted at last takes precedence.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1603701460

   hi @tanclary thanks for your review, fix the nit.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1577700324

   Hi @tanclary do you also have time to review this pr? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218856807


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3092,6 +3096,36 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      String defaultStringDelimiter = ",";
+      String defaultKeyValueDelimiter = ":";
+      if (argValueList.size() == 1) {
+        return Expressions.call(
+            BuiltInMethod.STR_TO_MAP.method,
+            argValueList.get(0),
+            Expressions.constant(defaultStringDelimiter),
+            Expressions.constant(defaultKeyValueDelimiter));
+      } else if (argValueList.size() == 2) {
+        return Expressions.call(
+          BuiltInMethod.STR_TO_MAP.method,
+              argValueList.get(0),
+              argValueList.get(1),
+              Expressions.constant(defaultKeyValueDelimiter));

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

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1215554451


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3917,6 +3918,38 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {
+      throw new IllegalArgumentException(
+          "Invalid function MAP_FROM_ARRAYS call:\n"
+              + "The length of the keys array "
+              + keysArray.size()
+              + " is not equal to the length of the values array "
+              + valuesArray.size());
+    }
+    final Map map = new LinkedHashMap<>();
+    for (int i = 0; i < keysArray.size(); i++) {
+      map.put(keysArray.get(i), valuesArray.get(i));
+    }
+    return map;
+  }
+
+  /** Support the STR_TO_MAP function. */
+  public static Map strToMap(String string, String stringDelimiter, String keyValueDelimiter) {
+    final Map map = new LinkedHashMap();
+    final String[] keyValues = string.split(stringDelimiter, -1);

Review Comment:
   the limit should be -1, which aligns with spark.
   what is the difference with limit -1, you can se here
   https://stackabuse.com/how-to-split-a-string-in-java/



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1575636104

   hi @tanclary @JiajunBernoulli @MasseGuillaume do you have time to help review ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218855816


##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -536,6 +536,18 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference TO_MAP =
       ARG0.andThen(SqlTypeTransforms.TO_MAP);
 
+  /**
+   * Returns a MAP type.
+   *
+   * <p>For example, given {@code STRING}, returns
+   * {@code (STRING, STRING) MAP}.
+   */
+  public static final SqlReturnTypeInference IDENTITY_TO_MAP =
+      ARG0.andThen(SqlTypeTransforms.IDENTITY_TO_MAP);
+
+  public static final SqlReturnTypeInference IDENTITY_TO_MAP_NULLABLE =

Review Comment:
   added



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] tanclary commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.

tanclary commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1219924761


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3119,6 +3125,37 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      switch (call.getOperands().size()) {
+      case 1:
+        return Expressions.call(

Review Comment:
   nit: I'm still wondering if the return can be extracted out of the switch statement. You notice that in each case it's "return Expressions.call(...)". 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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1220571865


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",

Review Comment:
   @tanclary thanks for your quick response, thank you so much and we only should test the logical of map_from_arrays. becuase array[cast(1 as varchar), 'hello'] should only test for ARRAY_VALUE_CONSTRUCTOR.
   and the result is not difference with my test for array['foo', 'bar']. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1220558806


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3119,6 +3125,37 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      switch (call.getOperands().size()) {
+      case 1:
+        return Expressions.call(

Review Comment:
   However, personally, I think the difference is not significant, and many do the same 
   for example ItemImplementor/LogImplementor/DatetimeArithmeticImplementor/...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241087753


##########
core/src/main/java/org/apache/calcite/runtime/CalciteResource.java:
##########
@@ -897,6 +897,9 @@ ExInst<CalciteException> illegalArgumentForTableFunctionCall(String a0,
   @BaseMessage("Substring error: negative substring length not allowed")
   ExInst<CalciteException> illegalNegativeSubstringLength();
 
+  @BaseMessage("Illegal arguments: The length of the keys array {0,number,#} is not equal to the length of the values array {1,number,#} in MAP_FROM_ARRAYS function")

Review Comment:
   1) this info is more  concrete, it will make user know what is the problem?
   2) the error info aligns with flink. because i think it is better than spark's info.
   
   ```
   spark error info
     "_LEGACY_ERROR_TEMP_2128" : {
       "message" : [
         "The key array and value array of MapData must have the same length."
       ]
   ```
   
   ```
   fink error info
           if (keysArray.size() != valuesArray.size()) {
               throw new FlinkRuntimeException(
                       "Invalid function MAP_FROM_ARRAYS call:\n"
                               + "The length of the keys array "
                               + keysArray.size()
                               + " is not equal to the length of the values array "
                               + valuesArray.size());
   
   
   https://github.com/apache/flink/blob/65217c35e7c8f19acb1c7761d2a513ec9c0d316d/flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/MapFromArraysFunction.java#L30
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1573868243

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.1%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] tanclary commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.

tanclary commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1238724033


##########
site/_docs/reference.md:
##########
@@ -2737,6 +2737,8 @@ BigQuery's type system uses confusingly different names for types and functions:
 | s | MAP_ENTRIES(map)                               | Returns the entries of the *map* as an array, the order of the entries is not defined
 | s | MAP_KEYS(map)                                  | Returns the keys of the *map* as an array, the order of the entries is not defined
 | s | MAP_VALUES(map)                                | Returns the values of the *map* as an array, the order of the entries is not defined
+| s | MAP_FROM_ARRAYS(array1, array2)                | Returns a map created from an *array1* and *array2*. Note that the lengths of two arrays should be the same
+| s | STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]]) | Returns a map after splitting the *string* into key/value pairs using delimiters. Default delimiters are ',' for *stringDelimiter* and ':' for *keyValueDelimiter*

Review Comment:
   In the example you provided, there are spaces between the words and the brackets (rep and [ has a space in between) we should probably stay consistent w/ that



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1581923467

   because both of you @tanclary @MasseGuillaume says add array with different element type, it will throw exception.
   i add
   
   ```
      f.checkFails("map_from_arrays(^array[1, '1', true]^, array['a', 'b', 'c'])",
           "Parameters must be of the same type",
           false);
       f.checkFails("map_from_arrays(array['a', 'b', 'c'], ^array[1, '1', true]^)",
           "Parameters must be of the same type",
           false);
   ```
   
   and from my side, it should be test in array_value_constructor function instead of this. i also add test in array_value_constructor
   ```
       f.checkFails("^array[1, '1', true]^", "Parameters must be of the same type", false);
   
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] tanclary commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.

tanclary commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1219827354


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -965,6 +965,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType deriveTypeMapFromArrays(SqlOperatorBinding opBinding) {
+    final RelDataType keysArrayType = opBinding.collectOperandTypes().get(0);

Review Comment:
   can you do opBinding.getOperandType? if so, that's a little cleaner 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

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1215556292


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3917,6 +3918,38 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {
+      throw new IllegalArgumentException(

Review Comment:
   the length check is need, and i add a negative test for it 
   ```
       f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
           "Invalid function MAP_FROM_ARRAYS call:\n"
               + "The length of the keys array 2 is not equal to the length of the values array 1",
           true);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] tanclary commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.

tanclary commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218724377


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3092,6 +3096,36 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      String defaultStringDelimiter = ",";
+      String defaultKeyValueDelimiter = ":";
+      if (argValueList.size() == 1) {

Review Comment:
   i personally find switch (operand count) to look better here but it's very subjective i suppose.



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3092,6 +3096,36 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      String defaultStringDelimiter = ",";
+      String defaultKeyValueDelimiter = ":";
+      if (argValueList.size() == 1) {
+        return Expressions.call(
+            BuiltInMethod.STR_TO_MAP.method,
+            argValueList.get(0),
+            Expressions.constant(defaultStringDelimiter),
+            Expressions.constant(defaultKeyValueDelimiter));
+      } else if (argValueList.size() == 2) {
+        return Expressions.call(
+          BuiltInMethod.STR_TO_MAP.method,
+              argValueList.get(0),
+              argValueList.get(1),
+              Expressions.constant(defaultKeyValueDelimiter));

Review Comment:
   if you only use the default Strings for Expressions.constant, you could just initialize them as expressions instead?



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3917,6 +3918,38 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */

Review Comment:
   is this different/unrelated to STR_TO_MAP? if so, the commit message etc should be updated to reflect everything the PR includes.



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3092,6 +3096,36 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      String defaultStringDelimiter = ",";

Review Comment:
   is there a way to use the "SPLIT" implementation offered by Bigquery here? Some of this looks very similar.



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -944,6 +944,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+    final RelDataType keysArrayType = opBinding.collectOperandTypes().get(0);
+    final RelDataType valuesArrayType = opBinding.collectOperandTypes().get(1);
+    final boolean nullable = keysArrayType.isNullable() || valuesArrayType.isNullable();
+    return SqlTypeUtil.createMapType(
+        opBinding.getTypeFactory(),
+        requireNonNull(keysArrayType.getComponentType(), "inferred key type"),
+        requireNonNull(valuesArrayType.getComponentType(), "inferred value type"),
+        nullable);
+  }
+
+  /** The "MAP_FROM_ARRAYS(keysArray, valuesArray)" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlFunction MAP_FROM_ARRAYS =

Review Comment:
   just reiterating that if this PR includes multiple functions, the JIRA case/commit message should reflect that



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -944,6 +944,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {

Review Comment:
   I am not sure this belongs here. Maybe somewhere like ReturnTypes.java would be more appropriate.



##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -536,6 +536,18 @@ public static SqlCall stripSeparator(SqlCall call) {
   public static final SqlReturnTypeInference TO_MAP =
       ARG0.andThen(SqlTypeTransforms.TO_MAP);
 
+  /**
+   * Returns a MAP type.
+   *
+   * <p>For example, given {@code STRING}, returns
+   * {@code (STRING, STRING) MAP}.
+   */
+  public static final SqlReturnTypeInference IDENTITY_TO_MAP =
+      ARG0.andThen(SqlTypeTransforms.IDENTITY_TO_MAP);
+
+  public static final SqlReturnTypeInference IDENTITY_TO_MAP_NULLABLE =

Review Comment:
   nit: many *_NULLABLE return types have a JAVADOC



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3917,6 +3918,38 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {
+      throw new IllegalArgumentException(

Review Comment:
   can this exception be added as a constant to Resources like the other errors thrown from this class?



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3092,6 +3096,36 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      String defaultStringDelimiter = ",";
+      String defaultKeyValueDelimiter = ":";
+      if (argValueList.size() == 1) {
+        return Expressions.call(
+            BuiltInMethod.STR_TO_MAP.method,
+            argValueList.get(0),
+            Expressions.constant(defaultStringDelimiter),
+            Expressions.constant(defaultKeyValueDelimiter));
+      } else if (argValueList.size() == 2) {
+        return Expressions.call(
+          BuiltInMethod.STR_TO_MAP.method,
+              argValueList.get(0),
+              argValueList.get(1),
+              Expressions.constant(defaultKeyValueDelimiter));

Review Comment:
   i am wondering whether the return statement could be extracted out, and the if-else-if (or switch, if you choose to use it) could just be used for assigning operands and whatnot. Right now it looks like a lot of duplicate code.



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -944,6 +944,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+    final RelDataType keysArrayType = opBinding.collectOperandTypes().get(0);
+    final RelDataType valuesArrayType = opBinding.collectOperandTypes().get(1);
+    final boolean nullable = keysArrayType.isNullable() || valuesArrayType.isNullable();
+    return SqlTypeUtil.createMapType(
+        opBinding.getTypeFactory(),
+        requireNonNull(keysArrayType.getComponentType(), "inferred key type"),
+        requireNonNull(valuesArrayType.getComponentType(), "inferred value type"),
+        nullable);
+  }
+
+  /** The "MAP_FROM_ARRAYS(keysArray, valuesArray)" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlFunction MAP_FROM_ARRAYS =
+      SqlBasicFunction.create(SqlKind.MAP_FROM_ARRAYS,
+          SqlLibraryOperators::mapReturnType,
+          OperandTypes.ARRAY_ARRAY);
+
+  /** The "STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]])" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlFunction STR_TO_MAP =
+      SqlBasicFunction.create(SqlKind.STR_TO_MAP,

Review Comment:
   I think there is an entry in OperandTypes called something like "STRING_OPTIONAL_STRING" or something along those lines, either use that or add an entry to fit this case, so future functions can use it easily if need be.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1220568149


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",

Review Comment:
   @tanclary "do all elements of the array have to be of the same type"  yeap, the args should be find leastRestrictive, otherwise it will throw exception like "SqlValidatorException: Parameters must be of the same type"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1239335343


##########
site/_docs/reference.md:
##########
@@ -2737,6 +2737,8 @@ BigQuery's type system uses confusingly different names for types and functions:
 | s | MAP_ENTRIES(map)                               | Returns the entries of the *map* as an array, the order of the entries is not defined
 | s | MAP_KEYS(map)                                  | Returns the keys of the *map* as an array, the order of the entries is not defined
 | s | MAP_VALUES(map)                                | Returns the values of the *map* as an array, the order of the entries is not defined
+| s | MAP_FROM_ARRAYS(array1, array2)                | Returns a map created from an *array1* and *array2*. Note that the lengths of two arrays should be the same
+| s | STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]]) | Returns a map after splitting the *string* into key/value pairs using delimiters. Default delimiters are ',' for *stringDelimiter* and ':' for *keyValueDelimiter*

Review Comment:
   fixed @tanclary 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241086232


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4003,6 +4004,33 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {

Review Comment:
   @NobiGo yeap. the calcite supports NullPolicy(6 policy), which will insert code in runtime, spark  using NullIntolerant(only supports one policy), which is equal to NullPolicy.ANY.
   
   for example：
   if the NullPolicy.ANY, the calcite will return null  If any of the arguments are null. so the user code doesn't need check whether the input is null
   
   ```
   defineMethod(MAP_FROM_ARRAYS, BuiltInMethod.MAP_FROM_ARRAYS.method, NullPolicy.ANY);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1573597402

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [15 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![96.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '96.2%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [96.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218855652


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -944,6 +944,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+    final RelDataType keysArrayType = opBinding.collectOperandTypes().get(0);
+    final RelDataType valuesArrayType = opBinding.collectOperandTypes().get(1);
+    final boolean nullable = keysArrayType.isNullable() || valuesArrayType.isNullable();
+    return SqlTypeUtil.createMapType(
+        opBinding.getTypeFactory(),
+        requireNonNull(keysArrayType.getComponentType(), "inferred key type"),
+        requireNonNull(valuesArrayType.getComponentType(), "inferred value type"),
+        nullable);
+  }
+
+  /** The "MAP_FROM_ARRAYS(keysArray, valuesArray)" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlFunction MAP_FROM_ARRAYS =
+      SqlBasicFunction.create(SqlKind.MAP_FROM_ARRAYS,
+          SqlLibraryOperators::mapReturnType,
+          OperandTypes.ARRAY_ARRAY);
+
+  /** The "STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]])" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlFunction STR_TO_MAP =
+      SqlBasicFunction.create(SqlKind.STR_TO_MAP,

Review Comment:
   just reuse, not define a new OperandTypes like STRING_OPTIONAL_STRING because i don't find a good way to express it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1219849652


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -965,6 +965,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType deriveTypeMapFromArrays(SqlOperatorBinding opBinding) {
+    final RelDataType keysArrayType = opBinding.collectOperandTypes().get(0);

Review Comment:
   fixed @tanclary 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] tanclary commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.

tanclary commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1220569447


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",

Review Comment:
   Ok that could be a good test, then.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1579625358

   @tanclary squash the commits


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1578999018

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.2%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1577927328

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.2%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1574969753

   hi @JiajunBernoulli close the map_from_array pr , and one pr for the two commits


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] tanclary commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.

tanclary commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1223392544


##########
site/_docs/reference.md:
##########
@@ -2737,6 +2737,8 @@ BigQuery's type system uses confusingly different names for types and functions:
 | s | MAP_ENTRIES(map)                               | Returns the entries of the *map* as an array, the order of the entries is not defined
 | s | MAP_KEYS(map)                                  | Returns the keys of the *map* as an array, the order of the entries is not defined
 | s | MAP_VALUES(map)                                | Returns the values of the *map* as an array, the order of the entries is not defined
+| s | MAP_FROM_ARRAYS(array1, array2)                | Returns a map created from an *array1* and *array2*. Note that the lengths of two arrays should be the same
+| s | STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]]) | Returns a map after splitting the *string* into key/value pairs using delimiters. Default delimiters are ',' for *stringDelimiter* and ':' for *keyValueDelimiter*

Review Comment:
   nit: should there be spaces here? check other examples with optional args



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1581926901

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.5%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1581923892

   hi @tanclary @MasseGuillaume fix all your reviews, will you have a look again?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1585317402

   Hi @tanclary @MasseGuillaume @JiajunBernoulli @julianhjulidoes anyone make it further. My other pr depends on it.  Thanks for all reviews very much


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218856691


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3092,6 +3096,36 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      String defaultStringDelimiter = ",";

Review Comment:
   but not same



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1577979130

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.2%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1577938834

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.2%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1593262202

   @tanclary @MasseGuillaume will you have time to look again? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1574982405

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.1%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241090465


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5688,6 +5688,79 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("^map_from_arrays(2, array[1, 2])^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER>,"
+            + " <INTEGER ARRAY>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(^array[1, '1', true]^, array['a', 'b', 'c'])",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array['a', 'b', 'c'], ^array[1, '1', true]^)",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }
+
+  /** Tests {@code STR_TO_MAP} function from Spark. */
+  @Test void testStrToMapFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.STR_TO_MAP);
+    f0.checkFails("^str_to_map('a=1,b=2', ',', '=')^",
+        "No match found for function signature STR_TO_MAP\\("
+            + "<CHARACTER>, <CHARACTER>, <CHARACTER>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("str_to_map('a=1,b=2', ',', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2', ',')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a=1&b=2', '&', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k#2%v#3', '%', '#')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1&b:2', '&')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k:2%v:3', '%')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a')", "{a=null}",
+        "(CHAR(1) NOT NULL, CHAR(1) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a,b,c')", "{a=null, b=null, c=null}",
+        "(CHAR(5) NOT NULL, CHAR(5) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a-b--c', '--')", "{a-b=null, c=null}",
+        "(CHAR(6) NOT NULL, CHAR(6) NOT NULL) MAP NOT NULL");
+    f.checkType("str_to_map(cast(null as varchar))",
+        "(VARCHAR, VARCHAR) MAP");
+    f.checkNull("str_to_map(cast(null as varchar))");

Review Comment:
   @NobiGo added the unit tests



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1605951456

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [12 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.6%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1601154375

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.5%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1603714264

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [12 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.5%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241086232


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4003,6 +4004,33 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {

Review Comment:
   @NobiGo yeap. the calcite supports NullPolicy(6 policy), which will insert code in runtime, spark  using NullIntolerant(only one policy), which is equal to NullPolicy.ANY.
   
   for example：
   if the NullPolicy.ANY, the calcite will return null  If any of the arguments are null. so the user code doesn't need check whether the input is null
   
   ```
   defineMethod(MAP_FROM_ARRAYS, BuiltInMethod.MAP_FROM_ARRAYS.method, NullPolicy.ANY);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1222473808


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");

Review Comment:
   this is because spark's map is different from calcite is map implementation.
   spark suppprts two way, while calcite is LAST_WIN. if we 100% match spark, it should break the behavior of all map functions
   ```
     object MapKeyDedupPolicy extends Enumeration {
       val EXCEPTION, LAST_WIN = Value
     }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1222474429


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",

Review Comment:
   added



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1595493994

   Hi @tanclary do you have any other comment.  If not,  i will squalsh the commits


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218857556


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -944,6 +944,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {

Review Comment:
   other function also puts here like split you support in bigquery



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1220571865


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",

Review Comment:
   @tanclary thanks for your quick response, thank you so much 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1222475811


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }

Review Comment:
   no need. there is already exist
   ```
       f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
           "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
       f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1223622466


##########
site/_docs/reference.md:
##########
@@ -2737,6 +2737,8 @@ BigQuery's type system uses confusingly different names for types and functions:
 | s | MAP_ENTRIES(map)                               | Returns the entries of the *map* as an array, the order of the entries is not defined
 | s | MAP_KEYS(map)                                  | Returns the keys of the *map* as an array, the order of the entries is not defined
 | s | MAP_VALUES(map)                                | Returns the values of the *map* as an array, the order of the entries is not defined
+| s | MAP_FROM_ARRAYS(array1, array2)                | Returns a map created from an *array1* and *array2*. Note that the lengths of two arrays should be the same
+| s | STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]]) | Returns a map after splitting the *string* into key/value pairs using delimiters. Default delimiters are ',' for *stringDelimiter* and ':' for *keyValueDelimiter*

Review Comment:
   @tanclary does it too, for example
   
   ```
   | m o | REGEXP_REPLACE(string, regexp, rep [, pos [, occurrence [, matchType]]]) | Replaces all substrings of *string* that match *regexp* with *rep* at the starting *pos* in expr (if omitted, the default is 1), *occurrence* means which occurrence of a match to search for (if omitted, the default is 1), *matchType* specifies how to perform matching
   
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1573584337

   hi @JiajunBernoulli @tanclary @MasseGuillaume  do you have time to review this ,just review STR_TO_MAP and after
   the former commits merged, i will rebase it.otherwise, it will lots of conflict


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1215551391


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -944,6 +944,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+    final RelDataType keysArrayType = opBinding.collectOperandTypes().get(0);
+    final RelDataType valuesArrayType = opBinding.collectOperandTypes().get(1);
+    final boolean nullable = keysArrayType.isNullable() || valuesArrayType.isNullable();
+    return SqlTypeUtil.createMapType(
+        opBinding.getTypeFactory(),
+        requireNonNull(keysArrayType.getComponentType(), "inferred key type"),
+        requireNonNull(valuesArrayType.getComponentType(), "inferred value type"),
+        nullable);
+  }

Review Comment:
   comments:
           final boolean nullable = keysArrayType.isNullable() || valuesArrayType.isNullable();
    this should be ,so i add the unit test
       f.checkType("str_to_map(cast(null as varchar))",
           "(VARCHAR, VARCHAR) 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-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1587094804

   hi @tanclary @MasseGuillaume do you have time to look again? thank you


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241086232


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4003,6 +4004,33 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {

Review Comment:
   @NobiGo yeap. the calcite supports NullPolicy(6 policy), which will insert code in runtime, spark  using NullIntolerant(only one policy), which is equal to NullPolicy.
   
   for example：
   if the NullPolicy.ANY, the calcite will return null  If any of the arguments are null. so the user code doesn't need check whether the input is null
   
   ```
   defineMethod(MAP_FROM_ARRAYS, BuiltInMethod.MAP_FROM_ARRAYS.method, NullPolicy.ANY);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1601094029

   hi @tanclary the pr has approved, i squash the commits. will you help review it again or merge it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] NobiGo merged pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "NobiGo (via GitHub)" <gi...@apache.org>.

NobiGo merged PR #3238:
URL: https://github.com/apache/calcite/pull/3238


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] NobiGo commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "NobiGo (via GitHub)" <gi...@apache.org>.

NobiGo commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241074645


##########
core/src/main/java/org/apache/calcite/runtime/CalciteResource.java:
##########
@@ -897,6 +897,9 @@ ExInst<CalciteException> illegalArgumentForTableFunctionCall(String a0,
   @BaseMessage("Substring error: negative substring length not allowed")
   ExInst<CalciteException> illegalNegativeSubstringLength();
 
+  @BaseMessage("Illegal arguments: The length of the keys array {0,number,#} is not equal to the length of the values array {1,number,#} in MAP_FROM_ARRAYS function")

Review Comment:
   How about we change this message to 'The key array and value array of parameters must have the same length'?



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4003,6 +4004,33 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {

Review Comment:
   the parameter keysArray and values Array has confirmed is Not NULL?



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5688,6 +5688,79 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("^map_from_arrays(2, array[1, 2])^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER>,"
+            + " <INTEGER ARRAY>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(^array[1, '1', true]^, array['a', 'b', 'c'])",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array['a', 'b', 'c'], ^array[1, '1', true]^)",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }
+
+  /** Tests {@code STR_TO_MAP} function from Spark. */
+  @Test void testStrToMapFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.STR_TO_MAP);
+    f0.checkFails("^str_to_map('a=1,b=2', ',', '=')^",
+        "No match found for function signature STR_TO_MAP\\("
+            + "<CHARACTER>, <CHARACTER>, <CHARACTER>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("str_to_map('a=1,b=2', ',', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2', ',')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a=1&b=2', '&', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k#2%v#3', '%', '#')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1&b:2', '&')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k:2%v:3', '%')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a')", "{a=null}",
+        "(CHAR(1) NOT NULL, CHAR(1) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a,b,c')", "{a=null, b=null, c=null}",
+        "(CHAR(5) NOT NULL, CHAR(5) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a-b--c', '--')", "{a-b=null, c=null}",
+        "(CHAR(6) NOT NULL, CHAR(6) NOT NULL) MAP NOT NULL");
+    f.checkType("str_to_map(cast(null as varchar))",
+        "(VARCHAR, VARCHAR) MAP");
+    f.checkNull("str_to_map(cast(null as varchar))");

Review Comment:
   Add tests like(Any parameters is NULL, the result will be NULL):
   
   `SELECT str_to_map(null, ',', ':');
   
   SELECT str_to_map('a:1,b:2,c:3', null, ':');
   
   SELECT str_to_map('a:1,b:2,c:3', ',',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

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1241090465


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5688,6 +5688,79 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("^map_from_arrays(2, array[1, 2])^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER>,"
+            + " <INTEGER ARRAY>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(^array[1, '1', true]^, array['a', 'b', 'c'])",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array['a', 'b', 'c'], ^array[1, '1', true]^)",
+        "Parameters must be of the same type",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }
+
+  /** Tests {@code STR_TO_MAP} function from Spark. */
+  @Test void testStrToMapFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.STR_TO_MAP);
+    f0.checkFails("^str_to_map('a=1,b=2', ',', '=')^",
+        "No match found for function signature STR_TO_MAP\\("
+            + "<CHARACTER>, <CHARACTER>, <CHARACTER>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("str_to_map('a=1,b=2', ',', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2', ',')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a=1&b=2', '&', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k#2%v#3', '%', '#')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1&b:2', '&')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k:2%v:3', '%')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a')", "{a=null}",
+        "(CHAR(1) NOT NULL, CHAR(1) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a,b,c')", "{a=null, b=null, c=null}",
+        "(CHAR(5) NOT NULL, CHAR(5) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a-b--c', '--')", "{a-b=null, c=null}",
+        "(CHAR(6) NOT NULL, CHAR(6) NOT NULL) MAP NOT NULL");
+    f.checkType("str_to_map(cast(null as varchar))",
+        "(VARCHAR, VARCHAR) MAP");
+    f.checkNull("str_to_map(cast(null as varchar))");

Review Comment:
   @NobiGo added the unit tests  Thanks for your review!
   
   ```
       f.checkNull("str_to_map(null, ',', ':')");
       f.checkNull("str_to_map('a:1,b:2,c:3', null, ':')");
       f.checkNull("str_to_map('a:1,b:2,c:3', ',',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

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.

sonarcloud[bot] commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1574984685

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3238)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3238&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3238&resolved=false&types=CODE_SMELL)
   
   [![95.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '95.1%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list) [95.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3238&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1222472026


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array[1, 1, null], array['foo', 'bar', 'name'])",
+        "{1=bar, null=name}", "(INTEGER, CHAR(4) NOT NULL) MAP NOT NULL");
+    f.checkScalar("map_from_arrays(array(), array())",
+        "{}", "(UNKNOWN NOT NULL, UNKNOWN NOT NULL) MAP NOT NULL");
+    f.checkType("map_from_arrays(cast(null as integer array), array['foo', 'bar'])",
+        "(INTEGER NOT NULL, CHAR(3) NOT NULL) MAP");
+    f.checkNull("map_from_arrays(cast(null as integer array), array['foo', 'bar'])");
+
+    f.checkFails("^map_from_arrays(array[1, 2], 2)^",
+        "Cannot apply 'MAP_FROM_ARRAYS' to arguments of type 'MAP_FROM_ARRAYS\\(<INTEGER ARRAY>,"
+            + " <INTEGER>\\)'. Supported form\\(s\\): 'MAP_FROM_ARRAYS\\(<ARRAY>, <ARRAY>\\)'",
+        false);
+    f.checkFails("map_from_arrays(array[1, 2], array['foo'])",
+        "Illegal arguments: The length of the keys array 2 is not equal to the length "
+            + "of the values array 1 in MAP_FROM_ARRAYS function",
+        true);
+  }
+
+  /** Tests {@code STR_TO_MAP} function from Spark. */
+  @Test void testStrToMapFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.STR_TO_MAP);
+    f0.checkFails("^str_to_map('a=1,b=2', ',', '=')^",
+        "No match found for function signature STR_TO_MAP\\("
+            + "<CHARACTER>, <CHARACTER>, <CHARACTER>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("str_to_map('a=1,b=2', ',', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1,b:2', ',')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a=1&b=2', '&', '=')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k#2%v#3', '%', '#')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a:1&b:2', '&')", "{a=1, b=2}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('k:2%v:3', '%')", "{k=2, v=3}",
+        "(CHAR(7) NOT NULL, CHAR(7) NOT NULL) MAP NOT NULL");
+    f.checkScalar("str_to_map('a')", "{a=null}",
+        "(CHAR(1) NOT NULL, CHAR(1) NOT NULL) MAP NOT NULL");
+    f.checkType("str_to_map(cast(null as varchar))",
+        "(VARCHAR, VARCHAR) MAP");
+    f.checkNull("str_to_map(cast(null as varchar))");
+  }

Review Comment:
   added test
   ```
       f.checkScalar("str_to_map('a,b,c')", "{a=null, b=null, c=null}",
           "(CHAR(5) NOT NULL, CHAR(5) NOT NULL) MAP NOT NULL");
       f.checkScalar("str_to_map('a-b--c', '--')", "{a-b=null, c=null}",
           "(CHAR(6) NOT NULL, CHAR(6) NOT NULL) MAP NOT 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

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218857018


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3917,6 +3918,38 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */
+  public static Map mapFromArrays(List keysArray, List valuesArray) {
+    if (keysArray.size() != valuesArray.size()) {
+      throw new IllegalArgumentException(

Review Comment:
   use Resources



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218855652


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -944,6 +944,31 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
           ReturnTypes.TO_MAP_VALUES_NULLABLE,
           OperandTypes.MAP);
 
+  private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
+    final RelDataType keysArrayType = opBinding.collectOperandTypes().get(0);
+    final RelDataType valuesArrayType = opBinding.collectOperandTypes().get(1);
+    final boolean nullable = keysArrayType.isNullable() || valuesArrayType.isNullable();
+    return SqlTypeUtil.createMapType(
+        opBinding.getTypeFactory(),
+        requireNonNull(keysArrayType.getComponentType(), "inferred key type"),
+        requireNonNull(valuesArrayType.getComponentType(), "inferred value type"),
+        nullable);
+  }
+
+  /** The "MAP_FROM_ARRAYS(keysArray, valuesArray)" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlFunction MAP_FROM_ARRAYS =
+      SqlBasicFunction.create(SqlKind.MAP_FROM_ARRAYS,
+          SqlLibraryOperators::mapReturnType,
+          OperandTypes.ARRAY_ARRAY);
+
+  /** The "STR_TO_MAP(string[, stringDelimiter[, keyValueDelimiter]])" function. */
+  @LibraryOperator(libraries = {SPARK})
+  public static final SqlFunction STR_TO_MAP =
+      SqlBasicFunction.create(SqlKind.STR_TO_MAP,

Review Comment:
   fixed @tanclary define STRING_OPTIONAL_STRING_OPTIONAL_STRING



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] tanclary commented on a diff in pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.

tanclary commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1219926955


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5543,6 +5543,65 @@ private static void checkIf(SqlOperatorFixture f) {
         "INTEGER ARRAY NOT NULL");
   }
 
+  /** Tests {@code MAP_FROM_ARRAYS} function from Spark. */
+  @Test void testMapFromArraysFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(SqlLibraryOperators.MAP_FROM_ARRAYS);
+    f0.checkFails("^map_from_arrays(array[1, 2], array['foo', 'bar'])^",
+        "No match found for function signature MAP_FROM_ARRAYS\\(<INTEGER ARRAY>, "
+            + "<CHAR\\(3\\) ARRAY>\\)", false);
+
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+    f.checkScalar("map_from_arrays(array[1, 2], array['foo', 'bar'])", "{1=foo, 2=bar}",

Review Comment:
   do all elements of the array have to be of the same type ? could you have [1, "a", true]?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218856500


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3092,6 +3096,36 @@ private static class ArrayConcatImplementor extends AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for str_to_map. */
+  private static class StringToMapImplementor extends AbstractRexCallImplementor {
+    StringToMapImplementor() {
+      super("str_to_map", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(RexToLixTranslator translator,
+        RexCall call, List<Expression> argValueList) {
+      String defaultStringDelimiter = ",";
+      String defaultKeyValueDelimiter = ":";
+      if (argValueList.size() == 1) {

Review Comment:
   change to switch



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on a diff in pull request #3238: [CALCITE-5744] Add STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on code in PR #3238:
URL: https://github.com/apache/calcite/pull/3238#discussion_r1218854281


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3917,6 +3918,38 @@ public static List mapValues(Map map) {
     return new ArrayList<>(map.values());
   }
 
+  /** Support the MAP_FROM_ARRAYS function. */

Review Comment:
   fix pr info and issue description



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] liuyongvs commented on pull request #3238: [CALCITE-5744] Add MAP_FROM_ARRAYS, STR_TO_MAP function (enabled in Spark library)

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.

liuyongvs commented on PR #3238:
URL: https://github.com/apache/calcite/pull/3238#issuecomment-1581056368

   @tanclary @MasseGuillaume will you help look again? 


-- 
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