You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "herunkang2018 (via GitHub)" <gi...@apache.org> on 2023/07/09 02:43:28 UTC

[GitHub] [calcite] herunkang2018 opened a new pull request, #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

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

   (no comment)


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

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

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

[GitHub] [calcite] herunkang2018 commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1630033642

   @tanclary all comments resolved, please help to review if you have time.


-- 
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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259125972


##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract] ) | Returns the specified part from the *urlString*. Valid values for *partToExtract* include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO

Review Comment:
   I would make mention of `keyToExtract` here



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    String extractValue;
+    PartToExtract part;
+    try {
+      part = PartToExtract.valueOf(partToExtract);
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+
+    switch (part) {
+    case HOST:
+      extractValue = uri.getHost();
+      break;
+    case PATH:
+      extractValue = uri.getRawPath();
+      break;
+    case QUERY:
+      extractValue = uri.getRawQuery();
+      break;
+    case REF:
+      extractValue = uri.getRawFragment();
+      break;
+    case PROTOCOL:
+      extractValue = uri.getScheme();
+      break;
+    case FILE:
+      if (uri.getRawQuery() != null) {
+        extractValue = uri.getRawPath() + "?" + uri.getRawQuery();
+      } else {
+        extractValue = uri.getRawPath();
+      }
+      break;
+    case AUTHORITY:
+      extractValue = uri.getRawAuthority();
+      break;
+    case USERINFO:
+      extractValue = uri.getRawUserInfo();
+      break;
+    default:
+      return null;

Review Comment:
   Oh I see you have a test for so I guess it is right



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    String extractValue;
+    PartToExtract part;
+    try {
+      part = PartToExtract.valueOf(partToExtract);
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+
+    switch (part) {
+    case HOST:
+      extractValue = uri.getHost();
+      break;
+    case PATH:
+      extractValue = uri.getRawPath();
+      break;
+    case QUERY:
+      extractValue = uri.getRawQuery();
+      break;
+    case REF:
+      extractValue = uri.getRawFragment();
+      break;
+    case PROTOCOL:
+      extractValue = uri.getScheme();
+      break;
+    case FILE:
+      if (uri.getRawQuery() != null) {
+        extractValue = uri.getRawPath() + "?" + uri.getRawQuery();
+      } else {
+        extractValue = uri.getRawPath();
+      }
+      break;
+    case AUTHORITY:
+      extractValue = uri.getRawAuthority();
+      break;
+    case USERINFO:
+      extractValue = uri.getRawUserInfo();
+      break;
+    default:
+      return null;
+    }
+    return extractValue;
+  }
+
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract, @Nullable String keyToExtract) {
+    if (partToExtract == null || !partToExtract.equals("QUERY")) {
+      return null;
+    }
+
+    String query = parseUrl(urlStr, partToExtract);
+    if (query == null) {
+      return null;
+    }
+
+    Pattern p = Pattern.compile("(&|^)" + keyToExtract + "=([^&]*)");
+    Matcher m = p.matcher(query);
+    if (m.find()) {
+      return m.group(2);

Review Comment:
   can this be simplified: `return m.find() ? m.group(2) : 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] tanclary commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259026542


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -394,6 +394,14 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
   public static final SqlFunction SUBSTR_POSTGRESQL =
       SUBSTR.withKind(SqlKind.SUBSTR_POSTGRESQL);
 
+  /** The "PARSE_URL(urlString, partToExtract [, keyToExtract])" function. */

Review Comment:
   Yeah that's true there are inconsistencies. I think it makes sense to have spaces on either both sides or neither. Since nearly all examples have a space (`foo [, bar`) at the beginning it makes sense to match. But you are right it is not consistent across all documentation.



-- 
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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259120506


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,70 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract])} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    if (partToExtract.equals("HOST")) {

Review Comment:
   I have refactor it. Thanks for your advice.



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,70 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract])} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    if (partToExtract.equals("HOST")) {

Review Comment:
   I have refactored it. Thanks for your advice.



-- 
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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1627606787

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![89.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '89.8%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [89.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259992877


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,80 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract, @Nullable String keyToExtract) {
+    if (partToExtract == null || !partToExtract.equals("QUERY")) {
+      return null;
+    }
+
+    String query = parseUrl(urlStr, partToExtract);
+    if (query == null) {
+      return null;
+    }
+
+    Pattern p = Pattern.compile("(&|^)" + keyToExtract + "=([^&]*)");
+    Matcher m = p.matcher(query);
+    return m.find() ? m.group(2) : null;
+  }
+
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    String extractValue;
+    PartToExtract part;
+    try {
+      part = PartToExtract.valueOf(partToExtract);
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+
+    switch (part) {
+    case HOST:
+      extractValue = uri.getHost();
+      break;
+    case PATH:
+      extractValue = uri.getRawPath();
+      break;
+    case QUERY:
+      extractValue = uri.getRawQuery();
+      break;
+    case REF:
+      extractValue = uri.getRawFragment();
+      break;
+    case PROTOCOL:
+      extractValue = uri.getScheme();
+      break;
+    case FILE:
+      if (uri.getRawQuery() != null) {
+        extractValue = uri.getRawPath() + "?" + uri.getRawQuery();
+      } else {
+        extractValue = uri.getRawPath();
+      }
+      break;
+    case AUTHORITY:
+      extractValue = uri.getRawAuthority();
+      break;
+    case USERINFO:
+      extractValue = uri.getRawUserInfo();
+      break;
+    default:
+      extractValue = null;
+      break;

Review Comment:
   I don't think you need a `break` in the default case.



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,80 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract, @Nullable String keyToExtract) {
+    if (partToExtract == null || !partToExtract.equals("QUERY")) {
+      return null;
+    }
+
+    String query = parseUrl(urlStr, partToExtract);
+    if (query == null) {
+      return null;
+    }
+
+    Pattern p = Pattern.compile("(&|^)" + keyToExtract + "=([^&]*)");
+    Matcher m = p.matcher(query);
+    return m.find() ? m.group(2) : null;
+  }
+
+  public static @Nullable String parseUrl(@Nullable String urlStr,

Review Comment:
   I think this should have a javadoc too, other implementations each method has a javadoc so I think we should try to match that.



##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract] ) | Returns the specified part from the *urlString*. Valid values for *partToExtract* include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO. *keyToExtract* specifies which query to extract

Review Comment:
   Backslashes are just to avoid formatting in github



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4565,4 +4642,16 @@ public enum FlatProductInputType {
     SCALAR, LIST, MAP
   }
 
+  /** Type of part to extract passed into {@link #parseUrl}. */
+  public enum PartToExtract {
+    HOST,

Review Comment:
   Is there a reason for this specific ordering? If not maybe we could alphabetize this and the switch statement? This is just a nit so if you feel strongly one way or the other it's probably fine.



##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract] ) | Returns the specified part from the *urlString*. Valid values for *partToExtract* include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO. *keyToExtract* specifies which query to extract

Review Comment:
   Does it make more sense to say `Returns specified \*partToExtract\* from the \*urlString\*`?



-- 
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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1661490872

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![90.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '90.6%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [90.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259726419


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */

Review Comment:
   I have adjusted the places of these two method, and the doc.



-- 
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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1629930201

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![89.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '89.6%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [89.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259023298


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -394,6 +394,14 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
   public static final SqlFunction SUBSTR_POSTGRESQL =
       SUBSTR.withKind(SqlKind.SUBSTR_POSTGRESQL);
 
+  /** The "PARSE_URL(urlString, partToExtract [, keyToExtract])" function. */

Review Comment:
   I found that both with and without spaces were used in the previous code, so I want to know which one is recommended and would you like to explain the reason?



-- 
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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1260422587


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4565,4 +4642,16 @@ public enum FlatProductInputType {
     SCALAR, LIST, MAP
   }
 
+  /** Type of part to extract passed into {@link #parseUrl}. */
+  public enum PartToExtract {
+    HOST,

Review Comment:
   IMO the enum type doesn't need to keep alphabetize order, like `SqlKind`.
   In this case, this is related to the order of the extracted parts of URL, like HOST/PATH/QUERY/REF. Although the latter part is not strictly in accordance with the order of the URL, it may be used by users less, so it is placed later.
   And this way is keep the same as Hive's and Spark's implementation, so I think it's probably fine.



-- 
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] herunkang2018 commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1631685014

   @tanclary All comments resolved, please help to review if you have time.


-- 
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] herunkang2018 commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1661475879

   @tanclary I have squashed to the final commit. Thanks for the reviewing work very kindly.


-- 
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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1260424030


##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract] ) | Returns the specified part from the *urlString*. Valid values for *partToExtract* include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO. *keyToExtract* specifies which query to extract

Review Comment:
   Thanks for review. I will improve it soon.



-- 
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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1631698097

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![90.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '90.6%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [90.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259019911


##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract]) | Returns the specified part from the URL. Valid values for partToExtract include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO

Review Comment:
   Thanks for reviewing, I will fix it soon.



-- 
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] herunkang2018 commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1656507617

   @tanclary would like to have a look if you have time, since the 1.35.0 is released and the main branch may open 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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1631128532

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![90.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '90.6%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [90.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259030984


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -394,6 +394,14 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
   public static final SqlFunction SUBSTR_POSTGRESQL =
       SUBSTR.withKind(SqlKind.SUBSTR_POSTGRESQL);
 
+  /** The "PARSE_URL(urlString, partToExtract [, keyToExtract])" function. */

Review Comment:
   Fixed.



##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract]) | Returns the specified part from the URL. Valid values for partToExtract include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO

Review Comment:
   Fixed.



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

To unsubscribe, e-mail: commits-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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1629959096

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![88.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '88.5%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [88.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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] NobiGo commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259217075


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */

Review Comment:
   Looks like the Java Doc is more suitable for placing on the method below.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -3946,6 +3946,87 @@ static void checkRlikeFails(SqlOperatorFixture f) {
     f.checkNull("ASCII(cast(null as varchar(1)))");
   }
 
+  @Test void testParseUrl() {
+    final SqlOperatorFixture f0 = fixture()
+        .setFor(SqlLibraryOperators.PARSE_URL);
+    f0.checkFails("^parse_url('http://calcite.apache.org/path1', 'HOST')^",
+        "No match found for function signature PARSE_URL\\(<CHARACTER>, <CHARACTER>\\)",
+        false);
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      // test with each valid partToExtract
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'HOST')",
+          "calcite.apache.org",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'PATH')",
+          "/path1/p.php",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/%20p.php?k1=v1&k2=v2#Ref1',"
+              + " 'PATH')",
+          "/path1/%20p.php",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'QUERY')",
+          "k1=v1&k2=v2",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'REF')",
+          "Ref1",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'QUERY', 'k2')",
+          "v2",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'QUERY', 'k1')",
+          "v1",
+          "VARCHAR NOT NULL");
+      f.checkNull("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+          + " 'QUERY', 'k3')");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'FILE')",
+          "/path1/p.php?k1=v1&k2=v2",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php',"
+              + " 'FILE')",
+          "/path1/p.php",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'PROTOCOL')",
+          "http",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('https://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'PROTOCOL')",
+          "https",
+          "VARCHAR NOT NULL");
+      f.checkString("parse_url('http://bob@calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'USERINFO')",
+          "bob",
+          "VARCHAR NOT NULL");
+      f.checkNull("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+          + " 'USERINFO')");
+      f.checkString("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'AUTHORITY')",
+          "calcite.apache.org",
+          "VARCHAR NOT NULL");
+
+      // test with invalid partToExtract
+      f.checkNull("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"
+              + " 'INVALID_PART_TO_EXTRACT')");
+
+      // test with operands with null values
+      f.checkNull("parse_url('http://calcite.apache.org/path1/p.php?k1=v1&k2=v2#Ref1',"

Review Comment:
   Please add a test when we have an invalid URL.



##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract] ) | Returns the specified part from the *urlString*. Valid values for *partToExtract* include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO

Review Comment:
   I agree with this too.



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */

Review Comment:
   Looks like the Java Doc is more suitable for placing on the method below.



##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract] ) | Returns the specified part from the *urlString*. Valid values for *partToExtract* include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO

Review Comment:
   I agree with this too.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -3946,6 +3946,87 @@ static void checkRlikeFails(SqlOperatorFixture f) {
     f.checkNull("ASCII(cast(null as varchar(1)))");
   }
 
+  @Test void testParseUrl() {

Review Comment:
   I will put this method in parseTime after, making it more reasonable. But it is Ok to put it here.



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

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

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

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1627584622

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![89.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '89.8%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [89.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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] herunkang2018 commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1630897475

   @tanclary @NobiGo Thanks a lot for reviewing.
   All comments resolved, please help to review if you have time.


-- 
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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1258657227


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,70 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract])} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    if (partToExtract.equals("HOST")) {

Review Comment:
   if there;s a defined set of what `partToExtract` can be, you may want to do an enum and then you can make a switch statement



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -394,6 +394,14 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
   public static final SqlFunction SUBSTR_POSTGRESQL =
       SUBSTR.withKind(SqlKind.SUBSTR_POSTGRESQL);
 
+  /** The "PARSE_URL(urlString, partToExtract [, keyToExtract])" function. */

Review Comment:
   may want to add space after `]`



##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract]) | Returns the specified part from the URL. Valid values for partToExtract include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO

Review Comment:
   nit: add space after `]`



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,70 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract])} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    if (partToExtract.equals("HOST")) {
+      return uri.getHost();
+    }
+    if (partToExtract.equals("PATH")) {
+      return uri.getRawPath();
+    }
+    if (partToExtract.equals("QUERY")) {
+      return uri.getRawQuery();
+    }
+    if (partToExtract.equals("REF")) {
+      return uri.getRawFragment();
+    }
+    if (partToExtract.equals("PROTOCOL")) {
+      return uri.getScheme();
+    }
+    if (partToExtract.equals("FILE")) {
+      if (uri.getRawQuery() != null) {
+        return uri.getRawPath() + "?" + uri.getRawQuery();
+      }
+      return uri.getRawPath();
+    }
+    if (partToExtract.equals("AUTHORITY")) {
+      return uri.getRawAuthority();
+    }
+    if (partToExtract.equals("USERINFO")) {
+      return uri.getRawUserInfo();
+    }
+
+    return null;
+  }
+
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract, @Nullable String keyToExtract) {
+    if (!partToExtract.equals("QUERY")) {

Review Comment:
   I think you need to add an assertion or check to avoid the checker failures for referencing a possibly-null object (since they are annotated as nullable)



##########
site/_docs/reference.md:
##########
@@ -2769,6 +2769,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | PARSE_DATETIME(format, string)                 | Uses format specified by *format* to convert *string* representation of datetime to a TIMESTAMP value
 | b | PARSE_TIME(format, string)                     | Uses format specified by *format* to convert *string* representation of time to a TIME value
 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
+| h s | PARSE_URL(urlString, partToExtract [, keyToExtract]) | Returns the specified part from the URL. Valid values for partToExtract include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO

Review Comment:
   Reference the names you used in the function signature and surround them with **.



-- 
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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259688444


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    String extractValue;
+    PartToExtract part;
+    try {
+      part = PartToExtract.valueOf(partToExtract);
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+
+    switch (part) {
+    case HOST:
+      extractValue = uri.getHost();
+      break;
+    case PATH:
+      extractValue = uri.getRawPath();
+      break;
+    case QUERY:
+      extractValue = uri.getRawQuery();
+      break;
+    case REF:
+      extractValue = uri.getRawFragment();
+      break;
+    case PROTOCOL:
+      extractValue = uri.getScheme();
+      break;
+    case FILE:
+      if (uri.getRawQuery() != null) {
+        extractValue = uri.getRawPath() + "?" + uri.getRawQuery();
+      } else {
+        extractValue = uri.getRawPath();
+      }
+      break;
+    case AUTHORITY:
+      extractValue = uri.getRawAuthority();
+      break;
+    case USERINFO:
+      extractValue = uri.getRawUserInfo();
+      break;
+    default:
+      return null;
+    }
+    return extractValue;
+  }
+
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract, @Nullable String keyToExtract) {
+    if (partToExtract == null || !partToExtract.equals("QUERY")) {
+      return null;
+    }
+
+    String query = parseUrl(urlStr, partToExtract);
+    if (query == null) {
+      return null;
+    }
+
+    Pattern p = Pattern.compile("(&|^)" + keyToExtract + "=([^&]*)");
+    Matcher m = p.matcher(query);
+    if (m.find()) {
+      return m.group(2);

Review Comment:
   I will improve it.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -3946,6 +3946,87 @@ static void checkRlikeFails(SqlOperatorFixture f) {
     f.checkNull("ASCII(cast(null as varchar(1)))");
   }
 
+  @Test void testParseUrl() {

Review Comment:
   Thanks for reviewing. I have considerd this option, but these tests are related to date/time/timestmap processing, not string processing, so I place it here.



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

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

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

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1629227531

   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=3300)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3300&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=3300&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=3300&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3300&resolved=false&types=CODE_SMELL)
   
   [![88.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '88.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&metric=new_coverage&view=list) [88.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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=3300&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3300&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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259686317


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    String extractValue;
+    PartToExtract part;
+    try {
+      part = PartToExtract.valueOf(partToExtract);
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+
+    switch (part) {
+    case HOST:
+      extractValue = uri.getHost();
+      break;
+    case PATH:
+      extractValue = uri.getRawPath();
+      break;
+    case QUERY:
+      extractValue = uri.getRawQuery();
+      break;
+    case REF:
+      extractValue = uri.getRawFragment();
+      break;
+    case PROTOCOL:
+      extractValue = uri.getScheme();
+      break;
+    case FILE:
+      if (uri.getRawQuery() != null) {
+        extractValue = uri.getRawPath() + "?" + uri.getRawQuery();
+      } else {
+        extractValue = uri.getRawPath();
+      }
+      break;
+    case AUTHORITY:
+      extractValue = uri.getRawAuthority();
+      break;
+    case USERINFO:
+      extractValue = uri.getRawUserInfo();
+      break;
+    default:
+      return null;

Review Comment:
   Thanks for remind, I think this can be improved to `break;`, I will update it soon.
   Note that since we have handled `IllegalArgumentException` before, here we don't need to handle mismatched enum value again, and can safely use `break;`.



-- 
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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259018434


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,70 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract])} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    if (partToExtract.equals("HOST")) {
+      return uri.getHost();
+    }
+    if (partToExtract.equals("PATH")) {
+      return uri.getRawPath();
+    }
+    if (partToExtract.equals("QUERY")) {
+      return uri.getRawQuery();
+    }
+    if (partToExtract.equals("REF")) {
+      return uri.getRawFragment();
+    }
+    if (partToExtract.equals("PROTOCOL")) {
+      return uri.getScheme();
+    }
+    if (partToExtract.equals("FILE")) {
+      if (uri.getRawQuery() != null) {
+        return uri.getRawPath() + "?" + uri.getRawQuery();
+      }
+      return uri.getRawPath();
+    }
+    if (partToExtract.equals("AUTHORITY")) {
+      return uri.getRawAuthority();
+    }
+    if (partToExtract.equals("USERINFO")) {
+      return uri.getRawUserInfo();
+    }
+
+    return null;
+  }
+
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract, @Nullable String keyToExtract) {
+    if (!partToExtract.equals("QUERY")) {

Review Comment:
   I will fix it soon.



-- 
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] herunkang2018 commented on a diff in pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259031115


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,70 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract])} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    if (partToExtract.equals("HOST")) {
+      return uri.getHost();
+    }
+    if (partToExtract.equals("PATH")) {
+      return uri.getRawPath();
+    }
+    if (partToExtract.equals("QUERY")) {
+      return uri.getRawQuery();
+    }
+    if (partToExtract.equals("REF")) {
+      return uri.getRawFragment();
+    }
+    if (partToExtract.equals("PROTOCOL")) {
+      return uri.getScheme();
+    }
+    if (partToExtract.equals("FILE")) {
+      if (uri.getRawQuery() != null) {
+        return uri.getRawPath() + "?" + uri.getRawQuery();
+      }
+      return uri.getRawPath();
+    }
+    if (partToExtract.equals("AUTHORITY")) {
+      return uri.getRawAuthority();
+    }
+    if (partToExtract.equals("USERINFO")) {
+      return uri.getRawUserInfo();
+    }
+
+    return null;
+  }
+
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract, @Nullable String keyToExtract) {
+    if (!partToExtract.equals("QUERY")) {

Review Comment:
   Fixed.



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

To unsubscribe, e-mail: commits-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 #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3300:
URL: https://github.com/apache/calcite/pull/3300#discussion_r1259124630


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -875,6 +878,82 @@ public static String translateWithCharset(String s, String transcodingName) {
     }
   }
 
+  /** SQL {@code PARSE_URL(urlStr, partToExtract [, keyToExtract] )} function. */
+  public static @Nullable String parseUrl(@Nullable String urlStr,
+      @Nullable String partToExtract) {
+    if (urlStr == null || partToExtract == null) {
+      return null;
+    }
+
+    URI uri;
+    try {
+      uri = new URI(urlStr);
+    } catch (URISyntaxException e) {
+      return null;
+    }
+
+    String extractValue;
+    PartToExtract part;
+    try {
+      part = PartToExtract.valueOf(partToExtract);
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+
+    switch (part) {
+    case HOST:
+      extractValue = uri.getHost();
+      break;
+    case PATH:
+      extractValue = uri.getRawPath();
+      break;
+    case QUERY:
+      extractValue = uri.getRawQuery();
+      break;
+    case REF:
+      extractValue = uri.getRawFragment();
+      break;
+    case PROTOCOL:
+      extractValue = uri.getScheme();
+      break;
+    case FILE:
+      if (uri.getRawQuery() != null) {
+        extractValue = uri.getRawPath() + "?" + uri.getRawQuery();
+      } else {
+        extractValue = uri.getRawPath();
+      }
+      break;
+    case AUTHORITY:
+      extractValue = uri.getRawAuthority();
+      break;
+    case USERINFO:
+      extractValue = uri.getRawUserInfo();
+      break;
+    default:
+      return null;

Review Comment:
   Is this the right behavior? Or should it throw an error?



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

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

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

[GitHub] [calcite] herunkang2018 commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "herunkang2018 (via GitHub)" <gi...@apache.org>.
herunkang2018 commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1636001982

   @tanclary @NobiGo would you like to help to review again if you have time? Thanks 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] NobiGo commented on pull request #3300: [CALCITE-5820] Add PARSE_URL function (enabled in Hive and Spark library)

Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on PR #3300:
URL: https://github.com/apache/calcite/pull/3300#issuecomment-1635086340

   > Thanks for fixing all the comments! I left a couple more small ones but I think this is nearly mergeable, what do you think @NobiGo ?
   
   LGTM. We can continue.


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