[
https://issues.apache.org/jira/browse/CALCITE-6265?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17836187#comment-17836187
]
Ruben Q L edited comment on CALCITE-6265 at 4/11/24 1:45 PM:
-------------------------------------------------------------
FYI, this fix caused a few regressions on my project:
A) one test that used to return 1 row now returns 0 rows
B) a couple of tests that used to work fine now fail with a ClassCastException
on the dynamic code
After some investigation, I'd say A seems a real regression on Calcite, whereas
B could be arguably on my side; but I share this info because probably both
could happen to other projects if we release the next version with this patch
as it is.
A) The problem with the test that no longer returns the expected 1 row is as
follows:
- A table with a decimal field, there is one column with value {{BigDecimal
DECIMAL1 = new BigDecimal("299792.457999999985");}}
- A simple query: SELECT t.id FROM myTable t WHERE t.myDecimalField = ?
- I set the parameter value to DECIMAL1
- Execute the query, should get 1 row, it gets nothing.
The problem is that, before the patch, the dynamic code would simply "read" the
parameter value from the root context and use it, [but now with the new code
there is this cast logic into Number and back to
BigDecimal|https://github.com/apache/calcite/blob/e7c1f0c9baecef62289facf9aaa03bdff950b938/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java#L1396]
(unnecessary in this case, since everything is BigDecimal), which leads to the
following dynamic code:
{code}
(Number) root.get("?1") == null ? (java.math.BigDecimal) null :
java.math.BigDecimal.valueOf(((Number) root.get("?1")).longValue())
{code}
The problem is that doing {{BigDecimal.valueOf(myBigDecimal.longValue())}} does
not necessarily bring back the original value of myBigDecimal, since
BigDecimal#longValue can lose information, and I guess this is what is
happening here.
B) The issue with the ClassCastException (BigDecimal cannot be cast to Integer)
is also related to the new casting in dynamic code, but it is a bit more
subtle, and it seems related to some internal Hoisting that we have in place on
my project (transforming literals into parameters to increase the chances of
internal cache hits when converting a logical plan into physical plan). The
thing is, in the literal=>parameter conversion, we use
[RexLiteral#getValue3|https://github.com/apache/calcite/blob/e7c1f0c9baecef62289facf9aaa03bdff950b938/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L989]
(which as stated on its javadoc returns "the value of this literal, in the
form that the rex-to-lix translator wants it") to extract the value (and use it
as parameter value). So far it worked like charm. But with this change, the
ClassCastException arises because actually getValue3 of a RexLiteral with type
= BasicSqlTypeINTEGER, returns the value as a BigDecimal (as it is stored in
RexLiteral#value field), because its typeName is SqlTypeName#DECIMAL; btw this
RexLiteral is created by the standard behavior that we get if we create a
RexLiteral from an Integer via Calcite's RelBuilder#literal(IntegerValue).
was (Author: rubenql):
FYI, this fix caused a few regressions on my project:
A) one test that used to return 1 row now returns 0 rows
B) a couple of tests that used to work fine now fail with a ClassCastException
on the dynamic code
After some investigation, I'd say A seems a real regression on Calcite, whereas
B could be arguably on my side; but I share this info because probably both
could happen to other projects if we release the next version with this patch
as it is.
A) The problem with the test that no longer returns the expected 1 row is as
follows:
- A table with a decimal field, there is one column with value {{BigDecimal
DECIMAL1 = new BigDecimal("299792.457999999985");}}
- A simple query: SELECT t.id FROM myTable t WHERE t.myDecimalValue = ?
- I set the parameter value to DECIMAL1
- Execute the query, should get 1 row, it gets nothing.
The problem is that, before the patch, the dynamic code would simply "read" the
parameter value from the root context and use it, [but now with the new code
there is this cast logic into Number and back to
BigDecimal|https://github.com/apache/calcite/blob/e7c1f0c9baecef62289facf9aaa03bdff950b938/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java#L1396]
(unnecessary in this case, since everything is BigDecimal), which leads to the
following dynamic code:
{code}
(Number) root.get("?1") == null ? (java.math.BigDecimal) null :
java.math.BigDecimal.valueOf(((Number) root.get("?1")).longValue())
{code}
The problem is that doing {{BigDecimal.valueOf(myBigDecimal.longValue())}} does
not necessarily bring back the original value of myBigDecimal, since
BigDecimal#longValue can lose information, and I guess this is what is
happening here.
B) The issue with the ClassCastException (BigDecimal cannot be cast to Integer)
is also related to the new casting in dynamic code, but it is a bit more
subtle, and it seems related to some internal Hoisting that we have in place on
my project (transforming literals into parameters to increase the chances of
internal cache hits when converting a logical plan into physical plan). The
thing is, in the literal=>parameter conversion, we use
[RexLiteral#getValue3|https://github.com/apache/calcite/blob/e7c1f0c9baecef62289facf9aaa03bdff950b938/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L989]
(which as stated on its javadoc returns "the value of this literal, in the
form that the rex-to-lix translator wants it") to extract the value (and use it
as parameter value). So far it worked like charm. But with this change, the
ClassCastException arises because actually getValue3 of a RexLiteral with type
= BasicSqlTypeINTEGER, returns the value as a BigDecimal (as it is stored in
RexLiteral#value field), because its typeName is SqlTypeName#DECIMAL; btw this
RexLiteral is created by the standard behavior that we get if we create a
RexLiteral from an Integer via Calcite's RelBuilder#literal(IntegerValue).
> Type coercion is failing for numeric values in prepared statements
> ------------------------------------------------------------------
>
> Key: CALCITE-6265
> URL: https://issues.apache.org/jira/browse/CALCITE-6265
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Tim Nieradzik
> Assignee: Tim Nieradzik
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.37.0
>
>
> Given a column of type {{{}INT{}}}. When providing a {{short}} value as a
> placeholder in a prepared statement, a {{ClassCastException}} is thrown.
> h2. Test case
> {{final String sql =}}
> {{ "select \"empid\" from \"hr\".\"emps\" where \"empid\" in (?, ?)";}}{{
> CalciteAssert.hr()}}
> {{ .query(sql)}}
> {{ .consumesPreparedStatement(p -> {}}
> {{ p.setShort(1, (short) 100);}}
> {{ p.setShort(2, (short) 110);}}
> {{ })}}
> {{ .returnsUnordered("empid=100", "empid=110");}}
> h2. Stack trace
> {{java.lang.ClassCastException: class java.lang.Short cannot be cast to class
> java.lang.Integer (java.lang.Short and java.lang.Integer are in module
> java.base of loader 'bootstrap')}}
> {{ at Baz$1$1.moveNext(Unknown Source)}}
> {{ at
> org.apache.calcite.linq4j.Linq4j$EnumeratorIterator.<init>(Linq4j.java:679)}}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
