Re: [PR] [CALCITE-5825] Add URL_ENCODE and URL_DECODE function (enabled in Spark library) [calcite]

2023-10-25 Thread via GitHub


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

   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=3318)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3318=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=3318=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3318=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=3318=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=3318=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3318=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=3318=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=3318=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3318=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=3318=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=3318=false=CODE_SMELL)
 [4 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=CODE_SMELL)
   
   
[![90.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png
 
'90.2%')](https://sonarcloud.io/component_measures?id=apache_calcite=3318=new_coverage=list)
 [90.2% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3318=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=3318=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3318=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-25 Thread via GitHub


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

   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-5825] Add URL_ENCODE and URL_DECODE function (enabled in Spark library) [calcite]

2023-10-25 Thread via GitHub


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


##
site/_docs/reference.md:
##
@@ -2854,6 +2854,8 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | UNIX_MILLIS(timestamp) | Returns the number of 
milliseconds since 1970-01-01 00:00:00
 | b | UNIX_SECONDS(timestamp)| Returns the number of 
seconds since 1970-01-01 00:00:00
 | b | UNIX_DATE(date)| Returns the number of 
days since 1970-01-01
+| s | URL_DECODE(string) | Decodes a *string* in 
'application/x-www-form-urlencoded' format using a specific encoding scheme

Review Comment:
   It will return original string when decoded error. I have fixed this and 
added the behaviour to doc.



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

To unsubscribe, e-mail: commits-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 library) [calcite]

2023-10-25 Thread via GitHub


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


##
site/_docs/reference.md:
##
@@ -2729,6 +2729,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | o | EXTRACT(xml, xpath, [, namespaces ])   | Returns the XML 
fragment of the element or elements matched by the XPath expression. The 
optional namespace value that specifies a default mapping or namespace mapping 
for prefixes, which is used when evaluating the XPath expression
 | o | EXISTSNODE(xml, xpath, [, namespaces ])| Determines whether 
traversal of a XML document using a specified xpath results in any nodes. 
Returns 0 if no nodes remain after applying the XPath traversal on the document 
fragment of the element or elements matched by the XPath expression. Returns 1 
if any nodes remain. The optional namespace value that specifies a default 
mapping or namespace mapping for prefixes, which is used when evaluating the 
XPath expression.
 | m | EXTRACTVALUE(xml, xpathExpr))  | Returns the text of the 
first text node which is a child of the element or elements matched by the 
XPath expression.
+| h s | FIND_IN_SET(string, stringArray) | Returns the index 
(1-based) of the given *string* in the comma-delimited *stringArray*. Returns 
0, if the given *string* was not found or if *string* contains a comma

Review Comment:
   I think we can add an example to clarify it, like `FIND_IN_SET('bc', 
'a,bc,def')` returns TRUE.



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

To unsubscribe, e-mail: commits-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-25 Thread via GitHub


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

   @mihaibudiu @macroguo-ghy Thanks for review, all comments resolved, please 
help review again if you have time.


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

To unsubscribe, e-mail: commits-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-25 Thread via GitHub


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

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


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

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

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



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

2023-10-25 Thread via GitHub


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


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -4520,22 +4520,87 @@ void testBitGetFunc(SqlOperatorFixture f, String 
functionName) {
 f.checkNull("to_hex(cast(null as varbinary))");
   }
 
+  @Test void testHex() {

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-5918] Add MAP function (enabled in Spark library) [calcite]

2023-10-25 Thread via GitHub


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

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


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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in `BuiltInMethod` to store all the methods. 
it occupy 'MAP' name before, so I can't continue to use this name as our spark 
map method name, so i changed it, then we can use 'MAP' as method name as you 
suggested for consistency in naming. (I set 'MAP_FUNCTION' in `BuiltInMethod` 
previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application/engine use java reflection to 
call this field. But is this as expected? 
   
   I'm ok if we need to reset spark MAP method to 'MAP_FUNCTION' name in 
`BuiltInMethod` and keep here unchanged. @tanclary what do you think?



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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372573530


##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
 }
   }
 
+  /**
+   * Operand type-checking strategy for a MAP function, it allows empty map.
+   */
+  private static class MapFunctionOperandTypeChecker
+  extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+  super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding 
callBinding,
+final boolean throwOnFailure) {
+  final List argTypes =
+  SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+  // allows empty map
+  if (argTypes.size() == 0) {
+return true;
+  }
+  // the size of map arg types must be even.
+  if (argTypes.size() % 2 > 0) {
+throw 
callBinding.newValidationError(RESOURCE.mapRequiresEvenArgCount());
+  }
+  final Pair<@Nullable RelDataType, @Nullable RelDataType> componentType =
+  getComponentTypes(
+  callBinding.getTypeFactory(), argTypes);
+  // check key type & value type
+  if (null == componentType.left || null == componentType.right) {
+if (throwOnFailure) {
+  throw 
callBinding.newValidationError(RESOURCE.needSameTypeParameter());
+}
+return false;
+  }
+  return true;
+}
+
+/**
+ * Extract the key type and value type of arg types.
+ */
+private static Pair<@Nullable RelDataType, @Nullable RelDataType> 
getComponentTypes(
+RelDataTypeFactory typeFactory,
+List argTypes) {
+  return Pair.of(
+  typeFactory.leastRestrictive(Util.quotientList(argTypes, 2, 0)),

Review Comment:
   > Can we add UT for `typeFactory.leastRestrictive`?
   > 
   > How about? `MAP('k1', 1, 'k2', 2.0, 'k3', '3')`
   
   yes, added it. thanks for reviewing.
   



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

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

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



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

2023-10-25 Thread via GitHub


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

   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-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-25 Thread via GitHub


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

   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-6065] Add HEX and UNHEX functions (enabled in Hive and Spark libraries) [calcite]

2023-10-25 Thread via GitHub


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

   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-5918] Add MAP function (enabled in Spark library) [calcite]

2023-10-25 Thread via GitHub


JiajunBernoulli commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372495642


##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -1221,6 +1225,53 @@ private static class MapFromEntriesOperandTypeChecker
 }
   }
 
