[GitHub] [calcite] danny0405 commented on a change in pull request #1563: [CALCITE-3435] Enable decimal modulus operation to allow numeric with non-zero scale

[GitBox <gi...@apache.org>]

danny0405 commented on a change in pull request #1563: [CALCITE-3435] Enable decimal modulus operation to allow numeric with non-zero scale
URL: https://github.com/apache/calcite/pull/1563#discussion_r343522096
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystemImpl.java
 ##########
 @@ -252,6 +253,74 @@ public boolean allowExtendedTrim() {
     return false;
   }
 
+  /**
+   * Infers the return type of a decimal modulus operation. Decimal modulus
+   * involves at least one decimal operand.
+   *
+   * <p>Rules:
+   *
+   * <ul>
+   * <li>Let p1, s1 be the precision and scale of the first operand</li>
+   * <li>Let p2, s2 be the precision and scale of the second operand</li>
+   * <li>Let p, s be the precision and scale of the result</li>
+   * <li>Let d be the number of whole digits in the result</li>
+   * <li>Then the result type is a decimal with:
+   *   <ul>
+   *   <li>s = max(s1, s2)</li>
+   *   <li>p = min(p1 - s1, p2 - s2) + max(s1, s2)</li>
+   *   </ul>
+   * </li>
+   * <li>p and s are capped at their maximum values</li>
+   * </ul>
+   *
+   * @param typeFactory typeFactory used to create output type
+   * @param type1 type of the first operand
+   * @param type2 type of the second operand
+   * @return the result type for a decimal modulus, or null if decimal
+   * modulus should not be applied to the operands.
+   */
+  @Override public RelDataType deriveDecimalModType(RelDataTypeFactory typeFactory,
+      RelDataType type1, RelDataType type2) {
+    if (SqlTypeUtil.isExactNumeric(type1)
+        && SqlTypeUtil.isExactNumeric(type2)) {
+      if (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();
+        // keep consistency with SQL standard
+        if (s1 == 0 && s2 == 0) {
+          return type2;
+        }
+
+        int scale = Math.max(s1, s2);
+        assert scale <= getMaxNumericScale();
+
+        int precision = Math.min(p1 - s1, p2 - s2) + Math.max(s1, s2);
+        precision = Math.min(precision, getMaxNumericPrecision());
+        assert precision > 0;
+
+        RelDataType ret;
+        ret = typeFactory.createSqlType(
+                SqlTypeName.DECIMAL,
+                precision,
+                scale);
 
 Review comment:
   You can return `typeFactory.createSqlType` directly.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services