You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by ch...@apache.org on 2020/08/06 06:36:35 UTC

[calcite] branch master updated: [CALCITE-4081] Round-tripping a DECIMAL literal throws validation error

This is an automated email from the ASF dual-hosted git repository.

chunwei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new 63c1667  [CALCITE-4081] Round-tripping a DECIMAL literal throws validation error
63c1667 is described below

commit 63c1667459d594cf4761e794b5f882a49a9ae197
Author: Chunwei Lei <ch...@gmail.com>
AuthorDate: Wed Jul 29 11:56:07 2020 +0800

    [CALCITE-4081] Round-tripping a DECIMAL literal throws validation error
---
 .../java/org/apache/calcite/rex/RexBuilder.java    | 11 +++++++++++
 .../org/apache/calcite/sql/type/SqlTypeUtil.java   | 23 ++++++++++++++++++++++
 .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 15 ++++++++++++++
 .../org/apache/calcite/rex/RexBuilderTest.java     |  8 ++++++--
 .../apache/calcite/test/SqlToRelConverterTest.java |  2 +-
 .../apache/calcite/test/SqlToRelConverterTest.xml  |  2 +-
 core/src/test/resources/sql/misc.iq                |  8 ++++----
 7 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
index 092bb5b..760c1b5 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
@@ -648,6 +648,12 @@ public class RexBuilder {
         throw new AssertionError(toType);
       }
     }
+
+    if (toType.getSqlTypeName() == SqlTypeName.DECIMAL) {
+      final BigDecimal decimalValue = (BigDecimal) value;
+      return SqlTypeUtil.isValidDecimalValue(decimalValue, toType);
+    }
+
     return true;
   }
 
@@ -952,6 +958,11 @@ public class RexBuilder {
       o = ((TimestampString) o).round(p);
       break;
     }
+    if (type.getSqlTypeName() == SqlTypeName.DECIMAL && !SqlTypeUtil
+        .isValidDecimalValue((BigDecimal) o, type)) {
+      throw new IllegalArgumentException(
+          "Cannot convert " + o + " to " + type  + " due to overflow");
+    }
     return new RexLiteral(o, type, typeName);
   }
 
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
index 3ba355c..7b7d44b 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
@@ -46,6 +46,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
 
+import java.math.BigDecimal;
 import java.nio.charset.Charset;
 import java.util.AbstractList;
 import java.util.ArrayList;
@@ -1679,4 +1680,26 @@ public abstract class SqlTypeUtil {
     return typeFactory.createStructType(
         type.getFieldList().subList(fieldsCnt - numToKeep, fieldsCnt));
   }
+
+  /**
+   * Returns whether the decimal value is valid for the type. For example, 1111.11 is not
+   * valid for DECIMAL(3, 1) since it overflows.
+   *
+   * @param value Value of literal
+   * @param toType Type of the literal
+   * @return whether the value is valid for the type
+   */
+  public static boolean isValidDecimalValue(BigDecimal value, RelDataType toType) {
+    if (value == null) {
+      return true;
+    }
+    switch (toType.getSqlTypeName()) {
+    case DECIMAL:
+      final int intDigits = value.precision() - value.scale();
+      final int maxIntDigits = toType.getPrecision() - toType.getScale();
+      return intDigits <= maxIntDigits;
+    default:
+      return true;
+    }
+  }
 }
diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index 26cae12..9378bf2 100644
--- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -5137,6 +5137,21 @@ class RelToSqlConverterTest {
     sql(expected).exec();
   }
 