+  /**
+   * Operand type-checking strategy for a MAP function, it allows empty map.
+   */
+  private static class MapFunctionOperandTypeChecker
+  extends SameOperandTypeChecker {
+
+MapFunctionOperandTypeChecker() {
+  super(-1);
+}
+
+@Override public boolean checkOperandTypes(final SqlCallBinding 
callBinding,
+final boolean throwOnFailure) {
+  final List argTypes =
+  SqlTypeUtil.deriveType(callBinding, callBinding.operands());
+  // allows empty map
+  if (argTypes.size() == 0) {
+return true;
+  }
+  // the size of map arg types must be even.
+  if (argTypes.size() % 2 > 0) {
+throw 
callBinding.newValidationError(RESOURCE.mapRequiresEvenArgCount());
+  }
+  final Pair<@Nullable RelDataType, @Nullable RelDataType> componentType =
+  getComponentTypes(
+  callBinding.getTypeFactory(), argTypes);
+  // check key type & value type
+  if (null == componentType.left || null == componentType.right) {
+if (throwOnFailure) {
+  throw 
callBinding.newValidationError(RESOURCE.needSameTypeParameter());
+}
+return false;
+  }
+  return true;
+}
+
+/**
+ * Extract the key type and value type of arg types.
+ */
+private static Pair<@Nullable RelDataType, @Nullable RelDataType> 
getComponentTypes(
+RelDataTypeFactory typeFactory,
+List argTypes) {
+  return Pair.of(
+  typeFactory.leastRestrictive(Util.quotientList(argTypes, 2, 0)),

Review Comment:
   Can we add UT for `typeFactory.leastRestrictive`?
   
   How about?
   `MAP('k1',  1, 'k2', 2.0, 'k3', '3')`



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

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

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



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

2023-10-25 Thread via GitHub


mihaibudiu commented on PR #3472:
URL: https://github.com/apache/calcite/pull/3472#issuecomment-1780284974

   I have applied the suggested changes and squashed the commits optimistically.
   But an audit of the code has uncovered the fact that DOUBLE, REAL, and FLOAT 
are treated widely inconsistently in the code. I have filed 
https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6074 to handle 
that. I hope that the documentation I updated it in this PR is correct - it was 
wrong before.


-- 
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-6024] ListSqlOperatorTable is inefficient [calcite]

2023-10-25 Thread via GitHub


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

   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=3483)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3483=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=3483=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3483=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=3483=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=3483=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3483=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=3483=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=3483=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3483=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=3483=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=3483=false=CODE_SMELL)
 [3 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3483=false=CODE_SMELL)
   
   
[![85.9%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png
 
'85.9%')](https://sonarcloud.io/component_measures?id=apache_calcite=3483=new_coverage=list)
 [85.9% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3483=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=3483=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3483=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-6024] ListSqlOperatorTable is inefficient [calcite]

2023-10-25 Thread via GitHub


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

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



[PR] [CALCITE-6024] ListSqlOperatorTable is inefficient [calcite]

2023-10-25 Thread via GitHub


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

   ListSqlOperatorTable is inefficient if it contains a large number of 
operators. It currently examines the operators one by one. 
ReflectiveSqlOperatorTable (and its subclass, SqlStdOperatorTable) builds a map 
of operators by name (actually two maps, one case-sensitive and one 
case-insensitive).
   
   ListSqlOperatorTable should do the same, at least in its immutable form.


-- 
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-25 Thread via GitHub


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


##
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
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar
+| h s | HEX(varchar) | Converts *varchar* into 
a hexadecimal varchar

Review Comment:
   Sure, I will add some examples to explain.



-- 
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-25 Thread via GitHub


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


##
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
+| h s | HEX(bigint)  | Converts *bigint* into 
a shortened hexadecimal varchar
+| h s | HEX(varchar) | Converts *varchar* into 
a hexadecimal varchar

Review Comment:
   Sure, I will add some example to explain.



-- 
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-5825] Add URL_ENCODE and URL_DECODE function (enabled in Spark library) [calcite]

2023-10-25 Thread via GitHub


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


##
site/_docs/reference.md:
##
@@ -2854,6 +2854,8 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b | UNIX_MILLIS(timestamp) | Returns the number of 
milliseconds since 1970-01-01 00:00:00
 | b | UNIX_SECONDS(timestamp)| Returns the number of 
seconds since 1970-01-01 00:00:00
 | b | UNIX_DATE(date)| Returns the number of 
days since 1970-01-01
+| s | URL_DECODE(string) | Decodes a *string* in 
'application/x-www-form-urlencoded' format using a specific encoding scheme

Review Comment:
   Thanks for remind, I will check 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-6038] Remove 'ORDER BY ... LIMIT n' when input has at most one row, n >= 1, and there is no 'OFFSET' clause [calcite]

2023-10-25 Thread via GitHub


asfgit closed pull request #3467: [CALCITE-6038] Remove 'ORDER BY ... LIMIT n' 
when input has at most one row, n >= 1, and there is no 'OFFSET' clause
URL: https://github.com/apache/calcite/pull/3467


-- 
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-6038] Remove 'ORDER BY ... LIMIT n' when input has at most one row, n >= 1, and there is no 'OFFSET' clause

2023-10-25 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 79f2f61dcc [CALCITE-6038] Remove 'ORDER BY ... LIMIT n' when input has 
at most one row, n >= 1, and there is no 'OFFSET' clause
79f2f61dcc is described below

commit 79f2f61dcca9ab57600d3723b147b3e05d25ecb2
Author: shenlang 
AuthorDate: Wed Oct 11 21:43:58 2023 +0800

[CALCITE-6038] Remove 'ORDER BY ... LIMIT n' when input has at most one 
row, n >= 1, and there is no 'OFFSET' clause

This change extends SortRemoveRedundantRule, which was added
in [CALCITE-5994].

Following [CALCITE-5940], which added SortMergeRule, rename
field LIMIT_MREGE to LIMIT_MERGE.

