Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CALCITE-5040] SqlTypeFactoryTest.testUnknownCreateWithNullabilityTypeConsistency fails #2744

Merged
merged 1 commit into from Mar 16, 2022
Merged

[CALCITE-5040] SqlTypeFactoryTest.testUnknownCreateWithNullabilityTypeConsistency fails #2744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 1 addition & 26 deletions core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java
View file
Open in desktop
Expand Up @@ -93,7 +93,7 @@ public SqlTypeFactoryImpl(RelDataTypeSystem typeSystem) {
}

@Override public RelDataType createUnknownType() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By adding UNKNOWN under SqlTypeName enum we are essentially making it a basic SQL type so we could consider deprecating and eventually removing the createUnknownType API. It's probably out of the scope of this PR but if it makes sense we could log a followup JIRA to tackle it later.

return canonize(new UnknownSqlType(this));
return createSqlType(SqlTypeName.UNKNOWN);
}

@Override public RelDataType createMultisetType(
Expand Down Expand Up @@ -550,7 +550,6 @@ private RelDataType copyMapType(RelDataType type, boolean nullable) {
return new MapSqlType(keyType, valueType, nullable);
}

// override RelDataTypeFactoryImpl
@Override protected RelDataType canonize(RelDataType type) {
type = super.canonize(type);
if (!(type instanceof ObjectSqlType)) {
Expand All @@ -567,28 +566,4 @@ private RelDataType copyMapType(RelDataType type, boolean nullable) {
}
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");
}
}
}
31 changes: 21 additions & 10 deletions core/src/test/java/org/apache/calcite/sql/type/SqlTypeFactoryTest.java
View file
Open in desktop
Expand Up @@ -21,7 +21,6 @@
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;
Expand Down Expand Up @@ -288,18 +287,30 @@ private void checkCreateSqlTypeWithPrecision(
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"));
}
}