Re: [PR] [CALCITE-5826] Add FIND_IN_SET function (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3317)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=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=3317=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=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=3317=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=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=3317=false=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=3317=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=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=3317=false=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=3317=false=CODE_SMELL)
 [0 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL)
   
   
[![92.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png
 
'92.1%')](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list)
 [92.1% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=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=3317=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6074] The size of REAL, DOUBLE, and FLOAT is not consistent [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3492)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3492=false=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=3492=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3492=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3492=false=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=3492=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3492=false=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=3492=false=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=3492=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3492=false=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=3492=false=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=3492=false=CODE_SMELL)
 [6 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3492=false=CODE_SMELL)
   
   
[![57.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png
 
'57.1%')](https://sonarcloud.io/component_measures?id=apache_calcite=3492=new_coverage=list)
 [57.1% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3492=new_coverage=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=3492=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3492=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-5863] incorrect validation for window with range and multiple order by [calcite]

2023-10-30 Thread via GitHub


JiajunBernoulli commented on PR #3325:
URL: https://github.com/apache/calcite/pull/3325#issuecomment-1786309702

   Nice work, can we add some tests in RelOptRulesTest?
   > Use some window rule like `CoreRules.WINDOW_REDUCE_EXPRESSIONS`.


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

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

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



[PR] [CALCITE-6074] The size of REAL, DOUBLE, and FLOAT is not consistent [calcite]

2023-10-30 Thread via GitHub


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

   This is based on a code audit.
   The assumption is that FLOAT=DOUBLE=64 bit and REAL=32 bit
   I also deleted some deprecated functions instead of fixing them.


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

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

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



Re: [PR] [CALCITE-5826] Add FIND_IN_SET function (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3317:
URL: https://github.com/apache/calcite/pull/3317#discussion_r1376958065


##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1076,6 +1078,41 @@ public static int levenshtein(String string1, String 
string2) {
 return LEVENSHTEIN_DISTANCE.apply(string1, string2);
   }
 
+  /** SQL FIND_IN_SET(matchStr, textStr) function.
+   * Returns the index (1-based) of the given matchStr
+   * in the comma-delimited list textStr. Returns 0,
+   * if the matchStr is not found or if the matchStr
+   * contains a comma. */
+  public static @Nullable Integer findInSet(
+  @Nullable String matchStr,
+  @Nullable String textStr) {
+if (matchStr == null || textStr == null) {
+  return null;
+}
+if (matchStr.contains(String.valueOf(COMMA_DELIMITER))) {
+  return 0;
+}
+final int textStrLen = textStr.length();
+final int matchStrLen = matchStr.length();
+int n = 1;
+int lastCommaIndex = -1;
+for (int i = 0; i < textStrLen; i++) {
+  if (textStr.charAt(i) == COMMA_DELIMITER) {

Review Comment:
   Updated.



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3479)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=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=3479=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3479=false=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=3479=false=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=3479=false=CODE_SMELL)
 [6 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=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=3479=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3489)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=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=3489=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3489=false=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=3489=false=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=3489=false=CODE_SMELL)
 [0 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_coverage=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=3489=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1376391896


##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);

Review Comment:
   In the doc, ANY means 'If any of the arguments are null, return null', and 
ALL means 'returns null only if all arguments are null'.
   
   NullPolicy's class comment says STRICT and ANY are similar, and prefers to 
use STRICT whenever possible. I will change to use STRICT because HYPOT returns 
null only if any arguments are 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-unsubscr...@calcite.apache.org

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1376391896


##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);

Review Comment:
   In the doc, ANY means 'If any of the arguments are null, return null', and 
ALL means 'returns null only if all arguments are null'.
   
   NullPolicy's class comment says STRICT and ANY are similar, and prefers to 
use STRICT whenever possible. I will change to use STRICT because HYPOT returns 
NULLL only if any arguments are null.



##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);

Review Comment:
   In the doc, ANY means 'If any of the arguments are null, return null', and 
ALL means 'returns null only if all arguments are null'.
   
   NullPolicy's class comment says STRICT and ANY are similar, and prefers to 
use STRICT whenever possible. I will change to use STRICT because HYPOT returns 
NULL only if any arguments are 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-unsubscr...@calcite.apache.org

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1376391896


##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);

Review Comment:
   In the doc, ANY means 'If any of the arguments are null, return null', and 
ALL means 'returns null only if all arguments are null'.
   
   NullPolicy's class comment says STRICT and ANY are similar, and prefers to 
use STRICT. I will change to use STRICT.



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

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

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



Re: [PR] [CALCITE-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3481)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3481=false=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=3481=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3481=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3481=false=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=3481=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3481=false=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=3481=false=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=3481=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3481=false=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=3481=false=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=3481=false=CODE_SMELL)
 [4 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3481=false=CODE_SMELL)
   
   
[![82.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png
 
'82.4%')](https://sonarcloud.io/component_measures?id=apache_calcite=3481=new_coverage=list)
 [82.4% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3481=new_coverage=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=3481=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3481=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376077977


##
site/_docs/reference.md:
##
@@ -2649,7 +2649,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 * Similarly, `DATETIME(string)` returns a Calcite `TIMESTAMP`.
 
 | C | Operator syntax| Description
-|:- |:---|:---
+|: |:---|:---

Review Comment:
   This is unexpected. 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-unsubscr...@calcite.apache.org

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376921030


##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal string. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal string without leading zeros. For example, hex(10) 
returns 'A'

Review Comment:
   Thanks, I will refine it.



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

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

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



Re: [PR] [CALCITE-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-30 Thread via GitHub


mihaibudiu commented on code in PR #3481:
URL: https://github.com/apache/calcite/pull/3481#discussion_r1376911308


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -624,7 +624,7 @@ void testCastExactNumericLimits(CastType castType, 
SqlOperatorFixture f) {
 f.checkCastFails(numeric.minOverflowNumericString,
 type, LITERAL_OUT_OF_RANGE_MESSAGE, false, castType);
   } else {
-if (Bug.CALCITE_2539_FIXED) {
+if (numeric != Numeric.DECIMAL5_2 || Bug.CALCITE_2539_FIXED) {
   f.checkCastFails(numeric.maxOverflowNumericString,
   type, OUT_OF_RANGE_MESSAGE, true, castType);

Review Comment:
   This depended on a BUG_* which I couldn't locate anymore, so I improved the 
message and removed the reference to the BUG altogether.



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

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

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



Re: [PR] [CALCITE-5418] Fixing Nested Correlated Subquery Expansion [calcite]

2023-10-30 Thread via GitHub


jamesstarr closed pull request #2996: [CALCITE-5418] Fixing Nested Correlated 
Subquery Expansion
URL: https://github.com/apache/calcite/pull/2996


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

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

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



Re: [PR] [CALCITE-5863] incorrect validation for window with range and multiple order by [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3325)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=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=3325=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=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=3325=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=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=3325=false=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=3325=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3325=false=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=3325=false=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=3325=false=CODE_SMELL)
 [1 Code 
Smell](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=CODE_SMELL)
   
   
[![92.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png
 
'92.6%')](https://sonarcloud.io/component_measures?id=apache_calcite=3325=new_coverage=list)
 [92.6% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3325=new_coverage=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=3325=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3325=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-5863] incorrect validation for window with range and multiple order by [calcite]

2023-10-30 Thread via GitHub


itiels commented on code in PR #3325:
URL: https://github.com/apache/calcite/pull/3325#discussion_r1376722139


##
core/src/main/java/org/apache/calcite/sql/SqlWindow.java:
##
@@ -660,6 +664,12 @@ public boolean isAllowPartial() {
 }
   }
 
+  private boolean onlyLiteralBounds(@Nullable SqlNode lowerBound, @Nullable 
SqlNode upperBound) {

Review Comment:
   Naming is hard.. My thought was literal in the sense of `SqlLiteral`, but I 
can see why it's confusing. 
   Changed it to `onlySymbolBounds`. 
   



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

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

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



(calcite) branch main updated (e7ad459ee3 -> 782d327d24)

2023-10-30 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


 discard e7ad459ee3 [CALCITE-6052] SqlImplementor writes floating point 
literals as DECIMAL literals
 new 782d327d24 [CALCITE-6052] SqlImplementor writes REAL, FLOAT, or DOUBLE 
literals as DECIMAL literals

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (e7ad459ee3)
\
 N -- N -- N   refs/heads/main (782d327d24)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:



(calcite) 01/01: [CALCITE-6052] SqlImplementor writes REAL, FLOAT, or DOUBLE literals as DECIMAL literals

2023-10-30 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 782d327d24c04e2161102b22f8880204462befd4
Author: Mihai Budiu 
AuthorDate: Wed Oct 25 18:40:23 2023 -0700

[CALCITE-6052] SqlImplementor writes REAL, FLOAT, or DOUBLE literals as 
DECIMAL literals

Close apache/calcite#3472

Signed-off-by: Mihai Budiu 
---
 .../org/apache/calcite/rel/rel2sql/SqlImplementor.java  | 17 -
 .../calcite/rel/rel2sql/RelToSqlConverterTest.java  |  6 ++
 .../test/java/org/apache/calcite/test/PigRelOpTest.java |  2 +-
 site/_docs/reference.md | 11 ++-
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java 
b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
index f7fafd648e..c71dc6558d 100644
--- a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
+++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
@@ -1428,12 +1428,19 @@ public abstract class SqlImplementor {
   return SqlLiteral.createCharString((String) 
castNonNull(literal.getValue2()), POS);
 }
 case NUMERIC:
-case EXACT_NUMERIC:
-  return SqlLiteral.createExactNumeric(
-  castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+case EXACT_NUMERIC: {
+  if (SqlTypeName.APPROX_TYPES.contains(typeName)) {
+return SqlLiteral.createApproxNumeric(
+castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+  } else {
+return SqlLiteral.createExactNumeric(
+castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+  }
+}
 case APPROXIMATE_NUMERIC:
-  return SqlLiteral.createApproxNumeric(
-  castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+  // This case is currently unreachable, because the type family can never 
be
+  // APPROXIMATE_NUMERIC -- see the definition of SqlTypeName.
+  throw new AssertionError("Unreachable");
 case BOOLEAN:
   return 
SqlLiteral.createBoolean(castNonNull(literal.getValueAs(Boolean.class)),
   POS);
diff --git 
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java 
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index de7b08a6ab..ddfe9af0c6 100644
--- 
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ 
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -235,6 +235,12 @@ class RelToSqlConverterTest {
 sql("SELECT cast(null as binary)").withMysql().ok("SELECT NULL");
   }
 
+  @Test void testFloatingPointLiteral() {
+String query = "SELECT CAST(0.1E0 AS DOUBLE), CAST(0.1E0 AS REAL), 
CAST(0.1E0 AS DOUBLE)";
+String expected = "SELECT 1E-1, 1E-1, 1E-1";
+sql(query).withMysql().ok(expected);
+  }
+
   @Test void testGroupByBooleanLiteral() {
 String query = "select avg(\"salary\") from \"employee\" group by true";
 String expectedRedshift = "SELECT AVG(\"employee\".\"salary\")\n"
diff --git a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java 
b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
index f24bfdb818..0da4ea734c 100644
--- a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
+++ b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
@@ -232,7 +232,7 @@ class PigRelOpTest extends PigRelTestBase {
 final String sql = ""
 + "SELECT *\n"
 + "FROM scott.DEPT\n"
-+ "WHERE RAND() < 0.5";
++ "WHERE RAND() < 5E-1";
 pig(script).assertRel(hasTree(plan))
 .assertSql(is(sql));
   }
diff --git a/site/_docs/reference.md b/site/_docs/reference.md
index 0c1222fd09..eef28ad1cf 100644
--- a/site/_docs/reference.md
+++ b/site/_docs/reference.md
@@ -1156,10 +1156,11 @@ name will have been converted to upper case also.
 | SMALLINT| 2 byte signed integer | Range is -32768 to 32767
 | INTEGER, INT | 4 byte signed integer| Range is -2147483648 to 2147483647
 | BIGINT  | 8 byte signed integer | Range is -9223372036854775808 to 
9223372036854775807
-| DECIMAL(p, s) | Fixed point | Example: 123.45 or DECIMAL 
'123.45' is a DECIMAL(5, 2) value.
-| NUMERIC | Fixed point   |
-| REAL, FLOAT | 4 byte floating point | 6 decimal digits precision
-| DOUBLE  | 8 byte floating point | 15 decimal digits precision
+| DECIMAL(p, s) | Fixed point | Example: 123.45 and DECIMAL 
'123.45' are identical values, and have type DECIMAL(5, 2)
+| NUMERIC(p, s) | Fixed point | A synonym for DECIMAL
+| REAL| 4 byte floating point | 6 decimal digits precision; 
examples: CAST(1.2 AS REAL), CAST('Infinity' AS REAL)
+| 

(calcite) branch main updated: [CALCITE-6052] SqlImplementor writes floating point literals as DECIMAL literals

2023-10-30 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
 new e7ad459ee3 [CALCITE-6052] SqlImplementor writes floating point 
literals as DECIMAL literals
e7ad459ee3 is described below

commit e7ad459ee3a4b5af54603f47f3dee95fd4b137f1
Author: Mihai Budiu 
AuthorDate: Wed Oct 25 18:40:23 2023 -0700

[CALCITE-6052] SqlImplementor writes floating point literals as DECIMAL 
literals

Close apache/calcite#3472

Signed-off-by: Mihai Budiu 
---
 .../org/apache/calcite/rel/rel2sql/SqlImplementor.java  | 17 -
 .../calcite/rel/rel2sql/RelToSqlConverterTest.java  |  6 ++
 .../test/java/org/apache/calcite/test/PigRelOpTest.java |  2 +-
 site/_docs/reference.md | 11 ++-
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java 
b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
index f7fafd648e..c71dc6558d 100644
--- a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
+++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java
@@ -1428,12 +1428,19 @@ public abstract class SqlImplementor {
   return SqlLiteral.createCharString((String) 
castNonNull(literal.getValue2()), POS);
 }
 case NUMERIC:
-case EXACT_NUMERIC:
-  return SqlLiteral.createExactNumeric(
-  castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+case EXACT_NUMERIC: {
+  if (SqlTypeName.APPROX_TYPES.contains(typeName)) {
+return SqlLiteral.createApproxNumeric(
+castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+  } else {
+return SqlLiteral.createExactNumeric(
+castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+  }
+}
 case APPROXIMATE_NUMERIC:
-  return SqlLiteral.createApproxNumeric(
-  castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), 
POS);
+  // This case is currently unreachable, because the type family can never 
be
+  // APPROXIMATE_NUMERIC -- see the definition of SqlTypeName.
+  throw new AssertionError("Unreachable");
 case BOOLEAN:
   return 
SqlLiteral.createBoolean(castNonNull(literal.getValueAs(Boolean.class)),
   POS);
diff --git 
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java 
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index de7b08a6ab..ddfe9af0c6 100644
--- 
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ 
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -235,6 +235,12 @@ class RelToSqlConverterTest {
 sql("SELECT cast(null as binary)").withMysql().ok("SELECT NULL");
   }
 
+  @Test void testFloatingPointLiteral() {
+String query = "SELECT CAST(0.1E0 AS DOUBLE), CAST(0.1E0 AS REAL), 
CAST(0.1E0 AS DOUBLE)";
+String expected = "SELECT 1E-1, 1E-1, 1E-1";
+sql(query).withMysql().ok(expected);
+  }
+
   @Test void testGroupByBooleanLiteral() {
 String query = "select avg(\"salary\") from \"employee\" group by true";
 String expectedRedshift = "SELECT AVG(\"employee\".\"salary\")\n"
diff --git a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java 
b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
index f24bfdb818..0da4ea734c 100644
--- a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
+++ b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
@@ -232,7 +232,7 @@ class PigRelOpTest extends PigRelTestBase {
 final String sql = ""
 + "SELECT *\n"
 + "FROM scott.DEPT\n"
-+ "WHERE RAND() < 0.5";
++ "WHERE RAND() < 5E-1";
 pig(script).assertRel(hasTree(plan))
 .assertSql(is(sql));
   }
diff --git a/site/_docs/reference.md b/site/_docs/reference.md
index 0c1222fd09..eef28ad1cf 100644
--- a/site/_docs/reference.md
+++ b/site/_docs/reference.md
@@ -1156,10 +1156,11 @@ name will have been converted to upper case also.
 | SMALLINT| 2 byte signed integer | Range is -32768 to 32767
 | INTEGER, INT | 4 byte signed integer| Range is -2147483648 to 2147483647
 | BIGINT  | 8 byte signed integer | Range is -9223372036854775808 to 
9223372036854775807
-| DECIMAL(p, s) | Fixed point | Example: 123.45 or DECIMAL 
'123.45' is a DECIMAL(5, 2) value.
-| NUMERIC | Fixed point   |
-| REAL, FLOAT | 4 byte floating point | 6 decimal digits precision
-| DOUBLE  | 8 byte floating point | 15 decimal digits precision
+| DECIMAL(p, s) | Fixed point | Example: 123.45 and DECIMAL 
'123.45' are identical values, and have type DECIMAL(5, 2)
+| 

Re: [PR] [CALCITE-6052] SqlImplementor writes REAL, FLOAT, and DOUBLE literals as DECIMAL literals [calcite]

2023-10-30 Thread via GitHub


asfgit closed pull request #3472: [CALCITE-6052] SqlImplementor writes REAL, 
FLOAT, and DOUBLE literals as DECIMAL literals
URL: https://github.com/apache/calcite/pull/3472


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

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

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



Re: [PR] [CALCITE-129] Support recursive WITH queries [calcite]

2023-10-30 Thread via GitHub


asfgit closed pull request #3480: [CALCITE-129] Support recursive WITH queries
URL: https://github.com/apache/calcite/pull/3480


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

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

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



(calcite) branch main updated: [CALCITE-129] Support recursive WITH queries

2023-10-30 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
 new 0b12f60207 [CALCITE-129] Support recursive WITH queries
0b12f60207 is described below

commit 0b12f6020774f7203b5ad46109020319322a8e6f
Author: Hanumath Maduri 
AuthorDate: Thu Oct 19 13:26:44 2023 -0700

[CALCITE-129] Support recursive WITH queries

Close apache/calcite#3480
---
 core/src/main/codegen/templates/Parser.jj  |  10 +-
 .../java/org/apache/calcite/plan/RelOptUtil.java   |  22 +
 .../apache/calcite/runtime/CalciteResource.java|   6 +
 .../main/java/org/apache/calcite/sql/SqlKind.java  |   3 +
 .../main/java/org/apache/calcite/sql/SqlWith.java  |   4 +
 .../java/org/apache/calcite/sql/SqlWithItem.java   |  13 +-
 .../calcite/sql/validate/SqlValidatorImpl.java |  33 +-
 .../calcite/sql/validate/SqlWithItemTableRef.java  |  62 +++
 .../calcite/sql/validate/WithItemNamespace.java|  10 +-
 .../sql/validate/WithItemRecursiveNamespace.java   |  59 +++
 .../{WithScope.java => WithRecursiveScope.java}|  23 +-
 .../org/apache/calcite/sql/validate/WithScope.java |   6 +-
 .../sql2rel/RelStructuredTypeFlattener.java|  10 +
 .../apache/calcite/sql2rel/SqlToRelConverter.java  |  45 +-
 .../java/org/apache/calcite/tools/RelBuilder.java  |  36 +-
 .../calcite/runtime/CalciteResource.properties |   2 +
 .../apache/calcite/test/SqlToRelConverterTest.java |  10 +
 .../org/apache/calcite/test/SqlValidatorTest.java  |  68 +++
 .../test/enumerable/EnumerableRepeatUnionTest.java |  67 ++-
 .../apache/calcite/test/SqlToRelConverterTest.xml  |  22 +
 core/src/test/resources/sql/recursive_queries.iq   | 507 +
 site/_docs/algebra.md  |   4 -
 site/_docs/reference.md|   2 +-
 .../apache/calcite/sql/parser/SqlParserTest.java   |  52 +++
 24 files changed, 1013 insertions(+), 63 deletions(-)

diff --git a/core/src/main/codegen/templates/Parser.jj 
b/core/src/main/codegen/templates/Parser.jj
index f2fcb434bc..8dddb1a99c 100644
--- a/core/src/main/codegen/templates/Parser.jj
+++ b/core/src/main/codegen/templates/Parser.jj
@@ -3486,14 +3486,16 @@ SqlNodeList WithList() :
 {
 final Span s;
 final List list = new ArrayList();
+boolean recursive = false;
 }
 {
- { s = span(); }
-AddWithItem(list) (  AddWithItem(list) )*
+ [  { recursive = true; } ]{ s = span(); }
+AddWithItem(list, SqlLiteral.createBoolean(recursive, getPos()))
+(  AddWithItem(list, SqlLiteral.createBoolean(recursive, getPos())) 
)*
 { return new SqlNodeList(list, s.end(this)); }
 }
 
-void AddWithItem(List list) :
+void AddWithItem(List list, SqlLiteral recursive) :
 {
 final SqlIdentifier id;
 final SqlNodeList columnList;
@@ -3504,7 +3506,7 @@ void AddWithItem(List list) :
 ( columnList = ParenthesizedSimpleIdentifierList() | { columnList = null; 
} )
 
 definition = ParenthesizedExpression(ExprContext.ACCEPT_QUERY)
-{ list.add(new SqlWithItem(id.getParserPosition(), id, columnList, 
definition)); }
+{ list.add(new SqlWithItem(id.getParserPosition(), id, columnList, 
definition, recursive)); }
 }
 
 /**
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java 
b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 90dedd5cfa..e8c9a48809 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -3304,6 +3304,28 @@ public abstract class RelOptUtil {
 return createProject(projectFactory, child, 
Mappings.asListNonNull(mapping.inverse()));
   }
 
+  /** Returns the relational table node for {@code tableName} if it occurs 
within a
+   * relational expression {@code root} otherwise an empty option is returned. 
*/
+  public static @Nullable RelOptTable findTable(RelNode root, final String 
tableName) {
+try {
+  RelShuttle visitor = new RelHomogeneousShuttle() {
+@Override public RelNode visit(TableScan scan) {
+  final RelOptTable scanTable = scan.getTable();
+  final List qualifiedName = scanTable.getQualifiedName();
+  if (qualifiedName.get(qualifiedName.size() - 1).equals(tableName)) {
+throw new Util.FoundOne(scanTable);
+  }
+  return super.visit(scan);
+}
+  };
+  root.accept(visitor);
+  return null;
+} catch (Util.FoundOne e) {
+  Util.swallow(e, null);
+  return (RelOptTable) e.getNode();
+}
+  }
+
   /** Returns whether relational expression {@code target} occurs within a
* relational expression {@code ancestor}. */
   public static boolean contains(RelNode ancestor, final RelNode target) {
diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java 

Re: [PR] [CALCITE-5863] incorrect validation for window with range and multiple order by [calcite]

2023-10-30 Thread via GitHub


mihaibudiu commented on code in PR #3325:
URL: https://github.com/apache/calcite/pull/3325#discussion_r1376555417


##
core/src/main/java/org/apache/calcite/sql/SqlWindow.java:
##
@@ -660,6 +664,12 @@ public boolean isAllowPartial() {
 }
   }
 
+  private boolean onlyLiteralBounds(@Nullable SqlNode lowerBound, @Nullable 
SqlNode upperBound) {

Review Comment:
   This does not seem to check for what the compiler calls "literal", which is 
essentially a constant value.
   You are checking for specific kinds of bounds. So perhaps this function is 
not named properly.



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

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

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



Re: [PR] [CALCITE-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-30 Thread via GitHub


rubenada commented on code in PR #3481:
URL: https://github.com/apache/calcite/pull/3481#discussion_r1376517429


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -624,7 +624,7 @@ void testCastExactNumericLimits(CastType castType, 
SqlOperatorFixture f) {
 f.checkCastFails(numeric.minOverflowNumericString,
 type, LITERAL_OUT_OF_RANGE_MESSAGE, false, castType);
   } else {
-if (Bug.CALCITE_2539_FIXED) {
+if (numeric != Numeric.DECIMAL5_2 || Bug.CALCITE_2539_FIXED) {
   f.checkCastFails(numeric.maxOverflowNumericString,
   type, OUT_OF_RANGE_MESSAGE, true, castType);

Review Comment:
   I think the test does not fail because `OUT_OF_RANGE_MESSAGE` is currently 
defined as `(?s).*`, so any error message would be fine (and I think we could 
be a bit more specific and test an actually expected error thanks to your 
improvements).



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

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

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



Re: [PR] [CALCITE-6035] Unparse 'WITHIN GROUP' for BigQuery dialect to match BigQuery documentation [calcite]

2023-10-30 Thread via GitHub


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


##
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##
@@ -1294,6 +1294,19 @@ private static String toSql(RelNode root, SqlDialect 
dialect,
 .withPostgresql().ok(expectedPostgresql);
   }
 
+  @Test void testPercentileContWithinGroupClauseBigQuery() {
+final String query = "select percentile_cont(0.5) WITHIN GROUP (ORDER BY 
`product_class_id`)\n"
++ "from `foodmart`.`product`";
+final String expected = "SELECT PERCENTILE_CONT(0.5) OVER "

Review Comment:
   I think the PERCENTILE_CONT function for BigQuery must have two arguments. I 
think this expected query would probably fail since the column isn't passed to 
the function directly as an argument



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


mihaibudiu commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376452723


##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal string. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal string without leading zeros. For example, hex(10) 
returns 'A'

Review Comment:
   the explanation for shortened makes sense, but perhaps the documentation 
should say "with leading zeros removed"



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

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

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



Re: [PR] [CALCITE-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-30 Thread via GitHub


mihaibudiu commented on code in PR #3481:
URL: https://github.com/apache/calcite/pull/3481#discussion_r1376450983


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -624,7 +624,7 @@ void testCastExactNumericLimits(CastType castType, 
SqlOperatorFixture f) {
 f.checkCastFails(numeric.minOverflowNumericString,
 type, LITERAL_OUT_OF_RANGE_MESSAGE, false, castType);
   } else {
-if (Bug.CALCITE_2539_FIXED) {
+if (numeric != Numeric.DECIMAL5_2 || Bug.CALCITE_2539_FIXED) {
   f.checkCastFails(numeric.maxOverflowNumericString,
   type, OUT_OF_RANGE_MESSAGE, true, castType);

Review Comment:
   I will take a look to see what message is actually produced. If the message 
is wrong technically this test should fail, so something is fishy 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-unsubscr...@calcite.apache.org

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3489)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=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=3489=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3489=false=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=3489=false=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=3489=false=CODE_SMELL)
 [2 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_coverage=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=3489=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3489)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=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=3489=false=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=3489=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3489=false=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=3489=false=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=3489=false=CODE_SMELL)
 [2 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3489=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_coverage=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=3489=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3489=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-5826] Add FIND_IN_SET function (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3317:
URL: https://github.com/apache/calcite/pull/3317#discussion_r1376420977


##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1076,6 +1078,41 @@ public static int levenshtein(String string1, String 
string2) {
 return LEVENSHTEIN_DISTANCE.apply(string1, string2);
   }
 
+  /** SQL FIND_IN_SET(matchStr, textStr) function.
+   * Returns the index (1-based) of the given matchStr
+   * in the comma-delimited list textStr. Returns 0,
+   * if the matchStr is not found or if the matchStr
+   * contains a comma. */
+  public static @Nullable Integer findInSet(
+  @Nullable String matchStr,
+  @Nullable String textStr) {
+if (matchStr == null || textStr == null) {
+  return null;
+}
+if (matchStr.contains(String.valueOf(COMMA_DELIMITER))) {
+  return 0;
+}
+final int textStrLen = textStr.length();
+final int matchStrLen = matchStr.length();
+int n = 1;
+int lastCommaIndex = -1;
+for (int i = 0; i < textStrLen; i++) {
+  if (textStr.charAt(i) == COMMA_DELIMITER) {

Review Comment:
   Thanks for the advice. Current implementation is consistent with Spark's. 
For better code readability, change to use `split` is good, but some 
performance may be reduced since for loop can early exit when matched and 
`split` cannot.



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

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

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1376401349


##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);

Review Comment:
   `checkNull` only checks result value, so need to check type individually 
using `checkType`. Added test case for this.



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

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

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



Re: [PR] [CALCITE-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on PR #3481:
URL: https://github.com/apache/calcite/pull/3481#issuecomment-1785459589

   LGTM.


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

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

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1376401349


##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);

Review Comment:
   `checkNull` onlys check result, so need to check type individually using 
`checkType`. Added test case for this.



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

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

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1376395962


##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);
   defineMethod(TRUNC, BuiltInMethod.STRUNCATE.method, NullPolicy.STRICT);

Review Comment:
   Resolved.



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

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

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



Re: [PR] [CALCITE-6066] Add HYPOT function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1376391896


##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -664,6 +665,7 @@ Builder populate() {
   defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
   defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
   defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+  defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);

Review Comment:
   In the doc, ANY means 'If any of the arguments are null, return null', and 
ALL means 'returns null only if all arguments are null'.
   
   NullPolicy's class comment says STRICT and ANY are similar. And I checked 
that in codegen phase, STRICT and ANY is the same when generates the null 
condition, see more at `AbstractRexCallImplementor.getCondition`.
   
   BTW I find that NullPolicy.STRICT's comment is not very clear, it says 
'Returns null if and only if one of the arguments are null', which makes me 
confused that more arguments are null will do not return 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-unsubscr...@calcite.apache.org

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



Re: [PR] [CALCITE-5863] incorrect validation for window with range and multiple order by [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3325)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=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=3325=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=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=3325=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=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=3325=false=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=3325=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3325=false=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=3325=false=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=3325=false=CODE_SMELL)
 [2 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3325=false=CODE_SMELL)
   
   
[![92.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png
 
'92.6%')](https://sonarcloud.io/component_measures?id=apache_calcite=3325=new_coverage=list)
 [92.6% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3325=new_coverage=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=3325=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3325=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3479)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=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=3479=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3479=false=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=3479=false=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=3479=false=CODE_SMELL)
 [8 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=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=3479=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3479)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=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=3479=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3479=false=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=3479=false=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=3479=false=CODE_SMELL)
 [8 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=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=3479=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-5863] incorrect validation for window with range and multiple order by [calcite]

2023-10-30 Thread via GitHub


itiels commented on code in PR #3325:
URL: https://github.com/apache/calcite/pull/3325#discussion_r1376334494


##
core/src/main/java/org/apache/calcite/sql/SqlWindow.java:
##
@@ -610,8 +610,10 @@ public boolean isAllowPartial() {
   if (orderList.size() > 0) {
 // if order by is a compound list then range not allowed
 if (orderList.size() > 1 && !isRows()) {
-  throw validator.newValidationError(isRows,
-  RESOURCE.compoundOrderByProhibitsRange());
+  if (!onlyLiteralBounds(lowerBound, upperBound)) {

Review Comment:
   Added test case



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376089470


##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal varchar. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar. For example, hex('17') returns '11'

Review Comment:
   It would be better to change example to `hex(10) returns 'A'`.



##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal varchar. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar. For example, hex('17') returns '11'

Review Comment:
   Updated.



##
site/_docs/reference.md:
##
@@ -2768,7 +2771,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | m | TO_BASE64(string)  | Converts the *string* 
to base-64 encoded form and returns a encoded string
 | b m | FROM_BASE64(string)  | Returns the decoded 
result of a base-64 *string* as a string
 | b | TO_HEX(binary) | Converts *binary* into 
a hexadecimal varchar
-| b | FROM_HEX(varchar)  | Converts a 
hexadecimal-encoded *varchar* into bytes
+| b | FROM_HEX(varchar)  | Converts a 
hexadecimal-encoded *varchar* into bytes, throws exception if input is invalid 
hexadecimal-encoded *varchar*

Review Comment:
   Resolved.



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376314181


##
site/_docs/reference.md:
##
@@ -2649,7 +2649,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 * Similarly, `DATETIME(string)` returns a Calcite `TIMESTAMP`.
 
 | C | Operator syntax| Description
-|:- |:---|:---
+|: |:---|:---

Review Comment:
   Resolved.



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3479)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=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=3479=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3479=false=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=3479=false=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=3479=false=CODE_SMELL)
 [8 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=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=3479=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


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

   Kudos, SonarCloud Quality Gate passed!  [![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=3479)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=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=3479=false=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=3479=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3479=false=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=3479=false=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=3479=false=CODE_SMELL)
 [8 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3479=false=CODE_SMELL)
   
   
[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png
 
'100.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_coverage=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=3479=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3479=new_duplicated_lines_density=list)
   
   


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

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

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



Re: [PR] [CALCITE-5918] Add MAP function (enabled in Spark library) [calcite]

2023-10-30 Thread via GitHub


chucheng92 commented on PR #3459:
URL: https://github.com/apache/calcite/pull/3459#issuecomment-1785227201

   > LGTM, sorry for the delay! Congrats on becoming a committer!
   
   thanks tanner. you helped me to a lot. :)


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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376086886


##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal varchar. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar. For example, hex('17') returns '11'

Review Comment:
   A bigint value is stored using 8 bytes and will be converted into 16 
hexadecimal chars, and may contain leading zeros. In order to keep consistent 
with the behaviour of Hive and Spark, the leading zeros will be removed.



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376089470


##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal varchar. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar. For example, hex('17') returns '11'

Review Comment:
   It would be better to change example to `hex(10) returns 'a'`.



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

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

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



Re: [PR] [CALCITE-6041] MAP sub-query gives NullPointerException [calcite]

2023-10-30 Thread via GitHub


chucheng92 merged PR #3475:
URL: https://github.com/apache/calcite/pull/3475


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

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

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



(calcite) branch main updated: [CALCITE-6041] MAP sub-query gives NullPointerException

2023-10-30 Thread taoran
This is an automated email from the ASF dual-hosted git repository.

taoran pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
 new c5f3b8dcf1 [CALCITE-6041] MAP sub-query gives NullPointerException
c5f3b8dcf1 is described below

commit c5f3b8dcf14bd66d435a7fd3b6150ea1b3d3ccd7
Author: Ran Tao 
AuthorDate: Tue Oct 17 19:41:25 2023 +0800

[CALCITE-6041] MAP sub-query gives NullPointerException
---
 .../adapter/enumerable/EnumerableCollect.java  | 77 --
 .../java/org/apache/calcite/rel/core/Collect.java  |  1 +
 .../calcite/sql/fun/SqlMapQueryConstructor.java|  2 +-
 .../apache/calcite/sql/type/SqlTypeTransforms.java | 11 
 .../org/apache/calcite/util/BuiltInMethod.java |  1 +
 core/src/test/resources/sql/sub-query.iq   | 36 ++
 .../org/apache/calcite/test/SqlOperatorTest.java   | 34 ++
 7 files changed, 140 insertions(+), 22 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
 
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
index 64ead33167..d794dc6c4e 100644
--- 
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
+++ 
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableCollect.java
@@ -19,6 +19,7 @@ package org.apache.calcite.adapter.enumerable;
 import org.apache.calcite.linq4j.tree.BlockBuilder;
 import org.apache.calcite.linq4j.tree.Expression;
 import org.apache.calcite.linq4j.tree.Expressions;
+import org.apache.calcite.linq4j.tree.ParameterExpression;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.rel.RelNode;
@@ -88,39 +89,73 @@ public class EnumerableCollect extends Collect implements 
EnumerableRel {
 getRowType(),
 JavaRowFormat.LIST);
 
+final SqlTypeName collectionType = getCollectionType();
+
 // final Enumerable child = <>;
 // final Enumerable converted = child.select(<>);
-// final List list = converted.toList();
+// if collectionType is ARRAY or MULTISET: final List list = 
converted.toList();
+// if collectionType is MAP:   final Map map = 
converted.toMap();
 Expression child_ =
 builder.append(
 "child", result.block);
 
-RelDataType collectionComponentType =
-
requireNonNull(rowType().getFieldList().get(0).getType().getComponentType());
-RelDataType childRecordType = 
result.physType.getRowType().getFieldList().get(0).getType();
-
 Expression conv_ = child_;
-if (!SqlTypeUtil.sameNamedType(collectionComponentType, childRecordType)) {
-  // In the internal representation of multisets , every element must be a 
record. In case the
-  // result above is a scalar type we have to wrap it around a physical 
type capable of
-  // representing records. For this reason the following conversion is 
necessary.
-  // REVIEW zabetak January 7, 2019: If we can ensure that the input to 
this operator
-  // has the correct physical type (e.g., respecting the Prefer.ARRAY 
above)
-  // then this conversion can be removed.
-  conv_ =
-  builder.append(
-  "converted", result.physType.convertTo(child_, 
JavaRowFormat.ARRAY));
-}
+Expression collectionExpr;
+switch (collectionType) {
+case ARRAY:
+case MULTISET:
+  RelDataType collectionComponentType =
+  
requireNonNull(rowType().getFieldList().get(0).getType().getComponentType());
+  RelDataType childRecordType = 
result.physType.getRowType().getFieldList().get(0).getType();
+
+  if (!SqlTypeUtil.sameNamedType(collectionComponentType, 
childRecordType)) {
+// In the internal representation of multisets , every element must be 
a record. In case the
+// result above is a scalar type we have to wrap it around a physical 
type capable of
+// representing records. For this reason the following conversion is 
necessary.
+// REVIEW zabetak January 7, 2019: If we can ensure that the input to 
this operator
+// has the correct physical type (e.g., respecting the Prefer.ARRAY 
above)
+// then this conversion can be removed.
+conv_ =
+builder.append(
+"converted", result.physType.convertTo(child_, 
JavaRowFormat.ARRAY));
+  }
+
+  collectionExpr =
+  builder.append("list",
+  Expressions.call(conv_,
+  BuiltInMethod.ENUMERABLE_TO_LIST.method));
+  break;
+case MAP:
+  // Convert input 'Object[]' to MAP data, we don't specify a comparator, 
just
+  // keep the original order of this map. (the inner map is a 
LinkedHashMap)
+  ParameterExpression input = Expressions.parameter(Object.class, "input");
 
-Expression list_ =
-

Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376095312


##
site/_docs/reference.md:
##
@@ -2852,6 +2855,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b o p | TRANSLATE(expr, fromString, toString)  | Returns *expr* with all 
occurrences of each character in *fromString* replaced by its corresponding 
character in *toString*. Characters in *expr* that are not in *fromString* are 
not replaced
 | b | TRUNC(numeric1 [, numeric2 ])  | Truncates *numeric1* to 
optionally *numeric2* (if not specified 0) places right to the decimal point
 | q | TRY_CAST(value AS type)| Converts *value* to 
*type*, returning NULL if conversion fails
+| h s | UNHEX(varchar)   | Converts a 
hexadecimal-encoded *varchar* into bytes, returns NULL if input is invalid 
hexadecimal-encoded *varchar*

Review Comment:
   Due to UNHEX's behaviour is not the same as FROM_HEX when input is not a 
valid hexadecimal string, I'd like to keep the doc 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-unsubscr...@calcite.apache.org

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376089470


##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal varchar. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar. For example, hex('17') returns '11'

Review Comment:
   It would be better to change example to `hex('10') returns 'a'`.



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376086886


##
site/_docs/reference.md:
##
@@ -2739,6 +2739,9 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* 
according to the specified format *string*
 | s | GETBIT(value, position)| Equivalent to 
`BIT_GET(value, position)`
 | b o | GREATEST(expr [, expr ]*)| Returns the greatest of 
the expressions
+| h s | HEX(binary)  | Converts *binary* into 
a hexadecimal varchar. For example, hex(x'6162') returns '6162'
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar. For example, hex('17') returns '11'

Review Comment:
   The leading zeros will be removed to keep consistent with the behaviour of 
Hive and Spark.



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

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

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



Re: [PR] [CALCITE-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-30 Thread via GitHub


herunkang2018 commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1376077977


##
site/_docs/reference.md:
##
@@ -2649,7 +2649,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 * Similarly, `DATETIME(string)` returns a Calcite `TIMESTAMP`.
 
 | C | Operator syntax| Description
-|:- |:---|:---
+|: |:---|:---

Review Comment:
   This is unexpected. I will remove 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-unsubscr...@calcite.apache.org

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



Re: [PR] [CALCITE-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-30 Thread via GitHub


rubenada commented on code in PR #3481:
URL: https://github.com/apache/calcite/pull/3481#discussion_r1375882002


##
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##
@@ -367,28 +369,67 @@ public static List asList(double[] elements) {
   }
 
   /**
-   * Converts a number into a value of the type
-   * specified by this primitive.
+   * Check if a value after rounding falls within a specified range.
+   *
+   * @param value  Value to compare.
+   * @param minMinimum value allowed.
+   * @param maxMaximum value allowed.
+   */
+  static void checkRoundedRange(Number value, double min, double max) {
+double dbl = value.doubleValue();
+// The equivalent of DOWN rounding for BigDecimal
+dbl = dbl > 0 ? Math.floor(dbl) : Math.ceil(dbl);
+if (dbl < min || dbl > max) {
+  throw new ArithmeticException("Value " + value + " out of range");
+}
+  }
+
+  /**
+   * Converts a number into a value of the type specified by this primitive
+   * using the SQL CAST rules.  If the value conversion causes loss of 
significant digits,
+   * an exception is thrown.
*
* @param value  Value to convert.
-   * @return   The converted value, or null if the
-   *   conversion cannot be performed.
+   * @return   The converted value, or null if the type of the result is 
not a number.
*/
   public @Nullable Object numberValue(Number value) {
 switch (this) {
 case BYTE:
+  checkRoundedRange(value, Byte.MIN_VALUE, Byte.MAX_VALUE);
   return value.byteValue();
 case CHAR:
+  // No overflow checks for char values

Review Comment:
   I agree with @mihaibudiu , in the absence of clear specs, following the lead 
of Postgresql seems a good idea. I see the comment was improved to clarify this 
fact  



##
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##
@@ -367,28 +369,67 @@ public static List asList(double[] elements) {
   }
 
   /**
-   * Converts a number into a value of the type
-   * specified by this primitive.
+   * Check if a value after rounding falls within a specified range.
+   *
+   * @param value  Value to compare.
+   * @param minMinimum value allowed.
+   * @param maxMaximum value allowed.
+   */
+  static void checkRoundedRange(Number value, double min, double max) {
+double dbl = value.doubleValue();
+// The equivalent of DOWN rounding for BigDecimal
+dbl = dbl > 0 ? Math.floor(dbl) : Math.ceil(dbl);
+if (dbl < min || dbl > max) {
+  throw new ArithmeticException("Value " + value + " out of range");
+}
+  }
+
+  /**
+   * Converts a number into a value of the type specified by this primitive
+   * using the SQL CAST rules.  If the value conversion causes loss of 
significant digits,
+   * an exception is thrown.
*
* @param value  Value to convert.
-   * @return   The converted value, or null if the
-   *   conversion cannot be performed.
+   * @return   The converted value, or null if the type of the result is 
not a number.
*/
   public @Nullable Object numberValue(Number value) {
 switch (this) {
 case BYTE:
+  checkRoundedRange(value, Byte.MIN_VALUE, Byte.MAX_VALUE);
   return value.byteValue();
 case CHAR:
+  // No overflow checks for char values

Review Comment:
   I agree with @mihaibudiu , in the absence of clear specs, following the lead 
of Postgresql seems a good idea. I see the comment was improved to clarify this 
fact  



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

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

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



Re: [PR] [CALCITE-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-30 Thread via GitHub


rubenada commented on code in PR #3481:
URL: https://github.com/apache/calcite/pull/3481#discussion_r1375880333


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -624,7 +624,7 @@ void testCastExactNumericLimits(CastType castType, 
SqlOperatorFixture f) {
 f.checkCastFails(numeric.minOverflowNumericString,
 type, LITERAL_OUT_OF_RANGE_MESSAGE, false, castType);
   } else {
-if (Bug.CALCITE_2539_FIXED) {
+if (numeric != Numeric.DECIMAL5_2 || Bug.CALCITE_2539_FIXED) {
   f.checkCastFails(numeric.maxOverflowNumericString,
   type, OUT_OF_RANGE_MESSAGE, true, castType);

Review Comment:
   Thanks for the improvements @mihaibudiu . One last question, shouldn't we 
adjust the expected error message in these lines for the actual expected error 
message (instead of `OUT_OF_RANGE_MESSAGE`)?
   ```
if (numeric != Numeric.DECIMAL5_2 || Bug.CALCITE_2539_FIXED) {
 f.checkCastFails(numeric.maxOverflowNumericString,
   type, OUT_OF_RANGE_MESSAGE, true, castType);  // <-- *
 f.checkCastFails(numeric.minOverflowNumericString,
   type, OUT_OF_RANGE_MESSAGE, true, castType);  // <-- *
   }
   ```



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

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

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



Re: [PR] [CALCITE-129] Support recursive WITH queries [calcite]

2023-10-30 Thread via GitHub


julianhyde commented on code in PR #3480:
URL: https://github.com/apache/calcite/pull/3480#discussion_r1375732912


##
core/src/main/java/org/apache/calcite/sql/validate/WithItemRecursiveNameSpace.java:
##
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.sql.validate;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlBasicCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlWith;
+import org.apache.calcite.sql.SqlWithItem;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.Pair;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+/** Very similar to {@link WithItemNamespace} but created only for RECURSIVE 
queries. */
+class WithItemRecursiveNameSpace extends WithItemNamespace {
+  private final SqlWithItem withItem;
+  private final SqlWithItemTableRef withItemTableRef;
+  WithItemRecursiveNameSpace(SqlValidatorImpl validator,
+  SqlWithItem withItem,
+  @Nullable SqlNode enclosingNode) {
+super(validator, withItem, enclosingNode);
+this.withItem = withItem;
+this.withItemTableRef = new SqlWithItemTableRef(SqlParserPos.ZERO, 
withItem);
+  }
+
+  @Override public RelDataType getRowType() {
+if (rowType == null) {
+  SqlBasicCall call;
+  if (this.withItem.query.getKind() == SqlKind.WITH) {
+call = (SqlBasicCall) ((SqlWith) this.withItem.query).body;
+  } else {
+call = (SqlBasicCall) this.withItem.query;
+  }
+  // As this is a recursive query we only should evaluate left child for 
getting the rowType.
+  RelDataType leftChildType =
+  validator.getNamespaceOrThrow(
+  call.operand(0)).getRowType();
+  SqlNodeList columnList = withItem.columnList;
+  if (columnList == null || columnList.size() == 0) {
+// This should never happen but added to protect against the 
NullPointerException.
+return leftChildType;
+  }
+  final RelDataTypeFactory.Builder builder =
+  validator.getTypeFactory().builder();
+  Pair.forEach(SqlIdentifier.simpleNames(columnList),
+  leftChildType.getFieldList(),
+  (name, field) -> builder.add(name, field.getType()));
+  rowType = builder.build();

Review Comment:
   I agree, @macroguo-ghy. And I think that's the only remaining issue.
   
   @HanumathRao, I have created 
https://github.com/julianhyde/calcite/tree/129-with-recursive to address that 
one issue. Please take a look. If it's OK I will merge.



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

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

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