Close apache/calcite#3467
---
 .../org/apache/calcite/rel/rules/CoreRules.java|   2 +-
 .../apache/calcite/rel/rules/SortMergeRule.java|   2 +-
 .../calcite/rel/rules/SortRemoveRedundantRule.java | 117 ++---
 .../org/apache/calcite/test/RelOptRulesTest.java   |  62 +--
 .../org/apache/calcite/test/RelOptRulesTest.xml|  65 
 5 files changed, 201 insertions(+), 47 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java 
b/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java
index 395a04c3b3..77c19d58ab 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java
@@ -722,7 +722,7 @@ public class CoreRules {
 
   /** Rule that merge a {@link Sort} representing the Limit semantics and
* another {@link Sort} representing the Limit or TOPN semantics. */
-  public static final SortMergeRule LIMIT_MREGE =
+  public static final SortMergeRule LIMIT_MERGE =
   SortMergeRule.Config.LIMIT_MERGE.toRule();
 
   /** Rule that removes keys from a {@link Sort}
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/SortMergeRule.java 
b/core/src/main/java/org/apache/calcite/rel/rules/SortMergeRule.java
index b4a484e333..a69acb9206 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/SortMergeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/SortMergeRule.java
@@ -65,7 +65,7 @@ import org.immutables.value.Value;
  *
  *  In the future,we could also extend other sort merge logic in this rule.
  *
- * @see CoreRules#LIMIT_MREGE
+ * @see CoreRules#LIMIT_MERGE
  */
 @Value.Enclosing
 public class SortMergeRule
diff --git 
a/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java 
b/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java
index 18b3839163..766849a443 100644
--- 
a/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java
+++ 
b/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java
@@ -26,39 +26,66 @@ import org.immutables.value.Value;
 
 import java.util.Optional;
 
-/** Rule that removes redundant {@code Order By} or {@code Limit} when its 
input RelNode's
- * max row count is less than or equal to specified row count.All of them
- * are represented by {@link Sort}
+/**
+ * Rule that removes redundant {@code ORDER BY} or {@code LIMIT} when its input
+ * RelNode's maximum row count is less than or equal to specified row count.
+ * All of them are represented by {@link Sort}.
  *
- *  If a {@code Sort} is pure order by,and its offset is null,when its 
input RelNode's
- * max row count is less than or equal to 1,then we could remove the redundant 
sort.
+ * If a {@code Sort} is an {@code ORDER BY} with no {@code OFFSET}, and if
+ * input's maximum row count is less than or equal to 1, then the sort is
+ * redundant (because every relation with 1 or fewer rows is sorted), and the
+ * rule removes the redundant sort.
+ *
+ * For example:
  *
- *  For example:
  * {@code
- *  select max(totalprice) from orders order by 1}
- *  
+ * SELECT max(totalprice)
+ * FROM orders
+ * ORDER BY 1
+ * }
+ *
+ * could be converted to
  *
- *  could be converted to
  * {@code
- *  select max(totalprice) from orders}
- *  
+ * SELECT max(totalprice)
+ * FROM orders
+ * }
  *
- *  If a {@code Sort} is pure limit,and its offset is null, when its input
- * RelNode's max row count is less than or equal to the limit's fetch,then we 
could
- * remove the redundant sort.
+ * For example:
  *
- *  For example:
  * {@code
- * SELECT * FROM (VALUES 1,2,3,4,5,6) AS t1 LIMIT 10}
- * 
+ * SELECT count(*)
+ * FROM orders
+ * ORDER BY 1 LIMIT 10
+ * }
+ *
+ * could be converted to
+ *
+ * {@code
+ * SELECT count(*)
+ * FROM orders
+ * }
+ *
+ * If a {@code Sort} is a pure {@code LIMIT} (with no {@code ORDER BY} or
+ * {@code OFFSET}), and its input RelNode's maximum row count is less than or
+ * equal to the limit's fetch, the rule removes the redundant {@code LIMIT}.
+ *
+ * For example:
  *
- *  The above values max row count 

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

2023-10-25 Thread via GitHub


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

   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)
 [0 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3481=false=CODE_SMELL)
   
   
[![69.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png
 
'69.5%')](https://sonarcloud.io/component_measures?id=apache_calcite=3481=new_coverage=list)
 [69.5% 
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-5990] Explicit cast to numeric type doesn't check overflow [calcite]

2023-10-25 Thread via GitHub


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

   I have updated the PR and reenabled the test that was disabled.
   I have also discovered a lot of related broken tests in SqlOperatorTest, and 
I re-enabled many of them (but not all).
   I have filed another issue to look at the remaining broken tests further: 
https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6073
   I realize that there are a lot of things going in this PR, but hopefully it 
can make it for 1.36.


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

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

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



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

2023-10-25 Thread via GitHub


HanumathRao commented on PR #3480:
URL: https://github.com/apache/calcite/pull/3480#issuecomment-1780163695

   > Nice work @HanumathRao ! I have left some (minor) comments. I'm not the 
biggest expert on the parser and validator, so I'd prefer if someone else would 
take a look at that part. Thanks for adding such a thorough amount of tests.
   > 
   > I think there is one part of the documentation that needs to be updated in 
the PR: in `algebra.md`, in the "Recursive Queries" section, this paragraph 
should be removed :)
   > 
   > ```
   > Note that there is no support for recursive queries in the SQL layer yet
   > ([CALCITE-129](https://issues.apache.org/jira/browse/CALCITE-129));
   > the `WITH RECURSIVE` query above is only for illustrative purposes.
   > ```
   
   Thanks @rubenada for the review. I have addressed the review comments, 
please take a look when you get a chance.


-- 
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-25 Thread via GitHub


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

   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=3480)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3480=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=3480=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3480=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=3480=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=3480=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3480=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=3480=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=3480=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3480=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=3480=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=3480=false=CODE_SMELL)
 [10 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3480=false=CODE_SMELL)
   
   
[![85.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png
 
'85.2%')](https://sonarcloud.io/component_measures?id=apache_calcite=3480=new_coverage=list)
 [85.2% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3480=new_coverage=list)
  
   
[![4.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/5-16px.png
 
'4.5%')](https://sonarcloud.io/component_measures?id=apache_calcite=3480=new_duplicated_lines_density=list)
 [4.5% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3480=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-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in `BuiltInMethod` to store all the methods. 
it occupy 'MAP' name before, so I can't continue to use this name as our spark 
map method name, so i changed it, then we can use 'MAP' as method name as you 
suggested for consistency in naming. (I set MAP_FUNCTION in `BuiltInMethod` 
previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application/engine use java reflection to 
call this field. But is this as expected? 
   
   I'm ok if we need to reset spark MAP method to 'MAP_FUNCTION' name in 
`BuiltInMethod` and keep here unchanged. @tanclary what do you think?



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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in `BuiltInMethod` to store all the methods. 
it occupy 'MAP' name before, so I can't continue to use this name as our spark 
map method name, so i changed it, then we can use 'MAP' as method name as you 
suggested for consistency in naming. (I set MAP_FUNCTION in `BuiltInMethod` 
previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application/engine use java reflection to 
call this field. But is this as expected? 
   
   I'm ok if we need to reset spark MAP function to 'MAP_FUNCTION' name in 
`BuiltInMethod` and keep here unchanged. what do you think?



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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in `BuiltInMethod` to store all the 
functions. it occupy 'MAP' name before, so I can't continue to use this name as 
our spark map method name, so i changed it, then we can use 'MAP' as method 
name as you suggested for consistency in naming. (I set MAP_FUNCTION in 
`BuiltInMethod` previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application/engine use java reflection to 
call this field. But is this as expected? 
   
   I'm ok if we need to reset spark MAP function to 'MAP_FUNCTION' name in 
`BuiltInMethod` and keep here unchanged. what do you think?



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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in `BuiltInMethod` to store all the 
functions. it occupy 'MAP' name before, so I can't continue to use this name as 
our spark map method name, so i changed it, then we can use 'MAP' as method 
name as you suggested for consistency in naming. (I set MAP_FUNCTION in 
`BuiltInMethod` previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application/engine use java reflection to 
call this field. But is this as expected? 
   
   or should i reset spark MAP function to 'MAP_FUNCTION' name in 
`BuiltInMethod` and keep here unchanged? what do you think?



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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in `BuiltInMethod` to store all the 
functions. it occupy 'MAP' name before, so I can't continue to use this name as 
our spark map method name, so i changed it, then we can use 'MAP' as method 
name as you suggested for consistency in naming. (I set MAP_FUNCTION in 
`BuiltInMethod` previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application/engine use java reflection to 
call this field. But is this as expected? i'm not sure of it.
   
   or should i reset spark MAP function to 'MAP_FUNCTION' name in 
`BuiltInMethod` and keep here unchanged? what do you think?



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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in BuiltInMethod to store all the functions. 
it use 'MAP' name before, so I can't continue to use this name as our spark map 
method name as you suggested. (I set MAP_FUNCTION previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application that reflects the use of this 
field. 
   
   or should i reset spark MAP function to 'MAP_FUNCTION' name and keep here 
unchanged? what do you think?



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

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

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



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

2023-10-25 Thread via GitHub


chucheng92 commented on code in PR #3459:
URL: https://github.com/apache/calcite/pull/3459#discussion_r1372215059


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   because this is a static field in BuiltInMethod to store all the functions. 
it use 'MAP' name before, so I can't continue to use this name as our spark map 
method name as you suggested. (I set MAP_FUNCTION previously).  
   
   one disadvantage I can think of here is that it may cause compatibility 
issues? If there is an upper-layer application that reflects the use of this 
field. 
   
   or should i reset spark MAP function to 'MAP_FUNCTION' name? what do you 
think?



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

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

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



Re: [PR] [CALCITE-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure (follow-up) [calcite]

2023-10-25 Thread via GitHub


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


##
core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java:
##
@@ -99,6 +99,11 @@ String rewrite(String sql) throws SqlParseException {
 }
   }
 
+  @Override @Disabled("Runtime error message differs after parsing and 
unparsing")

Review Comment:
   @mihaibudiu  ooops, my bad, this looks like a copy-paste error (which does 
not have any impact due to the test being disabled), I meant to write:
   ```
   @Override @Disabled("Runtime error message differs after parsing and 
unparsing")
   void testBitAndFuncRuntimeFails() {
 super.testBitAndFuncRuntimeFails();
   }
   ```
   Feel free to correct this in the PR of 
[CALCITE-5990](https://issues.apache.org/jira/browse/CALCITE-5990).
   
   The reason I disabled it was that the runtime error message contains the SQL 
query, and it seems that after the parse+unparse, the SQL query was not exactly 
the same as the original one (e.g. newlines were added), so the actual error 
was not matching the expected one. 
   



-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure (follow-up) [calcite]

2023-10-25 Thread via GitHub


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


##
core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java:
##
@@ -99,6 +99,11 @@ String rewrite(String sql) throws SqlParseException {
 }
   }
 
+  @Override @Disabled("Runtime error message differs after parsing and 
unparsing")

Review Comment:
   @mihaibudiu  ooops, my bad, this looks like a copy-paste error (which does 
not have any impact due to the test being disabled), I meant to write:
   ```
   @Override @Disabled("Runtime error message differs after parsing and 
unparsing")
   void testBitAndFuncRuntimeFails() {
 super.testBitAndFuncRuntimeFails();
   }
   ```
   Feel free to correct this in the PR of 
[CALCITE-5990](https://issues.apache.org/jira/browse/CALCITE-5990).
   
   The reason I disabled it was that the runtime error message contains the SQL 
query, and it seems that after the parse+unparse, the SQL query was not exactly 
the same as the original one (e.g. newlines were added), so the actual error 
was not matching the expected one. 
   I was a bit on a hurry to re-stabilize master, and I did not have much time 
to dig into it. If you find a way to re-enable the test (within CALCITE-5990), 
please go ahead
   



-- 
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-6052] SqlImplementor writes REAL, FLOAT, and DOUBLE literals as DECIMAL literals [calcite]

2023-10-25 Thread via GitHub


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


##
site/_docs/reference.md:
##
@@ -2693,7 +2693,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | m p | CONCAT_WS(separator, str1 [, string ]*)  | Concatenates one or 
more strings, returns null only when separator is null, otherwise treats null 
arguments as empty strings
 | q | CONCAT_WS(separator, str1, str2 [, string ]*)  | Concatenates two or 
more strings, requires at least 3 arguments (up to 254), treats null arguments 
as empty strings
 | m | COMPRESS(string)   | Compresses a string 
using zlib compression and returns the result as a binary string
-| b | CONTAINS_SUBSTR(expression, string [ , 
json_scopejson_scope_value ]) | Returns whether *string* exists as a 
substring in *expression*. Optional *json_scope* argument specifies what scope 
to search if *expression* is in JSON format. Returns NULL if a NULL exists in 
*expression* that does not result in a match
+| b | CONTAINS_SUBSTR(expression, string [ , json_scope = json_scope_value 
]) | Returns true when *string* exists as a substring in *expression*. Optional 
*json_scope* argument specifies what scope to search if *expression* is in JSON 
format. Returns NULL if a NULL exists in *expression* that does not result in a 
match

Review Comment:
   I prefer the 'whether' formulation. The 'Returns true' formulation leaves 
the reader to infer under what conditions the function returns false.



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

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

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



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

2023-10-25 Thread via GitHub


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


##
site/_docs/reference.md:
##
@@ -1158,8 +1158,8 @@ name will have been converted to upper case also.
 | 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
+| REAL, FLOAT | 4 byte floating point | 6 decimal digits precision.  
Examples: 1.2E2, CAST('Infinity' AS REAL), CAST('-Infinity' AS FLOAT), 
CAST('NaN' AS FLOAT)

Review Comment:
   FLOAT is actually the same as DOUBLE. (It's complicated, but standard SQL 
allows variable-precision floating point, and but we only allow 8 bytes.) I 
think you should put FLOAT on a separate line saying 'Equivalent to DOUBLE'.
   
   No '.' at end of line.



-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure (follow-up) [calcite]

2023-10-25 Thread via GitHub


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


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -919,8 +919,8 @@ void testCastIntervalToInterval(CastType castType, 
SqlOperatorFixture f) {
   void testCastWithRoundingToScalar(CastType castType, SqlOperatorFixture f) {
 f.setFor(SqlStdOperatorTable.CAST, VmName.EXPAND);
 
-f.checkFails("cast(1.25 as int)", "INTEGER", true);
-f.checkFails("cast(1.25E0 as int)", "INTEGER", true);
+f.checkScalar("cast(1.25 as int)", 1, "INTEGER NOT NULL");

Review Comment:
   The new version of this test looks correct to me. I wonder why the original 
test was expected to fail.



##
core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java:
##
@@ -99,6 +99,11 @@ String rewrite(String sql) throws SqlParseException {
 }
   }
 
+  @Override @Disabled("Runtime error message differs after parsing and 
unparsing")

Review Comment:
   Something is weird here, because the super function called doesn't have the 
same name. 
   But since the test is disabled, it doesn't really matter.
   This fixture was created to find bugs in unparsing, and it's possible that 
it does find such a bug. I will investigate, and if that's the case I will file 
an issue for 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-25 Thread via GitHub


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

   I will update this PR to enable all the disabled test. Will send a new 
commit today.


-- 
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-avatica] branch main updated: [CALCITE-6034] Add isAutoIncrement and isGenerated args to MetaColumn constructor

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

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


The following commit(s) were added to refs/heads/main by this push:
 new 519d1ceeb [CALCITE-6034] Add isAutoIncrement and isGenerated args to 
MetaColumn constructor
519d1ceeb is described below

commit 519d1ceeb04cd99530bceb60c1a8e0966c413541
Author: TJ Banghart 
AuthorDate: Mon Mar 20 10:07:36 2023 -0700

[CALCITE-6034] Add isAutoIncrement and isGenerated args to MetaColumn 
constructor
---
 .../java/org/apache/calcite/avatica/MetaImpl.java  | 47 +-
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/avatica/MetaImpl.java 
b/core/src/main/java/org/apache/calcite/avatica/MetaImpl.java
index c41f8edfc..4eb6a2ebe 100644
--- a/core/src/main/java/org/apache/calcite/avatica/MetaImpl.java
+++ b/core/src/main/java/org/apache/calcite/avatica/MetaImpl.java
@@ -46,6 +46,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.Objects;
 
 /**
  * Basic implementation of {@link Meta}.
@@ -352,10 +353,13 @@ public abstract class MetaImpl implements Meta {
 public final String scopeTable = null;
 public final Short sourceDataType = null;
 @ColumnNoNulls
-public final String isAutoincrement = "";
+public final String isAutoincrement;
 @ColumnNoNulls
-public final String isGeneratedcolumn = "";
+public final String isGeneratedcolumn;
 
+// TODO: https://issues.apache.org/jira/browse/CALCITE-5549
+//  mark as deprecated once Calcite uses updated constructor.
+@SuppressWarnings("unused")
 public MetaColumn(
 String tableCat,
 String tableSchem,
@@ -370,6 +374,27 @@ public abstract class MetaImpl implements Meta {
 Integer charOctetLength,
 int ordinalPosition,
 String isNullable) {
+  this(tableCat, tableSchem, tableName, columnName, dataType, typeName, 
columnSize,
+  decimalDigits, numPrecRadix, nullable, charOctetLength, 
ordinalPosition, isNullable, "",
+  "");
+}
+
+public MetaColumn(
+String tableCat,
+String tableSchem,
+String tableName,
+String columnName,
+int dataType,
+String typeName,
+Integer columnSize,
+Integer decimalDigits,
+Integer numPrecRadix,
+int nullable,
+Integer charOctetLength,
+int ordinalPosition,
+String isNullable,
+String isAutoincrement,
+String isGeneratedcolumn) {
   this.tableCat = tableCat;
   this.tableSchem = tableSchem;
   this.tableName = tableName;
@@ -383,11 +408,29 @@ public abstract class MetaImpl implements Meta {
   this.charOctetLength = charOctetLength;
   this.ordinalPosition = ordinalPosition;
   this.isNullable = isNullable;
+  this.isAutoincrement = isAutoincrement;
+  this.isGeneratedcolumn = isGeneratedcolumn;
 }
 
 public String getName() {
   return columnName;
 }
+
+/** Returns a copy of this MetaColumn, overriding the value of {@code 
isAutoincrement}. */
+@SuppressWarnings("unused") // called from Calcite
+public MetaColumn withIsAutoincrement(String isAutoincrement) {
+  return new MetaColumn(tableCat, tableSchem, tableName, columnName, 
dataType, typeName,
+  columnSize, decimalDigits, numPrecRadix, nullable, charOctetLength, 
ordinalPosition,
+  isNullable, Objects.requireNonNull(isAutoincrement), 
isGeneratedcolumn);
+}
+
+/** Returns a copy of this MetaColumn, overriding the value of {@code 
isGeneratedcolumn}. */
+@SuppressWarnings("unused") // called from Calcite
+public MetaColumn withIsGeneratedcolumn(String isGeneratedcolumn) {
+  return new MetaColumn(tableCat, tableSchem, tableName, columnName, 
dataType, typeName,
+  columnSize, decimalDigits, numPrecRadix, nullable, charOctetLength, 
ordinalPosition,
+  isNullable, isAutoincrement, 
Objects.requireNonNull(isGeneratedcolumn));
+}
   }
 
   /** Metadata describing a table. */



