[
https://issues.apache.org/jira/browse/CALCITE-3142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16882691#comment-16882691
]
Feng Zhu commented on CALCITE-3142:
-----------------------------------
Hi [~vladimirsitnikov] [~zabetak] [~julianhyde], I opened an initial PR to
demonstrate the approach for this kind of issues.
I need your help to see whether it is in a right direction.
*Root Cause:*
It is not safe to implement a RexCall in current translator's BlockBuilder,
even all its operands have be translated with IS_NULL.
Current implementation relies on BlockBuilder's optimization phase to inline
these unsafe operations, bringing potential risks like NPE.
*Approach:*
We first use another translator (called "notNullTranslator") with a new
constructed BlockBuilder to keep this piece of code.
After that, we wrap this code with conditions and add it back.
We use "*_if_*" expression for conditional clause. For example, the new
generated code for this jira is:
{code:java}
public Object current() {
final Object[] current = (Object[]) inputEnumerator.current();
final Integer inp0_ = (Integer) current[0];
final Integer inp1_ = (Integer) current[1];
java.math.BigDecimal _call_result = (java.math.BigDecimal) null;
if (!(inp0_ == null || inp1_ == null)) {
final java.math.BigDecimal v1 = new java.math.BigDecimal(inp0_.intValue() /
inp1_.intValue());
_call_result = org.apache.calcite.runtime.SqlFunctions.sround(v1, 2);
}
final java.math.BigDecimal _call_result0 = _call_result;
return _call_result0;
}
{code}
The whole codegen process is shown in the figure below.
!newcodegen.png!
*TODO:*
This approach ensures the code correctness but introduces many patterns that
need to be optimized in BlockBuilder.
If you agree, I will summarize thses patterns and open separated Jira tasks.
> An NPE when rounding a nullable numeric
> ---------------------------------------
>
> Key: CALCITE-3142
> URL: https://issues.apache.org/jira/browse/CALCITE-3142
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.20.0
> Reporter: Muhammad Gelbana
> Assignee: Feng Zhu
> Priority: Major
> Labels: pull-request-available
> Attachments: newcodegen.png
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The following query throws a NPE in the generated code because it assumes the
> divided value to be an initialized Java object (Not null), which is fine for
> the first row, but not for the second.
> {code:sql}
> SELECT ROUND(CAST((X/Y) AS NUMERIC), 2) FROM (VALUES (1, 2), (NULLIF(5, 5),
> NULLIF(5, 5))) A(X, Y){code}
> If I modify the query a little bit, it runs ok:
> – No casting
> {code:sql}
> SELECT ROUND((X/Y), 2) FROM (VALUES (1, 2), (NULLIF(5, 5), NULLIF(5, 5)))
> A(X, Y){code}
> – No rounding
> {code:sql}
> SELECT (X/Y)::NUMERIC FROM (VALUES (1, 2), (NULLIF(5, 5), NULLIF(5, 5))) A(X,
> Y){code}
> +This is the optimized generated code+
> {code:java}
> final Object[] current = (Object[]) inputEnumerator.current();
> final Integer inp0_ = (Integer) current[0];
> final Integer inp1_ = (Integer) current[1];
> final java.math.BigDecimal v1 = new java.math.BigDecimal(
> inp0_.intValue() / inp1_.intValue()); // <<< NPE
> return inp0_ == null || inp1_ == null ? (java.math.BigDecimal) null :
> org.apache.calcite.runtime.SqlFunctions.sround(v1, 2);{code}
> +This is the non-optimized one+
> {code:java}
> final Object[] current = (Object[]) inputEnumerator.current();
> final Integer inp0_ = (Integer) current[0];
> final boolean inp0__unboxed = inp0_ == null;
> final Integer inp1_ = (Integer) current[1];
> final boolean inp1__unboxed = inp1_ == null;
> final boolean v = inp0__unboxed || inp1__unboxed;
> final int inp0__unboxed0 = inp0_.intValue(); // <<< NPE
> final int inp1__unboxed0 = inp1_.intValue(); // <<< NPE
> final int v0 = inp0__unboxed0 / inp1__unboxed0;
> final java.math.BigDecimal v1 = new java.math.BigDecimal(
> v0);
> final java.math.BigDecimal v2 = v ? (java.math.BigDecimal) null :
> org.apache.calcite.runtime.SqlFunctions.sround(v1, 2);
> return v2;{code}
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
