You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by li...@apache.org on 2021/04/30 02:02:50 UTC
[calcite] branch master updated: [CALCITE-4510] RexLiteral can
produce wrong digest for some user defined types
This is an automated email from the ASF dual-hosted git repository.
liyafan 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 350802b [CALCITE-4510] RexLiteral can produce wrong digest for some user defined types
350802b is described below
commit 350802b59fa68ef1ee074b78af17bf1b2c435e13
Author: liyafan82 <fa...@foxmail.com>
AuthorDate: Tue Feb 23 11:57:41 2021 +0800
[CALCITE-4510] RexLiteral can produce wrong digest for some user defined types
---
.../java/org/apache/calcite/plan/RelOptUtil.java | 6 ++++--
.../org/apache/calcite/rel/type/RelDataTypeImpl.java | 8 +++++++-
.../main/java/org/apache/calcite/rex/RexLiteral.java | 7 +++++--
.../org/apache/calcite/sql/type/SqlTypeUtil.java | 5 +++--
.../java/org/apache/calcite/rex/RexBuilderTest.java | 20 ++++++++++++++++++++
.../java/org/apache/calcite/rex/RexProgramTest.java | 4 +++-
.../org/apache/calcite/rex/RexProgramTestBase.java | 4 +++-
.../apache/calcite/sql/test/SqlOperatorBaseTest.java | 9 +++++----
.../java/org/apache/calcite/sql/test/SqlTests.java | 3 ++-
.../java/org/apache/calcite/test/CalciteAssert.java | 3 ++-
10 files changed, 54 insertions(+), 15 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 1b89a75..61fb234 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -135,6 +135,8 @@ import java.util.TreeSet;
import java.util.function.Supplier;
import java.util.stream.Collectors;
+import static org.apache.calcite.rel.type.RelDataTypeImpl.NON_NULLABLE_SUFFIX;
+
/**
* <code>RelOptUtil</code> defines static utility methods for use in optimizing
* {@link RelNode}s.
@@ -4315,7 +4317,7 @@ public abstract class RelOptUtil {
this.indent = prevIndent;
pw.print(")");
if (!type.isNullable()) {
- pw.print(" NOT NULL");
+ pw.print(NON_NULLABLE_SUFFIX);
}
} else if (type instanceof MultisetSqlType) {
// E.g. "INTEGER NOT NULL MULTISET NOT NULL"
@@ -4326,7 +4328,7 @@ public abstract class RelOptUtil {
accept(componentType);
pw.print(" MULTISET");
if (!type.isNullable()) {
- pw.print(" NOT NULL");
+ pw.print(NON_NULLABLE_SUFFIX);
}
} else {
// E.g. "INTEGER" E.g. "VARCHAR(240) CHARACTER SET "ISO-8859-1"
diff --git a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeImpl.java b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeImpl.java
index 544d1ee..181cf53 100644
--- a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeImpl.java
+++ b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeImpl.java
@@ -50,6 +50,12 @@ import static java.util.Objects.requireNonNull;
*/
public abstract class RelDataTypeImpl
implements RelDataType, RelDataTypeFamily {
+
+ /**
+ * Suffix for the digests of non-nullable types.
+ */
+ public static final String NON_NULLABLE_SUFFIX = " NOT NULL";
+
//~ Instance fields --------------------------------------------------------
protected final @Nullable List<RelDataTypeField> fieldList;
@@ -279,7 +285,7 @@ public abstract class RelDataTypeImpl
StringBuilder sb = new StringBuilder();
generateTypeString(sb, true);
if (!isNullable()) {
- sb.append(" NOT NULL");
+ sb.append(NON_NULLABLE_SUFFIX);
}
digest = sb.toString();
}
diff --git a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java b/core/src/main/java/org/apache/calcite/rex/RexLiteral.java
index 75da3b3..e74eee4 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexLiteral.java
@@ -65,6 +65,7 @@ import java.util.Objects;
import java.util.TimeZone;
import static org.apache.calcite.linq4j.Nullness.castNonNull;
+import static org.apache.calcite.rel.type.RelDataTypeImpl.NON_NULLABLE_SUFFIX;
import static java.util.Objects.requireNonNull;
@@ -432,11 +433,13 @@ public class RexLiteral extends RexNode {
if (includeType != RexDigestIncludeType.NO_TYPE) {
sb.append(':');
final String fullTypeString = type.getFullTypeString();
- if (!fullTypeString.endsWith("NOT NULL")) {
+
+ if (!fullTypeString.endsWith(NON_NULLABLE_SUFFIX)) {
sb.append(fullTypeString);
} else {
// Trim " NOT NULL". Apparently, the literal is not null, so we just print the data type.
- sb.append(fullTypeString, 0, fullTypeString.length() - 9);
+ sb.append(fullTypeString, 0,
+ fullTypeString.length() - NON_NULLABLE_SUFFIX.length());
}
}
return sb.toString();
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 9c1b188..714371b 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
@@ -60,6 +60,7 @@ import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
+import static org.apache.calcite.rel.type.RelDataTypeImpl.NON_NULLABLE_SUFFIX;
import static org.apache.calcite.sql.type.NonNullableAccessors.getCharset;
import static org.apache.calcite.sql.type.NonNullableAccessors.getCollation;
import static org.apache.calcite.sql.type.NonNullableAccessors.getComponentTypeOrThrow;
@@ -1194,8 +1195,8 @@ public abstract class SqlTypeUtil {
}
return (x.length() == y.length()
- || x.length() == y.length() + 9 && x.endsWith(" NOT NULL"))
- && x.startsWith(y);
+ || x.length() == y.length() + NON_NULLABLE_SUFFIX.length()
+ && x.endsWith(NON_NULLABLE_SUFFIX)) && x.startsWith(y);
}
/**
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 e08cfc4..e51f3a7 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
@@ -22,6 +22,7 @@ import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
+import org.apache.calcite.rel.type.RelDataTypeImpl;
import org.apache.calcite.rel.type.RelDataTypeSystem;
import org.apache.calcite.rel.type.RelDataTypeSystemImpl;
import org.apache.calcite.sql.SqlCollation;
@@ -792,4 +793,23 @@ class RexBuilderTest {
RexChecker checker = new RexChecker(structType, () -> null, Litmus.THROW);
assertThat(fieldAccess.accept(checker), is(true));
}
+
+ /** Emulate a user defined type. */
+ private static class UDT extends RelDataTypeImpl {
+ UDT() {
+ this.digest = "(udt)NOT NULL";
+ }
+
+ @Override protected void generateTypeString(StringBuilder sb, boolean withDetail) {
+ sb.append("udt");
+ }
+ }
+
+ @Test void testUDTLiteralDigest() {
+ RexLiteral literal = new RexLiteral(new BigDecimal(0L), new UDT(), SqlTypeName.BIGINT);
+
+ // when the space before "NOT NULL" is missing, the digest is not correct
+ // and the suffix should not be removed.
+ assertThat(literal.digest, is("0L:(udt)NOT NULL"));
+ }
}
diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
index 8df9bea..887c597 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
@@ -23,6 +23,7 @@ import org.apache.calcite.plan.Strong;
import org.apache.calcite.rel.metadata.NullSentinel;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeImpl;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlSpecialOperator;
@@ -2524,7 +2525,8 @@ class RexProgramTest extends RexProgramTestBase {
RexNode rexNode, String representation, String type) {
assertEquals(representation, rexNode.toString());
assertEquals(type, rexNode.getType().toString()
- + (rexNode.getType().isNullable() ? "" : " NOT NULL"), "type of " + rexNode);
+ + (rexNode.getType().isNullable() ? "" : RelDataTypeImpl.NON_NULLABLE_SUFFIX),
+ "type of " + rexNode);
}
@Test void testIsDeterministic() {
diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTestBase.java b/core/src/test/java/org/apache/calcite/rex/RexProgramTestBase.java
index 36820db..8d415e6 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexProgramTestBase.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTestBase.java
@@ -17,6 +17,7 @@
package org.apache.calcite.rex;
import org.apache.calcite.plan.RelOptPredicateList;
+import org.apache.calcite.rel.type.RelDataTypeImpl;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.test.Matchers;
@@ -78,7 +79,8 @@ class RexProgramTestBase extends RexProgramBuilderBase {
// toString contains type (see RexCall.toString)
actual = node.toString();
} else {
- actual = node + ":" + node.getType() + (node.getType().isNullable() ? "" : " NOT NULL");
+ actual = node + ":" + node.getType() + (node.getType().isNullable() ? ""
+ : RelDataTypeImpl.NON_NULLABLE_SUFFIX);
}
assertEquals(expected, actual, message);
}
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 25c6584..1ca3b0a 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -93,6 +93,7 @@ import java.util.function.UnaryOperator;
import java.util.regex.Pattern;
import java.util.stream.Stream;
+import static org.apache.calcite.rel.type.RelDataTypeImpl.NON_NULLABLE_SUFFIX;
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.PI;
import static org.apache.calcite.util.DateTimeStringUtils.getDateFormatter;
@@ -468,7 +469,7 @@ public abstract class SqlOperatorBaseTest {
double delta) {
tester.checkScalarApprox(
getCastString(value, targetType, false),
- targetType + " NOT NULL",
+ targetType + NON_NULLABLE_SUFFIX,
expected,
delta);
}
@@ -480,7 +481,7 @@ public abstract class SqlOperatorBaseTest {
tester.checkString(
getCastString(value, targetType, false),
expected,
- targetType + " NOT NULL");
+ targetType + NON_NULLABLE_SUFFIX);
}
private void checkCastToScalarOkay(
@@ -489,7 +490,7 @@ public abstract class SqlOperatorBaseTest {
String expected) {
tester.checkScalarExact(
getCastString(value, targetType, false),
- targetType + " NOT NULL",
+ targetType + NON_NULLABLE_SUFFIX,
expected);
}
@@ -7040,7 +7041,7 @@ public abstract class SqlOperatorBaseTest {
if (binary) {
expected = DOUBLER.apply(expected);
}
- t.checkString(expression, expected, type + " NOT NULL");
+ t.checkString(expression, expected, type + NON_NULLABLE_SUFFIX);
}
}
}
diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlTests.java b/core/src/test/java/org/apache/calcite/sql/test/SqlTests.java
index 5404121..611a15d 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlTests.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlTests.java
@@ -18,6 +18,7 @@ package org.apache.calcite.sql.test;
import org.apache.calcite.avatica.ColumnMetaData;
import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeImpl;
import org.apache.calcite.runtime.CalciteContextException;
import org.apache.calcite.sql.parser.SqlParseException;
import org.apache.calcite.sql.parser.SqlParserUtil;
@@ -104,7 +105,7 @@ public abstract class SqlTests {
actual = actual + "(" + sqlType.getPrecision() + ")";
}
if (!sqlType.isNullable()) {
- actual += " NOT NULL";
+ actual += RelDataTypeImpl.NON_NULLABLE_SUFFIX;
}
return actual;
diff --git a/core/src/test/java/org/apache/calcite/test/CalciteAssert.java b/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
index 7bda2e1..88e3cea 100644
--- a/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
+++ b/core/src/test/java/org/apache/calcite/test/CalciteAssert.java
@@ -38,6 +38,7 @@ import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.RelRoot;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.RelDataTypeImpl;
import org.apache.calcite.runtime.CalciteException;
import org.apache.calcite.runtime.FlatLists;
import org.apache.calcite.runtime.GeoFunctions;
@@ -487,7 +488,7 @@ public class CalciteAssert {
+ " "
+ metaData.getColumnTypeName(i + 1)
+ (metaData.isNullable(i + 1) == ResultSetMetaData.columnNoNulls
- ? " NOT NULL"
+ ? RelDataTypeImpl.NON_NULLABLE_SUFFIX
: ""));
}
return list.toString();