[calcite] branch master updated: [CALCITE-5040] SqlTypeFactoryTest.testUnknownCreateWithNullabilityTypeConsistency fails

[li...@apache.org]

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 692b8f0  [CALCITE-5040] SqlTypeFactoryTest.testUnknownCreateWithNullabilityTypeConsistency fails
692b8f0 is described below

commit 692b8f0886a25538e010d1a8ae5efe89f80d89e9
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Mar 14 20:54:04 2022 -0700

    [CALCITE-5040] SqlTypeFactoryTest.testUnknownCreateWithNullabilityTypeConsistency fails
    
    [CALCITE-4872] added an UNKNOWN type that was implemented in some
    places by class SqlTypeFactoryImpl.UnknownSqlType, but in others
    by BasicSqlType. This would cause
    SqlTypeFactoryTest.testUnknownCreateWithNullabilityTypeConsistency
    to succeed or fail non-deterministically, depending on which of the
    above had made it into the map of canonical type instances.
    
    This commit solves the problem by removing class
    SqlTypeFactoryImpl.UnknownSqlType and always using BasicSqlType.
---
 .../calcite/sql/type/SqlTypeFactoryImpl.java       | 27 +------------------
 .../calcite/sql/type/SqlTypeFactoryTest.java       | 31 +++++++++++++++-------
 2 files changed, 22 insertions(+), 36 deletions(-)

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
index 532ccfe..8521c841 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
@@ -93,7 +93,7 @@ public class SqlTypeFactoryImpl extends RelDataTypeFactoryImpl {
   }
 
   @Override public RelDataType createUnknownType() {
-    return canonize(new UnknownSqlType(this));
+    return createSqlType(SqlTypeName.UNKNOWN);
   }
 
   @Override public RelDataType createMultisetType(
@@ -550,7 +550,6 @@ public class SqlTypeFactoryImpl extends RelDataTypeFactoryImpl {
     return new MapSqlType(keyType, valueType, nullable);
   }
 
-  // override RelDataTypeFactoryImpl
   @Override protected RelDataType canonize(RelDataType type) {
     type = super.canonize(type);
     if (!(type instanceof ObjectSqlType)) {
@@ -567,28 +566,4 @@ public class SqlTypeFactoryImpl extends RelDataTypeFactoryImpl {
     }
     return type;
   }
-
-  /** The unknown type. Similar to the NULL type, but is only equal to
-   * itself. */
-  static class UnknownSqlType extends BasicSqlType {
-    UnknownSqlType(RelDataTypeFactory typeFactory) {
-      this(typeFactory.getTypeSystem(), false);
-    }
-
-    private UnknownSqlType(RelDataTypeSystem typeSystem, boolean nullable) {
-      super(typeSystem, SqlTypeName.UNKNOWN, nullable);
-    }
-
-    @Override BasicSqlType createWithNullability(boolean nullable) {
-      if (nullable == this.isNullable) {
-        return this;
-      }
-      return new UnknownSqlType(this.typeSystem, nullable);
-    }
-
-    @Override protected void generateTypeString(StringBuilder sb,
-        boolean withDetail) {
-      sb.append("UNKNOWN");
-    }
-  }
 }
diff --git a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java
index fa07a31..9f7ec0a 100644
--- a/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java
@@ -21,7 +21,6 @@ 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.RelRecordType;
-import org.apache.calcite.sql.type.SqlTypeFactoryImpl.UnknownSqlType;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
@@ -288,18 +287,30 @@ class SqlTypeFactoryTest {
     assertThat(tsWithPrecision3 == tsWithPrecision8, is(true));
   }
 
-  /** Test that the {code UNKNOWN} type does not does not change class when nullified. */
+  /** Test that the {@code UNKNOWN} type is a {@link BasicSqlType} and remains
+   * so when nullified. */
   @Test void testUnknownCreateWithNullabilityTypeConsistency() {
-    SqlTypeFixture f = new SqlTypeFixture();
+    final SqlTypeFixture f = new SqlTypeFixture();
 
-    RelDataType unknownType  = f.typeFactory.createUnknownType();
-    assertThat(unknownType, isA(UnknownSqlType.class));
+    final RelDataType unknownType = f.typeFactory.createUnknownType();
+    assertThat(unknownType, isA(BasicSqlType.class));
     assertThat(unknownType.getSqlTypeName(), is(SqlTypeName.UNKNOWN));
     assertFalse(unknownType.isNullable());
-
-    RelDataType nullableRelDataType = f.typeFactory.createTypeWithNullability(unknownType, true);
-    assertThat(nullableRelDataType, isA(UnknownSqlType.class));
-    assertThat(nullableRelDataType.getSqlTypeName(), is(SqlTypeName.UNKNOWN));
-    assertTrue(nullableRelDataType.isNullable());
+    assertThat(unknownType.getFullTypeString(), is("UNKNOWN NOT NULL"));
+
+    final RelDataType nullableType =
+        f.typeFactory.createTypeWithNullability(unknownType, true);
+    assertThat(nullableType, isA(BasicSqlType.class));
+    assertThat(nullableType.getSqlTypeName(), is(SqlTypeName.UNKNOWN));
+    assertTrue(nullableType.isNullable());
+    assertThat(nullableType.getFullTypeString(), is("UNKNOWN"));
+
+    final RelDataType unknownType2 =
+        f.typeFactory.createTypeWithNullability(nullableType, false);
+    assertThat(unknownType2, is(unknownType));
+    assertThat(unknownType2, isA(BasicSqlType.class));
+    assertThat(unknownType2.getSqlTypeName(), is(SqlTypeName.UNKNOWN));
+    assertFalse(unknownType2.isNullable());
+    assertThat(unknownType2.getFullTypeString(), is("UNKNOWN NOT NULL"));
   }
 }