You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "mbudiu-vmw (via GitHub)" <gi...@apache.org> on 2023/04/15 01:47:29 UTC
[GitHub] [calcite] mbudiu-vmw opened a new pull request, #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
mbudiu-vmw opened a new pull request, #3156:
URL: https://github.com/apache/calcite/pull/3156
This PR includes the fix in #3155 as well, because it depends on 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-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 #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1515136908
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3156)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&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] rubenada commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1515057411
> I have rebased. How does one run the style checkers locally before submission? I couldn't find this in the https://calcite.apache.org/develop/#contributing guidelines.
If I am not mistaken a local `gradlew build` should include the verification of the code style
--
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 #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1516780303
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3156)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&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] rubenada commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1512636732
@zabetak this PR needs [CALCITE-5650](https://issues.apache.org/jira/browse/CALCITE-5650); if I understand correctly, you plan to take care of that, so I'll pause the merge of the current PR until then, ok?
--
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] zabetak commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1512576670
@mbudiu-vmw Small tip: it's better to avoid force-pushes after a review has started cause it makes review history hard to follow; no big deal for this change but it can be a pain in larger changes.
--
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] mbudiu-vmw commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "mbudiu-vmw (via GitHub)" <gi...@apache.org>.
mbudiu-vmw commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1516725683
Changed the commit message.
--
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] rubenada commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1511941464
@mbudiu-vmw I think the PR makes sense. There are some style errors though, could you please take care of them?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] mbudiu-vmw commented on a diff in pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "mbudiu-vmw (via GitHub)" <gi...@apache.org>.
mbudiu-vmw commented on code in PR #3156:
URL: https://github.com/apache/calcite/pull/3156#discussion_r1169233493
##########
core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java:
##########
@@ -1776,8 +1776,9 @@ public static boolean isAtomic(RelDataType type) {
* type system. */
public static RelDataType getMaxPrecisionScaleDecimal(RelDataTypeFactory factory) {
int maxPrecision = factory.getTypeSystem().getMaxNumericPrecision();
+ int maxScale = factory.getTypeSystem().getMaxNumericScale();
// scale should not greater than precision.
- int scale = maxPrecision / 2;
+ int scale = Math.min(maxPrecision / 2, maxScale);
Review Comment:
Personally I think that the choice of maxPrecision/2 is rather arbitrary, but I didn't want to change too many things at once. This used to be maxScale, and that should always be smaller than maxPrecision.
--
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] mbudiu-vmw commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "mbudiu-vmw (via GitHub)" <gi...@apache.org>.
mbudiu-vmw commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1515043161
I have rebased. How does one run the style checkers locally before submission?
I couldn't find this in the https://calcite.apache.org/develop/#contributing guidelines.
--
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 #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1509467829
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3156)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3156&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3156&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3156&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] rubenada merged pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada merged PR #3156:
URL: https://github.com/apache/calcite/pull/3156
--
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] rubenada commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1515816824
LGTM.
Thanks @mbudiu-vmw for rebasing. Could you please amend the commit message to simply its ticket number and title: `[CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale` ?
--
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] rubenada commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1515033587
[CALCITE-5650](https://issues.apache.org/jira/browse/CALCITE-5650); has been merged (thanks @zabetak); @mbudiu-vmw could you please revisit this PR?
--
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] zabetak commented on pull request #3156: [CALCITE-5651] Inferred scale for decimal should not exceed maximum allowed scale
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on PR #3156:
URL: https://github.com/apache/calcite/pull/3156#issuecomment-1512648335
@rubenada Sounds good let's do that!
--
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