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