+  @Test void testCastDecimalOverflow() {
+    final String query =
+        "SELECT CAST('11111111111111111111111111111111.111111' AS DECIMAL(38,6)) AS \"num\" from \"product\"";
+    final String expected =
+        "SELECT CAST('11111111111111111111111111111111.111111' AS DECIMAL(19, 6)) AS \"num\"\n"
+            + "FROM \"foodmart\".\"product\"";
+    sql(query).ok(expected);
+
+    final String query2 =
+        "SELECT CAST(1111111 AS DECIMAL(5,2)) AS \"num\" from \"product\"";
+    final String expected2 =
+        "SELECT CAST(1111111 AS DECIMAL(5, 2)) AS \"num\"\nFROM \"foodmart\".\"product\"";
+    sql(query2).ok(expected2);
+  }
+
   @Test void testCastInStringIntegerComparison() {
     final String query = "select \"employee_id\" "
         + "from \"foodmart\".\"employee\" "
diff --git a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
index 49b5e6b..d75758b 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
@@ -21,6 +21,7 @@ import org.apache.calcite.rel.core.CorrelationId;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
+import org.apache.calcite.rel.type.RelDataTypeSystemImpl;
 import org.apache.calcite.sql.SqlCollation;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.type.BasicSqlType;
@@ -590,8 +591,11 @@ class RexBuilderTest {
 
   /** Tests {@link RexBuilder#makeExactLiteral(java.math.BigDecimal)}. */
   @Test void testBigDecimalLiteral() {
-    final RelDataTypeFactory typeFactory =
-        new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+    final RelDataTypeFactory typeFactory = new SqlTypeFactoryImpl(new RelDataTypeSystemImpl() {
+      @Override public int getMaxPrecision(SqlTypeName typeName) {
+        return 38;
+      }
+    });
     final RexBuilder builder = new RexBuilder(typeFactory);
     checkBigDecimalLiteral(builder, "25");
     checkBigDecimalLiteral(builder, "9.9");
diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index d560dc1..741be09 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -2634,7 +2634,7 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
   }
 
   @Test void testReduceConstExpr() {
-    final String sql = "select sum(case when 'y' = 'n' then ename else 1 end) from emp";
+    final String sql = "select sum(case when 'y' = 'n' then ename else 0.1 end) from emp";
     sql(sql).ok();
   }
 
diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 3b45f50..35026af 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -3735,7 +3735,7 @@ LogicalProject(DEPTNO=[$7])
         <Resource name="plan">
             <![CDATA[
 LogicalAggregate(group=[{}], EXPR$0=[SUM($0)])
-  LogicalProject($f0=[1:DECIMAL(19, 19)])
+  LogicalProject($f0=[0.1:DECIMAL(19, 19)])
     LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
         </Resource>
diff --git a/core/src/test/resources/sql/misc.iq b/core/src/test/resources/sql/misc.iq
index 0980e04..029525c 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/core/src/test/resources/sql/misc.iq
@@ -1716,7 +1716,7 @@ EnumerableCalc(expr#0=[{inputs}], expr#1=[123:BIGINT], EXPR$0=[$t1])
 !plan
 
 # Cast an integer literal to a decimal; note: the plan does not contain CAST
-values cast('123.45' as decimal(4, 2));
+values cast('123.45' as decimal(5, 2));
 +--------+
 | EXPR$0 |
 +--------+
@@ -1725,12 +1725,12 @@ values cast('123.45' as decimal(4, 2));
 (1 row)
 
 !ok
-EnumerableCalc(expr#0=[{inputs}], expr#1=[123.45:DECIMAL(4, 2)], EXPR$0=[$t1])
+EnumerableCalc(expr#0=[{inputs}], expr#1=[123.45:DECIMAL(5, 2)], EXPR$0=[$t1])
   EnumerableValues(tuples=[[{ 0 }]])
 !plan
 
 # Cast a character literal to a decimal; note: the plan does not contain CAST
-values cast('123.45' as decimal(4, 2));
+values cast('123.45' as decimal(5, 2));
 +--------+
 | EXPR$0 |
 +--------+
@@ -1739,7 +1739,7 @@ values cast('123.45' as decimal(4, 2));
 (1 row)
 
 !ok
-EnumerableCalc(expr#0=[{inputs}], expr#1=[123.45:DECIMAL(4, 2)], EXPR$0=[$t1])
+EnumerableCalc(expr#0=[{inputs}], expr#1=[123.45:DECIMAL(5, 2)], EXPR$0=[$t1])
   EnumerableValues(tuples=[[{ 0 }]])
 !plan