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 2021/05/17 16:41:18 UTC
[GitHub] [calcite] amrishlal opened a new pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
amrishlal opened a new pull request #2415:
URL: https://github.com/apache/calcite/pull/2415
A NullPointerException is thrown in SqlNumericLiteral.isInteger(), due to "this.scale" being null. This can be reproduced by compiling SQL statement "SELECT * FROM testTable WHERE floatColumn > 1.7976931348623157E308".
A null check was added through CALCITE-4199 to fix the NullPointerException; however, the root cause is that scale and precision are not being properly set in SqlLiteral.createApproxNumeric function which is called to handle APPROX_NUMERIC_LITERAL token.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amrishlal closed pull request #2415: [CALCITE-4608] Fix NullPointerException in SqlNumericLiteral.isInteger()
Posted by GitBox <gi...@apache.org>.
amrishlal closed pull request #2415:
URL: https://github.com/apache/calcite/pull/2415
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-845704391
Hi seems a positive change, can you add a ut for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amrishlal commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
amrishlal commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-846114361
> Hi, seems a positive change, can you add a ut for this?
OK, will do.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amrishlal commented on pull request #2415: [CALCITE-4608] Fix NullPointerException in SqlNumericLiteral.isInteger()
Posted by GitBox <gi...@apache.org>.
amrishlal commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-855294477
Closing this PR based on discussion in CALCITE-4608. I will open a separate PR for modifications discussed in the Jira ticket.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amrishlal commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
amrishlal commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-847222570
> Would it be possible to have a unit test "showing the issue"? i.e. a unit test that throws a NPE with the current master code, but that works fine with this patch.
Hi, as I mentioned in the description, the NPE has already been fixed by CALCITE-4199 in master through a null check. This PR is making sure that scale and precision are being properly set. The test case that I added earlier validates that scale and precision are being properly set and also validates that NPE is no longer occurring.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amrishlal commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
amrishlal commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-847302101
> Use `assertThat(... is(...))` rather than `assertEquals`. People who use the latter tend to get the arguments the wrong way round. As you have.
Done.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
julianhyde commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-847277134
Use `assertThat(... is(...))` rather than `assertEquals`. People who use the latter tend to get the arguments the wrong way round. As you have.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-847117496
Would it be possible to have a unit test "showing the issue"? i.e. a unit test that throws a NPE with the current master code, but that works fine with this patch.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron edited a comment on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
Aaaaaaron edited a comment on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-845704391
Hi, seems a positive change, can you add a ut for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] hannerwang commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
hannerwang commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-847043541
I suggest that squash the commits to one so that the history can be brief.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amrishlal commented on pull request #2415: [CALCITE-4608] Set scale and precision in SqlLiteral.createApproxNumeric function.
Posted by GitBox <gi...@apache.org>.
amrishlal commented on pull request #2415:
URL: https://github.com/apache/calcite/pull/2415#issuecomment-846135154
@Aaaaaaron @julianhyde I have added the test case. Thanks.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org