Re: [PR] [CALCITE-6034] Add isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]

2023-10-25 Thread via GitHub


tjbanghart merged PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229


-- 
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-25 Thread via GitHub


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


##
core/src/main/java/org/apache/calcite/prepare/LixToRelTranslator.java:
##
@@ -97,7 +97,7 @@ public  RelNode translate(Queryable queryable) {
   public RelNode translate(Expression expression) {
 if (expression instanceof MethodCallExpression) {
   final MethodCallExpression call = (MethodCallExpression) expression;
-  BuiltInMethod method = BuiltInMethod.MAP.get(call.method);
+  BuiltInMethod method = BuiltInMethod.FUNCTIONS_MAPS.get(call.method);

Review Comment:
   why call it `FUNCTIONS_MAPS`?



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

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

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



Re: [PR] [CALCITE-5949] RexExecutable should return unchanged original expressions when it fails [calcite]

2023-10-25 Thread via GitHub


rubenada commented on PR #3390:
URL: https://github.com/apache/calcite/pull/3390#issuecomment-1779348238

   @libenchao , considering that we are approaching the RC deadline (it would 
be sad to not include this on it); in case @arkanovicz cannot respond in 24h, 
I'll create a separate PR (tagging him as co-author) to finalize this fix.


-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure (follow-up) [calcite]

2023-10-25 Thread via GitHub


rubenada merged PR #3482:
URL: https://github.com/apache/calcite/pull/3482


-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure (follow-up)

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

rubenql 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 345961a9d2 [CALCITE-5921] SqlOperatorFixture.checkFails and 
checkAggFails don't check runtime failure (follow-up)
345961a9d2 is described below

commit 345961a9d21f1f669a452571fe70520185cbb424
Author: rubenada 
AuthorDate: Wed Oct 25 13:19:03 2023 +0100

[CALCITE-5921] SqlOperatorFixture.checkFails and checkAggFails don't check 
runtime failure (follow-up)
---
 .../calcite/test/SqlOperatorUnparseTest.java   |  5 +++
 .../org/apache/calcite/test/SqlOperatorTest.java   | 43 +-
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git 
a/core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java 
b/core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java
index 13512d1aa4..49872ef988 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java
@@ -99,6 +99,11 @@ public class SqlOperatorUnparseTest extends 
CalciteSqlOperatorTest {
 }
   }
 
+  @Override @Disabled("Runtime error message differs after parsing and 
unparsing")
+  void testBitAndFuncRuntimeFails() {
+super.testContainsSubstrFunc();
+  }
+
   // Every test that is Disabled below corresponds to a bug.
   // These tests should just be deleted when the corresponding bugs are fixed.
 
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java 
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index a9a24cfe92..891df133c3 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -919,8 +919,8 @@ public class SqlOperatorTest {
   void testCastWithRoundingToScalar(CastType castType, SqlOperatorFixture f) {
 f.setFor(SqlStdOperatorTable.CAST, VmName.EXPAND);
 
-f.checkFails("cast(1.25 as int)", "INTEGER", true);
-f.checkFails("cast(1.25E0 as int)", "INTEGER", true);
+f.checkScalar("cast(1.25 as int)", 1, "INTEGER NOT NULL");
+f.checkScalar("cast(1.25E0 as int)", 1, "INTEGER NOT NULL");
 if (!f.brokenTestsEnabled()) {
   return;
 }
@@ -960,8 +960,8 @@ public class SqlOperatorTest {
   void testCastDecimalToDoubleToInteger(CastType castType, SqlOperatorFixture 
f) {
 f.setFor(SqlStdOperatorTable.CAST, VmName.EXPAND);
 
-f.checkFails("cast( cast(1.25 as double) as integer)", 
OUT_OF_RANGE_MESSAGE, true);
-f.checkFails("cast( cast(-1.25 as double) as integer)", 
OUT_OF_RANGE_MESSAGE, true);
+f.checkScalar("cast( cast(1.25 as double) as integer)", 1, "INTEGER NOT 
NULL");
+f.checkScalar("cast( cast(-1.25 as double) as integer)", -1, "INTEGER NOT 
NULL");
 if (!f.brokenTestsEnabled()) {
   return;
 }
@@ -1222,7 +1222,11 @@ public class SqlOperatorTest {
   "12:42:25.34", "TIME(2) NOT NULL");
 }
 
-f.checkFails("cast('nottime' as TIME)", BAD_DATETIME_MESSAGE, true);
+if (castType == CastType.CAST) {
+  f.checkFails("cast('nottime' as TIME)", BAD_DATETIME_MESSAGE, true);
+} else {
+  f.checkNull("cast('nottime' as TIME)");
+}
 f.checkScalar("cast('1241241' as TIME)", "72:40:12", "TIME(0) NOT NULL");
 f.checkScalar("cast('12:54:78' as TIME)", "12:55:18", "TIME(0) NOT NULL");
 f.checkScalar("cast('12:34:5' as TIME)", "12:34:05", "TIME(0) NOT NULL");
@@ -1287,7 +1291,11 @@ public class SqlOperatorTest {
   f.checkScalar("cast('1945-1-24 12:23:34.454' as TIMESTAMP)",
   "1945-01-24 12:23:34", "TIMESTAMP(0) NOT NULL");
 }
-f.checkFails("cast('nottime' as TIMESTAMP)", BAD_DATETIME_MESSAGE, true);
+if (castType == CastType.CAST) {
+  f.checkFails("cast('nottime' as TIMESTAMP)", BAD_DATETIME_MESSAGE, true);
+} else {
+  f.checkNull("cast('nottime' as TIMESTAMP)");
+}
 
 // date <-> string
 f.checkCastToString("DATE '1945-02-24'", null, "1945-02-24", castType);
@@ -1297,7 +1305,11 @@ public class SqlOperatorTest {
 f.checkScalar("cast(' 1945-2-4 ' as DATE)", "1945-02-04", "DATE NOT NULL");
 f.checkScalar("cast('  1945-02-24  ' as DATE)",
 "1945-02-24", "DATE NOT NULL");
-f.checkFails("cast('notdate' as DATE)", BAD_DATETIME_MESSAGE, true);
+if (castType == CastType.CAST) {
+  f.checkFails("cast('notdate' as DATE)", BAD_DATETIME_MESSAGE, true);
+} else {
+  f.checkNull("cast('notdate' as DATE)");
+}
 
 f.checkScalar("cast('52534253' as DATE)", "7368-10-13", "DATE NOT NULL");
 f.checkScalar("cast('1945-30-24' as DATE)", "1947-06-26", "DATE NOT NULL");
@@ -1407,12 +1419,20 @@ public class SqlOperatorTest {
 f.checkBoolean("cast('  trUe' as boolean)", true);
 f.checkBoolean("cast('  tr' || 'Ue' as boolean)", true);
 

Re: [PR] [CALCITE-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure (follow-up) [calcite]

2023-10-25 Thread via GitHub


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

   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=3482)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3482=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=3482=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3482=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=3482=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=3482=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3482=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=3482=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=3482=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3482=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=3482=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=3482=false=CODE_SMELL)
 [1 Code 
Smell](https://sonarcloud.io/project/issues?id=apache_calcite=3482=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=3482=new_coverage=list)
 [100.0% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3482=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=3482=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3482=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-25 Thread via GitHub


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

   hi, @tanclary Sorry to pin you, I have solved the comments. could you help 
to re-check it? 


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

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

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



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

2023-10-25 Thread via GitHub


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

   @mihaibudiu  do we need to update this PR to re-enable the test that was 
disabled on CALCITE-5921?


-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure [calcite]

2023-10-25 Thread via GitHub


rubenada commented on PR #3412:
URL: https://github.com/apache/calcite/pull/3412#issuecomment-1778902006

   Closing this one in favor of https://github.com/apache/calcite/pull/3471


-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure [calcite]

2023-10-25 Thread via GitHub


rubenada closed pull request #3412: [CALCITE-5921] 
SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure
URL: https://github.com/apache/calcite/pull/3412


-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure

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

rubenql 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 0bec957071 [CALCITE-5921] SqlOperatorFixture.checkFails and 
checkAggFails don't check runtime failure
0bec957071 is described below

commit 0bec957071468a2e54a22519290ac101a752fcad
Author: Mihai Budiu 
AuthorDate: Sat Oct 14 21:17:32 2023 -0700

[CALCITE-5921] SqlOperatorFixture.checkFails and checkAggFails don't check 
runtime failure

Signed-off-by: Mihai Budiu 
---
 .../apache/calcite/runtime/CalciteResource.java|  2 +-
 .../org/apache/calcite/runtime/SqlFunctions.java   | 25 +---
 .../calcite/runtime/CalciteResource.properties |  2 +-
 .../calcite/sql/test/SqlOperatorFixture.java   |  2 +-
 .../java/org/apache/calcite/sql/test/SqlTests.java | 12 +---
 .../calcite/test/SqlOperatorFixtureImpl.java   |  2 ++
 .../org/apache/calcite/test/SqlOperatorTest.java   | 34 +++---
 .../org/apache/calcite/test/SqlRuntimeTester.java  |  4 +--
 8 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java 
b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
index dabc84ba1f..ee03bfe959 100644
--- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
+++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -298,7 +298,7 @@ public interface CalciteResource {
   @BaseMessage("Date literal ''{0}'' out of range")
   ExInst dateLiteralOutOfRange(String a0);
 
-  @BaseMessage("Input arguments of {0} out of range: {1,number,#}; should be 
in the range of {2}")
+  @BaseMessage("Input arguments of {0} out of range: {1,number,#.#}; should be 
in the range of {2}")
   ExInst inputArgumentsOfFunctionOutOfRange(String a0, 
Number a1, String a2);
 
   @BaseMessage("String literal continued on same line")
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java 
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 8a0e04c6d1..4ac14126fa 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -860,17 +860,22 @@ public class SqlFunctions {
 assert map != null;
 Set keys = map.keySet();
 Collection values = map.values();
-switch (JsonScope.valueOf(jsonScope)) {
-case JSON_KEYS:
-  return keys.contains(substr);
-case JSON_KEYS_AND_VALUES:
-  return keys.contains(substr) || values.contains(substr);
-case JSON_VALUES:
-  return values.contains(substr);
-default:
-  throw new IllegalArgumentException("json_scope argument must be one of: 
\"JSON_KEYS\", "
-  + "\"JSON_VALUES\", \"JSON_KEYS_AND_VALUES\".");
+try {
+  switch (JsonScope.valueOf(jsonScope)) {
+  case JSON_KEYS:
+return keys.contains(substr);
+  case JSON_KEYS_AND_VALUES:
+return keys.contains(substr) || values.contains(substr);
+  case JSON_VALUES:
+return values.contains(substr);
+  default:
+break;
+  }
+} catch (IllegalArgumentException ignored) {
+  // Happens when jsonScope is not one of the legal enum values
 }
+throw new IllegalArgumentException("json_scope argument must be one of: 
\"JSON_KEYS\", "
+  + "\"JSON_VALUES\", \"JSON_KEYS_AND_VALUES\".");
   }
 
   /** SQL CONTAINS_SUBSTR(expr, substr) operator. */
diff --git 
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties 
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
index c1f28de6e8..5475f2dafd 100644
--- 
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
+++ 
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -102,7 +102,7 @@ OperandNotComparable=Operands {0} not comparable to each 
other
 TypeNotComparableEachOther=Types {0} not comparable to each other
 NumberLiteralOutOfRange=Numeric literal ''{0}'' out of range
 DateLiteralOutOfRange=Date literal ''{0}'' out of range
-InputArgumentsOfFunctionOutOfRange=Input arguments of {0} out of range: 
{1,number,#}; should be in the range of {2}
+InputArgumentsOfFunctionOutOfRange=Input arguments of {0} out of range: 
{1,number,#.#}; should be in the range of {2}
 StringFragsOnSameLine=String literal continued on same line
 AliasMustBeSimpleIdentifier=Table or column alias must be a simple identifier
 CharLiteralAliasNotValid=Expecting alias, found character literal
diff --git 
a/testkit/src/main/java/org/apache/calcite/sql/test/SqlOperatorFixture.java 
b/testkit/src/main/java/org/apache/calcite/sql/test/SqlOperatorFixture.java
index 5eb94dcbc1..3ae3a3e2bb 100644
--- a/testkit/src/main/java/org/apache/calcite/sql/test/SqlOperatorFixture.java
+++ 

Re: [PR] [CALCITE-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure [calcite]

2023-10-25 Thread via GitHub


rubenada merged PR #3471:
URL: https://github.com/apache/calcite/pull/3471


-- 
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-5921] SqlOperatorFixture.checkFails and checkAggFails don't check runtime failure [calcite]

2023-10-25 Thread via GitHub


rubenada commented on PR #3471:
URL: https://github.com/apache/calcite/pull/3471#issuecomment-177816

   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-129] Support recursive WITH queries [calcite]

2023-10-25 Thread via GitHub


rubenada commented on PR #3480:
URL: https://github.com/apache/calcite/pull/3480#issuecomment-1778712954

   Nice work @HanumathRao !  I have left some (minor) comments.
   I'm not the biggest expert on the parser and validator, so I'd prefer if 
someone else would take a look at that part.
   Thanks for adding such a thorough amount of tests.
   
   I think there is one part of the documentation that needs to be updated in 
the PR: in `algebra.md`, in the "Recursive Queries", this paragraph should be 
removed :)
   ```
   Note that there is no support for recursive queries in the SQL layer yet
   ([CALCITE-129](https://issues.apache.org/jira/browse/CALCITE-129));
   the `WITH RECURSIVE` query above is only for illustrative purposes.
   ```


-- 
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-25 Thread via GitHub


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


##
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableRepeatUnionTest.java:
##
@@ -70,6 +81,47 @@ class EnumerableRepeatUnionTest {
 .returnsOrdered("i=1", "i=2", "i=3", "i=4", "i=5", "i=6", "i=7", 
"i=8", "i=9", "i=10");
   }
 
+  @Test void testGenerateNumbers2UsingSql() {
+CalciteAssert.that()
+.query("WITH RECURSIVE aux(i) AS (\n"
++ " VALUES (0)"
++ " UNION "
++ " SELECT MOD((i+1), 10) FROM aux WHERE i < 10"
++ "  )"
++ "   SELECT * FROM aux\n")
+.returnsOrdered("I=0", "I=1", "I=2", "I=3", "I=4", "I=5", "I=6", 
"I=7", "I=8", "I=9");
+  }
+
+  @Test void testGenerateNumbers2UsingSqlCheckPlan() {
+CalciteAssert.that()
+.with(CalciteConnectionProperty.LEX, Lex.JAVA)
+.with(CalciteConnectionProperty.FORCE_DECORRELATE, false)
+.withSchema("s", new ReflectiveSchema(new HierarchySchema()))
+.withHook(Hook.PLANNER, (Consumer) planner -> {

Review Comment:
   Do we really need the Hook.PLANNER for this test? It adds/removes 
join-related rules, but the query does not have any join



-- 
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-25 Thread via GitHub


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


##
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##
@@ -3774,6 +3791,48 @@ protected RelRoot convertQueryRecursive(SqlNode query, 
boolean top,
 }
   }
 
+  public static boolean hasATableScanWithName(RelNode root, final String 
recurTableName) {

Review Comment:
   There is a RelOptTableFinder class inside RelBuider which seems to have the 
same purpose. Currently it is private static, but perhaps it could be moved to 
another localion (RelOptUtil?) as a public element, and reuse the same code 
here and in RelBuilder.



-- 
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-25 Thread via GitHub


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


##
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableRepeatUnionTest.java:
##
@@ -45,6 +45,17 @@
  */
 class EnumerableRepeatUnionTest {
 
+  @Test void testGenerateNumbersUsingSql() {
+CalciteAssert.that()
+.query("WITH RECURSIVE delta(n) AS (\n"
++ "  VALUES (1)\n"
++ " UNION ALL\n"

Review Comment:
   More info about how RECURSIVE UNION [ALL] works internally can be found here:
   
https://www.postgresql.org/docs/current/queries-with.html#QUERIES-WITH-RECURSIVE
   Calcite implementation was heavily inspired by this postgresql description.
   



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

To unsubscribe, e-mail: commits-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-25 Thread via GitHub


macroguo-ghy commented on code in PR #3479:
URL: https://github.com/apache/calcite/pull/3479#discussion_r1371217305


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -4520,22 +4520,87 @@ void testBitGetFunc(SqlOperatorFixture f, String 
functionName) {
 f.checkNull("to_hex(cast(null as varbinary))");
   }
 
+  @Test void testHex() {

Review Comment:
   Could be helpful to leave a comment to link jira case.
   For example 
   ```
   /** Test case for
* https://issues.apache.org/jira/browse/CALCITE-4861;>[CALCITE-4861]
* Optimization of chained CAST calls leads to unexpected behavior. */
   ```



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