NobiGo commented on code in PR #3906:
URL: https://github.com/apache/calcite/pull/3906#discussion_r1715142727
##########
core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java:
##########
@@ -259,7 +259,7 @@ default boolean
shouldUseDoubleMultiplication(RelDataTypeFactory typeFactory,
* <li>Then the result type is a decimal with:
* <ul>
* <li>d = p1 - s1 + s2</li>
- * <li>s < max(6, s1 + p2 + 1)</li>
+ * <li>s = max(6, s1 + p2 + 1)</li>
Review Comment:
if getMaxNumericScale < 6, the value will not be 6. Should we keep it
consistent with the code?
##########
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:
Do we need to add another tests for these rules?
--
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]
