You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2023/01/16 23:46:55 UTC

[GitHub] [calcite] birschick-bq opened a new pull request, #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric

birschick-bq opened a new pull request, #3031:
URL: https://github.com/apache/calcite/pull/3031

   Resolve issue with thrown exception if literal is non-numeric.
   
   https://issues.apache.org/jira/browse/CALCITE-5483


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

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

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

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3031:
URL: https://github.com/apache/calcite/pull/3031#issuecomment-1384684549

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


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

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

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

[GitHub] [calcite] sonarcloud[bot] commented on pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3031:
URL: https://github.com/apache/calcite/pull/3031#issuecomment-1386252119

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


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

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

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

[GitHub] [calcite] birschick-bq commented on a diff in pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric

Posted by GitBox <gi...@apache.org>.
birschick-bq commented on code in PR #3031:
URL: https://github.com/apache/calcite/pull/3031#discussion_r1072879759


##########
core/src/main/java/org/apache/calcite/rel/rules/ProjectAggregateMergeRule.java:
##########
@@ -99,7 +100,8 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
               && operands.get(1).getKind() == SqlKind.CAST
               && ((RexCall) operands.get(1)).operands.get(0).getKind()
               == SqlKind.INPUT_REF
-              && operands.get(2).getKind() == SqlKind.LITERAL) {
+              && operands.get(2).getKind() == SqlKind.LITERAL
+              && operands.get(2).getType().getFamily() == SqlTypeFamily.NUMERIC) {

Review Comment:
   @clesaec 
   I agree with you on this. I believe there could be a conversion for `BigDecimal` from `TIMESTAMP`.



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

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

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

[GitHub] [calcite] birschick-bq closed pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric

Posted by GitBox <gi...@apache.org>.
birschick-bq closed pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
URL: https://github.com/apache/calcite/pull/3031


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

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

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

[GitHub] [calcite] birschick-bq commented on pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric

Posted by GitBox <gi...@apache.org>.
birschick-bq commented on PR #3031:
URL: https://github.com/apache/calcite/pull/3031#issuecomment-1387586998

   cc: @CassieLyu / @sunnie629


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

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

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

[GitHub] [calcite] clesaec commented on a diff in pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric

Posted by GitBox <gi...@apache.org>.
clesaec commented on code in PR #3031:
URL: https://github.com/apache/calcite/pull/3031#discussion_r1072284872


##########
core/src/main/java/org/apache/calcite/rel/rules/ProjectAggregateMergeRule.java:
##########
@@ -99,7 +100,8 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
               && operands.get(1).getKind() == SqlKind.CAST
               && ((RexCall) operands.get(1)).operands.get(0).getKind()
               == SqlKind.INPUT_REF
-              && operands.get(2).getKind() == SqlKind.LITERAL) {
+              && operands.get(2).getKind() == SqlKind.LITERAL
+              && operands.get(2).getType().getFamily() == SqlTypeFamily.NUMERIC) {

Review Comment:
   I wonder where the code `literal.getValueAs(BigDecimal.class)` (line 111) could work as [getValueAs method](https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L1029)  has no case for "clazz == BigDecimal" ... May be not related to this specific case, but it seems there is also an issue on RexLiteral class.



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

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

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