mihaibudiu commented on code in PR #3906:
URL: https://github.com/apache/calcite/pull/3906#discussion_r1715540834
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java:
##########
@@ -294,22 +294,41 @@ default boolean
shouldUseDoubleMultiplication(RelDataTypeFactory typeFactory,
int s1 = type1.getScale();
int s2 = type2.getScale();
- 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 six = Math.min(6, getMaxNumericScale());
+ int d = p1 - s1 + s2;
+ int scale = Math.max(six, s1 + p2 + 1);
+ int precision = d + scale;
+
+ // Rules from
+ //
https://learn.microsoft.com/en-us/sql/t-sql/data-types/precision-scale-and-length-transact-sql
+ // Rules reproduced here in case the web page goes away.
+ // Note that in SQL server getMaxNumericPrecision() == 38 and
+ // getMaxNumericScale() > 6.
+ // In multiplication and division operations, we need precision -
scale places to store
+ // the integral part of the result. The scale might be reduced using
the following rules:
+ //
+ // - The resulting scale is reduced to min(scale, 38 -
(precision-scale))
Review Comment:
I think that the existing tests cover these changes - as a proof I had to
change many types in the division result types in the validator and operator
tests. I also added a test for a type system with lower precision, which
motivated filing this bug in the first place.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
