rubenada commented on code in PR #6523:
URL: https://github.com/apache/hive/pull/6523#discussion_r3612840149
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveTypeSystemImpl.java:
##########
@@ -189,4 +181,39 @@ public RelDataType deriveSumType(RelDataTypeFactory
typeFactory,
return argumentType;
}
+ /**
+ * Overridden because CALCITE-6464 changed the default behavior to match
MS-SQL-Server-style algorithm,
+ * which can cause a drop in the scale computation, hence a precision loss
in certain cases.
+ * We override this method to keep the "old behavior" (pre-CALCITE-6464); an
alternative could be not
+ * overridding it (and keep the new Calcite default MS-SQL-style
decimal-divide semantics), but that
+ * would lead to "regressions" (precision loss) and would require test
adjustments.
+ */
+ @Override
+ public RelDataType deriveDecimalDivideType(RelDataTypeFactory typeFactory,
+ RelDataType type1, RelDataType type2) {
+ if (SqlTypeUtil.isExactNumeric(type1) && SqlTypeUtil.isExactNumeric(type2)
&&
+ (SqlTypeUtil.isDecimal(type1) || SqlTypeUtil.isDecimal(type2))) {
+ // Java numeric will always have invalid precision/scale,
+ // use its default decimal precision/scale instead.
+ type1 = RelDataTypeFactoryImpl.isJavaType(type1) ?
typeFactory.decimalOf(type1) : type1;
+ type2 = RelDataTypeFactoryImpl.isJavaType(type2) ?
typeFactory.decimalOf(type2) : type2;
+ int p1 = type1.getPrecision();
+ int p2 = type2.getPrecision();
+ 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 precision = dout + scale;
+ assert precision <= maxNumericPrecision;
+ assert precision > 0;
+ return typeFactory.createSqlType(SqlTypeName.DECIMAL, precision, scale);
+ }
+ return null;
+ }
+
Review Comment:
Note that this is not the first overridden method here, we already have our
own definition for `deriveSumType`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
