[CALCITE-6560] Add supportsNegativeScale in RelDataTypeSystem

apache · Sep 4, 2024 · fb40c86 · fb40c86
1 parent 8771e3f
commit fb40c86
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 16 deletions.
diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
@@ -5930,8 +5930,8 @@ SqlTypeNameSpec SqlTypeName2(Span s) :
 SqlTypeNameSpec SqlTypeName3(Span s) :
 {
     final SqlTypeName sqlTypeName;
-    int precision = -1;
-    int scale = -1;
+    int precision = RelDataType.PRECISION_NOT_SPECIFIED;
+    int scale = RelDataType.SCALE_NOT_SPECIFIED;
 }
 {
     (
@@ -5944,7 +5944,7 @@ SqlTypeNameSpec SqlTypeName3(Span s) :
         precision = UnsignedIntLiteral()
         [
             <COMMA>
-            scale = UnsignedIntLiteral()
+            scale = IntLiteral()
         ]
         <RPAREN>
     ]

diff --git a/core/src/main/java/org/apache/calcite/rel/type/DelegatingTypeSystem.java b/core/src/main/java/org/apache/calcite/rel/type/DelegatingTypeSystem.java
@@ -52,6 +52,14 @@ protected DelegatingTypeSystem(RelDataTypeSystem typeSystem) {
     return typeSystem.getMaxNumericPrecision();
   }
 
+  @Override public boolean supportsNegativeScale() {
+    boolean isSupportNegativeScale =  typeSystem.supportsNegativeScale();
+    if (isSupportNegativeScale) {
+      throw new AssertionError("For now, Calcite doesn't support negative scale");
+    }
+    return false;
+  }
+
   @Override public RoundingMode roundingMode() {
     return typeSystem.roundingMode();
   }

diff --git a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystem.java
@@ -62,6 +62,9 @@ public interface RelDataTypeSystem {
   /** Returns the maximum precision of a NUMERIC or DECIMAL type. */
   int getMaxNumericPrecision();
 
+  /** Returns whether the scale of a NUMERIC or DECIMAL type can be negative. */
+  boolean supportsNegativeScale();
+
   /** Returns the rounding behavior for numerical operations capable of discarding precision. */
   RoundingMode roundingMode();
 

diff --git a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystemImpl.java b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeSystemImpl.java
@@ -57,7 +57,7 @@ public abstract class RelDataTypeSystemImpl implements RelDataTypeSystem {
     case INTERVAL_SECOND:
       return SqlTypeName.MAX_INTERVAL_FRACTIONAL_SECOND_PRECISION;
     default:
-      return -1;
+      return RelDataType.SCALE_NOT_SPECIFIED;
     }
   }
 
@@ -114,7 +114,7 @@ public abstract class RelDataTypeSystemImpl implements RelDataTypeSystem {
       // (microseconds) per SQL99 part 2 section 6.1 syntax rule 30.
       return 0;
     default:
-      return -1;
+      return RelDataType.PRECISION_NOT_SPECIFIED;
     }
   }
 
@@ -162,6 +162,10 @@ public abstract class RelDataTypeSystemImpl implements RelDataTypeSystem {
     return 19;
   }
 
+  @Override public boolean supportsNegativeScale() {
+    return false;
+  }
+
   @Override public RoundingMode roundingMode() {
     return RoundingMode.DOWN;
   }

diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -930,6 +930,12 @@ ExInst<CalciteException> illegalArgumentForTableFunctionCall(String a0,
   @BaseMessage("Illegal arguments for 'FORMAT_NUMBER' function: negative decimal value not allowed")
   ExInst<CalciteException> illegalNegativeDecimalValue();
 
+  @BaseMessage("DECIMAL precision {0,number,#} must be between 1 and max precision {1,number,#}")
+  ExInst<CalciteException> invalidPrecisionForDecimalType(int precision, int maxPrecision);
+
+  @BaseMessage("DECIMAL scale {0,number,#} must be between greater than or equal to 0")
+  ExInst<CalciteException> invalidNegativeScaleForDecimalType(int scale);
+
   @BaseMessage("Illegal arguments: The length of the keys array {0,number,#} is not equal to the length of the values array {1,number,#} in MAP_FROM_ARRAYS function")
   ExInst<CalciteException> illegalArgumentsInMapFromArraysFunc(int arg0, int arg1);
 

diff --git a/core/src/main/java/org/apache/calcite/sql/SqlBasicTypeNameSpec.java b/core/src/main/java/org/apache/calcite/sql/SqlBasicTypeNameSpec.java
@@ -103,16 +103,16 @@ public SqlBasicTypeNameSpec(
   }
 
   public SqlBasicTypeNameSpec(SqlTypeName typeName, SqlParserPos pos) {
-    this(typeName, -1, -1, null, pos);
+    this(typeName, RelDataType.PRECISION_NOT_SPECIFIED, RelDataType.SCALE_NOT_SPECIFIED, null, pos);
   }
 
   public SqlBasicTypeNameSpec(SqlTypeName typeName, int precision, SqlParserPos pos) {
-    this(typeName, precision, -1, null, pos);
+    this(typeName, precision, RelDataType.SCALE_NOT_SPECIFIED, null, pos);
   }
 
   public SqlBasicTypeNameSpec(SqlTypeName typeName, int precision,
       String charSetName, SqlParserPos pos) {
-    this(typeName, precision, -1, charSetName, pos);
+    this(typeName, precision, RelDataType.SCALE_NOT_SPECIFIED, charSetName, pos);
   }
 
   public SqlBasicTypeNameSpec(SqlTypeName typeName, int precision,
@@ -196,10 +196,11 @@ public int getPrecision() {
     // NOTE jvs 15-Jan-2009:  earlier validation is supposed to
     // have caught these, which is why it's OK for them
     // to be assertions rather than user-level exceptions.
-    if ((precision >= 0) && (scale >= 0)) {
+    if ((precision != RelDataType.PRECISION_NOT_SPECIFIED)
+        && (scale != RelDataType.SCALE_NOT_SPECIFIED)) {
       assert sqlTypeName.allowsPrecScale(true, true);
       type = typeFactory.createSqlType(sqlTypeName, precision, scale);
-    } else if (precision >= 0) {
+    } else if (precision != RelDataType.PRECISION_NOT_SPECIFIED) {
       assert sqlTypeName.allowsPrecNoScale();
       type = typeFactory.createSqlType(sqlTypeName, precision);
     } else {

diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
@@ -32,6 +32,8 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 
+import static org.apache.calcite.util.Static.RESOURCE;
+
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -59,13 +61,13 @@ public SqlTypeFactoryImpl(RelDataTypeSystem typeSystem) {
   @Override public RelDataType createSqlType(
       SqlTypeName typeName,
       int precision) {
+    if (typeName.allowsScale()) {
+      return createSqlType(typeName, precision, typeName.getDefaultScale());
+    }
     final int maxPrecision = typeSystem.getMaxPrecision(typeName);
     if (maxPrecision >= 0 && precision > maxPrecision) {
       precision = maxPrecision;
     }
-    if (typeName.allowsScale()) {
-      return createSqlType(typeName, precision, typeName.getDefaultScale());
-    }
     assertBasic(typeName);
     assert (precision >= 0)
         || (precision == RelDataType.PRECISION_NOT_SPECIFIED);
@@ -88,6 +90,15 @@ public SqlTypeFactoryImpl(RelDataTypeSystem typeSystem) {
     if (maxPrecision >= 0 && precision > maxPrecision) {
       precision = maxPrecision;
     }
+    if (typeName == SqlTypeName.DECIMAL) {
+      if (precision != RelDataType.PRECISION_NOT_SPECIFIED && precision <= 0) {
+        throw RESOURCE.invalidPrecisionForDecimalType(precision, maxPrecision).ex();
+      }
+      if (scale != RelDataType.SCALE_NOT_SPECIFIED && scale < 0
+          && !typeSystem.supportsNegativeScale()) {
+        throw RESOURCE.invalidNegativeScaleForDecimalType(scale).ex();
+      }
+    }
     RelDataType newType =
         new BasicSqlType(typeSystem, typeName, precision, scale);
     newType = SqlTypeUtil.addCharsetAndCollation(newType, this);

diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
@@ -1107,12 +1107,13 @@ public static SqlDataTypeSpec convertTypeToSpec(RelDataType type,
     if (isAtomic(type) || isNull(type)
         || type.getSqlTypeName() == SqlTypeName.UNKNOWN
         || type.getSqlTypeName() == SqlTypeName.GEOMETRY) {
-      int precision = typeName.allowsPrec() ? type.getPrecision() : -1;
+      int precision =
+          typeName.allowsPrec() ? type.getPrecision() : RelDataType.PRECISION_NOT_SPECIFIED;
       // fix up the precision.
       if (maxPrecision > 0 && precision > maxPrecision) {
         precision = maxPrecision;
       }
-      int scale = typeName.allowsScale() ? type.getScale() : -1;
+      int scale = typeName.allowsScale() ? type.getScale() :  RelDataType.SCALE_NOT_SPECIFIED;
       if (maxScale > 0 && scale > maxScale) {
         scale = maxScale;
       }
@@ -1166,7 +1167,8 @@ public static SqlDataTypeSpec convertTypeToSpec(RelDataType type,
   public static SqlDataTypeSpec convertTypeToSpec(RelDataType type) {
     // TODO jvs 28-Dec-2004:  collation
     String charSetName = inCharFamily(type) ? type.getCharset().name() : null;
-    return convertTypeToSpec(type, charSetName, -1, -1);
+    return convertTypeToSpec(type, charSetName,
+        RelDataType.PRECISION_NOT_SPECIFIED, RelDataType.SCALE_NOT_SPECIFIED);
   }
 
   public static RelDataType createMultisetType(

diff --git a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -304,6 +304,8 @@ IllegalNegativePadLength=Second argument for LPAD/RPAD must not be negative
 IllegalEmptyPadPattern=Third argument (pad pattern) for LPAD/RPAD must not be empty
 IllegalNegativeSubstringLength=Substring error: negative substring length not allowed
 IllegalNegativeDecimalValue=Illegal arguments for 'FORMAT_NUMBER' function: negative decimal value not allowed
+InvalidPrecisionForDecimalType=DECIMAL precision {0,number,#} must be between 1 and max precision {1,number,#}
+InvalidNegativeScaleForDecimalType=DECIMAL scale {0,number,#} must be between greater than or equal to 0
 IllegalArgumentsInMapFromArraysFunc=Illegal arguments: The length of the keys array {0,number,#} is not equal to the length of the values array {1,number,#} in MAP_FROM_ARRAYS function
 TrimError=Trim error: trim character must be exactly 1 character
 InvalidTypesForArithmetic=Invalid types for arithmetic: {0} {1} {2}

diff --git a/core/src/test/java/org/apache/calcite/sql/type/RelDataTypeSystemTest.java b/core/src/test/java/org/apache/calcite/sql/type/RelDataTypeSystemTest.java
@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.sql.type;
 
+import org.apache.calcite.rel.type.DelegatingTypeSystem;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeSystemImpl;
@@ -36,6 +37,7 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Tests the inference of return types using {@code RelDataTypeSystem}.
@@ -143,6 +145,31 @@ static class Fixture extends SqlTypeFixture {
     final SqlTypeFactoryImpl customTypeFactory = new SqlTypeFactoryImpl(new CustomTypeSystem());
   }
 
+  @Test void testSupportNegativeScale() {
+    final SqlTypeFactoryImpl customTypeFactory =
+        new SqlTypeFactoryImpl(new DelegatingTypeSystem(new CustomTypeSystem()) {
+          @Override public boolean supportsNegativeScale() {
+            return true;
+          }
+        });
+    RelDataType dataType = customTypeFactory.createSqlType(SqlTypeName.DECIMAL, 10, -5);
+    assertEquals(SqlTypeName.DECIMAL, dataType.getSqlTypeName());
+    assertEquals(10, dataType.getPrecision());
+    assertEquals(-5, dataType.getScale());
+  }
+
+  @Test void testNotSupportNegativeScale() {
+    final SqlTypeFactoryImpl customTypeFactory =
+        new SqlTypeFactoryImpl(new DelegatingTypeSystem(new CustomTypeSystem()) {
+          @Override public boolean supportsNegativeScale() {
+            return false;
+          }
+        });
+    assertThrows(CalciteException.class, () ->
+        customTypeFactory.createSqlType(SqlTypeName.DECIMAL, 10, -5),
+        "For now, Calcite doesn't support negative scale");
+  }
+
   @Test void testDecimalAdditionReturnTypeInference() {
     final SqlTypeFactoryImpl f = new Fixture().typeFactory;
     RelDataType operand1 = f.createSqlType(SqlTypeName.DECIMAL, 10, 1);

diff --git a/core/src/test/resources/sql/misc.iq b/core/src/test/resources/sql/misc.iq
@@ -3182,4 +3182,41 @@ from (values 0) as t(x);
 EnumerableValues(tuples=[[{ true, false, true, false, true, false, true, false }]])
 !plan
 
+# [CALCITE-6560] Add supportsNegativeScale in RelDataTypeSystem
+
+!set outputformat csv
+
+# throw an exception when CAST target decimal type has illegal precision and scale parameter
+
+# precision is negative number
+values cast(15.3 as decimal(-1,4));
+Error while executing SQL "values cast(15.3 as decimal(-1,4))": parse failed: Encountered "-" at line 1, column 29.
+!error
+
+# precision is 0
+values cast(15.3 as decimal(0,4));
+DECIMAL precision 0 must be between 1 and max precision 19
+!error
+
+# scale is negative number
+values cast(15.3 as decimal(1,-1));
+DECIMAL scale -1 must be between greater than or equal to 0
+!error
+
+# when precision greater than the max precision, use the max precision replace the precision
+values cast(15.3 as decimal(1000, 4));
+EXPR$0
+15.3000
+!ok
+
+# scale and precision is equal
+values cast(0 as decimal(3,3));
+EXPR$0
+0.000
+!ok
+
+values cast(0 as decimal(0,0));
+DECIMAL precision 0 must be between 1 and max precision 19
+!error
+
 # End misc.iq