[
https://issues.apache.org/jira/browse/CALCITE-6464?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17865758#comment-17865758
]
Tim Grein edited comment on CALCITE-6464 at 7/14/24 9:24 AM:
-------------------------------------------------------------
Just double-checking: the javadoc of
`RelDataTypeSystem.deriveDecimalDivideType` states the following algorithm for
deriving the decimal type of a decimal division:
{code:java}
d = p1 - s1 + s2
s < max(6, s1 + p2 + 1)
p = d + s{code}
The inequality here is kinda confusing to me as the result of an inequality is
not a number, but a boolean. Should it be the following?
{code:java}
d = p1 - s1 + s2
s = max(6, s1 + p2 + 1)
p = d + s {code}
I don't have access to the original SQL standard from 2003, but this would
align at least with how MS SQL Server defines type derivation for decimal
division
[here|[https://learn.microsoft.com/en-us/sql/t-sql/data-types/precision-scale-and-length-transact-sql?view=sql-server-ver16].]
If so I propose to fix and follow the docs and adjust:
{code:java}
final int maxNumericPrecision = getMaxNumericPrecision();
int dout =
Math.min(
p1 - s1 + s2,
maxNumericPrecision);
int scale = Math.max(6, s1 + p2 + 1);
scale =
Math.min(
scale,
maxNumericPrecision - dout);
scale = Math.min(scale, getMaxNumericScale());
int precision = dout + scale; {code}
to
{code:java}
int scale = Math.max(6, s1 + p2 + 1);
int precision = p1 - s1 + s2 + scale;
precision = Math.min(precision, getMaxNumericPrecision());
scale = Math.min(scale, getMaxNumericScale()); {code}
This also fixes the described issue above, I added an explicit regression test
to check this.
WDYT?
was (Author: JIRAUSER306177):
Just double-checking: the javadoc of
`RelDataTypeSystem.deriveDecimalDivideType` states the following algorithm for
deriving the decimal division type:
{code:java}
d = p1 - s1 + s2
s < max(6, s1 + p2 + 1)
p = d + s{code}
The inequality here is kinda confusing to me as the result of an inequality is
not a number, but a boolean. Should it be the following?
{code:java}
d = p1 - s1 + s2
s = max(6, s1 + p2 + 1)
p = d + s {code}
I don't have access to the original SQL standard from 2003, but this would
align at least with how MS SQL Server defines type derivation for decimal
division
[here|[https://learn.microsoft.com/en-us/sql/t-sql/data-types/precision-scale-and-length-transact-sql?view=sql-server-ver16].]
If so I propose to fix and follow the docs and adjust:
{code:java}
final int maxNumericPrecision = getMaxNumericPrecision();
int dout =
Math.min(
p1 - s1 + s2,
maxNumericPrecision);
int scale = Math.max(6, s1 + p2 + 1);
scale =
Math.min(
scale,
maxNumericPrecision - dout);
scale = Math.min(scale, getMaxNumericScale());
int precision = dout + scale; {code}
to
{code:java}
int scale = Math.max(6, s1 + p2 + 1);
int precision = p1 - s1 + s2 + scale;
precision = Math.min(precision, getMaxNumericPrecision());
scale = Math.min(scale, getMaxNumericScale()); {code}
This also fixes the described issue above, I added an explicit regression test
to check this.
WDYT?
> Type inference for DECIMAL division seems incorrect
> ---------------------------------------------------
>
> Key: CALCITE-6464
> URL: https://issues.apache.org/jira/browse/CALCITE-6464
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.37.0
> Reporter: Mihai Budiu
> Assignee: Tim Grein
> Priority: Minor
>
> This bug surfaces if one uses a custom type system, e.g., where DECIMAL is
> limited to (28, 10).
> The problem is in RelDataTypeSystem.deriveDecimalDivideType.
> The JavaDoc of this function gives the algorithm for deriving the division
> result type.
> According to these rules, if you divide two numbers of type DECIMAL(28, 10),
> you should get a result with type DECIMAL(28, 10).
> But the actual implementation infers a type of DECIMAL(28, 0), which seems
> incorrect.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
