This is an automated email from the ASF dual-hosted git repository.
zabetak 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 846494a [CALCITE-2464] Allow to set nullability for columns of
structured types (Ruben Quesada Lopez) - Added nullability flag to struct types
(previously they were always considered as non-nullable) - Struct types will be
built as non-nullable by default to avoid regresssions, but this can be changed
via RelDataTypeFactory#createTypeWithNullability - SqlCreateTable will generate
nullable struct types, unless they are defined as NOT NULL - New unit tests
846494a is described below
commit 846494a0091b00357f3ccb67c8a4a7d9d226cd7d
Author: rubenada <rube...@gmail.com>
AuthorDate: Thu Nov 1 17:19:40 2018 +0100
[CALCITE-2464] Allow to set nullability for columns of structured types
(Ruben Quesada Lopez)
- Added nullability flag to struct types (previously they were always
considered as non-nullable)
- Struct types will be built as non-nullable by default to avoid
regresssions, but this
can be changed via RelDataTypeFactory#createTypeWithNullability
- SqlCreateTable will generate nullable struct types, unless they are
defined as NOT NULL
- New unit tests
---
.../apache/calcite/jdbc/JavaTypeFactoryImpl.java | 28 +++++---
.../calcite/rel/type/RelDataTypeFactoryImpl.java | 74 +++++++++++++---------
.../org/apache/calcite/rel/type/RelRecordType.java | 25 +++++++-
.../calcite/sql/type/SqlTypeFactoryTest.java | 29 +++++++++
.../java/org/apache/calcite/test/JdbcTest.java | 15 +++++
.../org/apache/calcite/sql/ddl/SqlCreateTable.java | 4 ++
.../java/org/apache/calcite/test/ServerTest.java | 8 +++
server/src/test/resources/sql/type.iq | 34 +++++++++-
8 files changed, 173 insertions(+), 44 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java
b/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java
index 08a7aa8..7b689dc 100644
--- a/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java
+++ b/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java
@@ -243,18 +243,26 @@ public class JavaTypeFactoryImpl
/** Converts a type in Java format to a SQL-oriented type. */
public static RelDataType toSql(final RelDataTypeFactory typeFactory,
RelDataType type) {
+ return toSql(typeFactory, type, true);
+ }
+
+ private static RelDataType toSql(final RelDataTypeFactory typeFactory,
+ RelDataType type, boolean mustSetNullability) {
+ RelDataType sqlType = type;
if (type instanceof RelRecordType) {
- return typeFactory.createStructType(
- Lists.transform(type.getFieldList(),
- field -> toSql(typeFactory, field.getType())),
- type.getFieldNames());
- }
- if (type instanceof JavaType) {
- return typeFactory.createTypeWithNullability(
- typeFactory.createSqlType(type.getSqlTypeName()),
- type.isNullable());
+ // We do not need to change the nullability of the nested fields,
+ // since it can be overridden by the existing implementation of
createTypeWithNullability
+ // when we treat the nullability of the root struct type.
+ sqlType = typeFactory.createStructType(
+ Lists.transform(type.getFieldList(),
+ field -> toSql(typeFactory, field.getType(), false)),
+ type.getFieldNames());
+ } else if (type instanceof JavaType) {
+ sqlType = typeFactory.createSqlType(type.getSqlTypeName());
}
- return type;
+ return mustSetNullability
+ ? typeFactory.createTypeWithNullability(sqlType, type.isNullable())
+ : sqlType;
}
public Type createSyntheticType(List<Type> types) {
diff --git
a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
index d529ff7..26d4980 100644
--- a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
+++ b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java
@@ -70,7 +70,7 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
new RelDataTypeFieldImpl(
key.names.get(i), i, key.types.get(i)));
}
- return new RelRecordType(key.kind, list.build());
+ return new RelRecordType(key.kind, list.build(), key.nullable);
}
private static final Map<Class, RelDataTypeFamily> CLASS_FAMILIES =
@@ -141,8 +141,16 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
public RelDataType createStructType(StructKind kind,
final List<RelDataType> typeList,
final List<String> fieldNameList) {
+ return createStructType(kind, typeList,
+ fieldNameList, false);
+ }
+
+ private RelDataType createStructType(StructKind kind,
+ final List<RelDataType> typeList,
+ final List<String> fieldNameList,
+ final boolean nullable) {
assert typeList.size() == fieldNameList.size();
- return canonize(kind, fieldNameList, typeList);
+ return canonize(kind, fieldNameList, typeList, nullable);
}
@SuppressWarnings("deprecation")
@@ -171,6 +179,11 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
public final RelDataType createStructType(
final List<? extends Map.Entry<String, RelDataType>> fieldList) {
+ return createStructType(fieldList, false);
+ }
+
+ private RelDataType createStructType(
+ final List<? extends Map.Entry<String, RelDataType>> fieldList, boolean
nullable) {
return canonize(StructKind.FULLY_QUALIFIED,
new AbstractList<String>() {
@Override public String get(int index) {
@@ -189,7 +202,7 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
@Override public int size() {
return fieldList.size();
}
- });
+ }, nullable);
}
public RelDataType leastRestrictive(List<RelDataType> types) {
@@ -270,12 +283,8 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
final RelRecordType type,
final boolean ignoreNullable,
final boolean nullable) {
- // REVIEW: angel 18-Aug-2005 dtbug336
- // Shouldn't null refer to the nullability of the record type
- // not the individual field types?
// For flattening and outer joins, it is desirable to change
// the nullability of the individual fields.
-
return createStructType(type.getStructKind(),
new AbstractList<RelDataType>() {
@Override public RelDataType get(int index) {
@@ -292,18 +301,12 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
return type.getFieldCount();
}
},
- type.getFieldNames());
+ type.getFieldNames(), nullable);
}
// implement RelDataTypeFactory
public RelDataType copyType(RelDataType type) {
- if (type instanceof RelRecordType) {
- return copyRecordType((RelRecordType) type, true, false);
- } else {
- return createTypeWithNullability(
- type,
- type.isNullable());
- }
+ return createTypeWithNullability(type, type.isNullable());
}
// implement RelDataTypeFactory
@@ -318,15 +321,16 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
// REVIEW: angel 18-Aug-2005 dtbug 336 workaround
// Changed to ignore nullable parameter if nullable is false since
// copyRecordType implementation is doubtful
- if (nullable) {
- // Do a deep copy, setting all fields of the record type
- // to be nullable regardless of initial nullability
- newType = copyRecordType((RelRecordType) type, false, true);
- } else {
- // Keep same type as before, ignore nullable parameter
- // RelRecordType currently always returns a nullability of false
- newType = copyRecordType((RelRecordType) type, true, false);
- }
+ // - If nullable -> Do a deep copy, setting all fields of the record type
+ // to be nullable regardless of initial nullability.
+ // - If not nullable -> Do a deep copy, setting not nullable at top
RelRecordType
+ // level only, keeping its fields' nullability as before.
+ // According to the SQL standard, nullability for struct types can be
defined only for
+ // columns, which translates to top level structs. Nested struct
attributes are always
+ // nullable, so in principle we could always set the nested attributes
to be nullable.
+ // However, this might create regressions so we will not do it and we
will keep previous
+ // behavior.
+ newType = copyRecordType((RelRecordType) type, !nullable, nullable);
} else {
newType = copySimpleType(type, nullable);
}
@@ -353,14 +357,21 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
*/
protected RelDataType canonize(final StructKind kind,
final List<String> names,
- final List<RelDataType> types) {
- final RelDataType type = CACHE.getIfPresent(new Key(kind, names, types));
+ final List<RelDataType> types,
+ final boolean nullable) {
+ final RelDataType type = CACHE.getIfPresent(new Key(kind, names, types,
nullable));
if (type != null) {
return type;
}
final ImmutableList<String> names2 = ImmutableList.copyOf(names);
final ImmutableList<RelDataType> types2 = ImmutableList.copyOf(types);
- return CACHE.getUnchecked(new Key(kind, names2, types2));
+ return CACHE.getUnchecked(new Key(kind, names2, types2, nullable));
+ }
+
+ protected RelDataType canonize(final StructKind kind,
+ final List<String> names,
+ final List<RelDataType> types) {
+ return canonize(kind, names, types, false);
}
/**
@@ -658,15 +669,17 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
private final StructKind kind;
private final List<String> names;
private final List<RelDataType> types;
+ private final boolean nullable;
- Key(StructKind kind, List<String> names, List<RelDataType> types) {
+ Key(StructKind kind, List<String> names, List<RelDataType> types, boolean
nullable) {
this.kind = kind;
this.names = names;
this.types = types;
+ this.nullable = nullable;
}
@Override public int hashCode() {
- return Objects.hash(kind, names, types);
+ return Objects.hash(kind, names, types, nullable);
}
@Override public boolean equals(Object obj) {
@@ -674,7 +687,8 @@ public abstract class RelDataTypeFactoryImpl implements
RelDataTypeFactory {
|| obj instanceof Key
&& kind == ((Key) obj).kind
&& names.equals(((Key) obj).names)
- && types.equals(((Key) obj).types);
+ && types.equals(((Key) obj).types)
+ && nullable == ((Key) obj).nullable;
}
}
}
diff --git a/core/src/main/java/org/apache/calcite/rel/type/RelRecordType.java
b/core/src/main/java/org/apache/calcite/rel/type/RelRecordType.java
index 0bb4ed0..b9b0818 100644
--- a/core/src/main/java/org/apache/calcite/rel/type/RelRecordType.java
+++ b/core/src/main/java/org/apache/calcite/rel/type/RelRecordType.java
@@ -29,21 +29,40 @@ import java.util.Objects;
public class RelRecordType extends RelDataTypeImpl implements Serializable {
/** Name resolution policy; usually {@link StructKind#FULLY_QUALIFIED}. */
private final StructKind kind;
+ private final boolean nullable;
//~ Constructors -----------------------------------------------------------
/**
* Creates a <code>RecordType</code>. This should only be called from a
* factory method.
+ * @param kind Name resolution policy
+ * @param fields List of fields
+ * @param nullable Whether this record type allows null values
*/
- public RelRecordType(StructKind kind, List<RelDataTypeField> fields) {
+ public RelRecordType(StructKind kind, List<RelDataTypeField> fields, boolean
nullable) {
super(fields);
+ this.nullable = nullable;
this.kind = Objects.requireNonNull(kind);
computeDigest();
}
+ /**
+ * Creates a <code>RecordType</code>. This should only be called from a
+ * factory method.
+ * Shorthand for <code>RelRecordType(kind, fields, false)</code>.
+ */
+ public RelRecordType(StructKind kind, List<RelDataTypeField> fields) {
+ this(kind, fields, false);
+ }
+
+ /**
+ * Creates a <code>RecordType</code>. This should only be called from a
+ * factory method.
+ * Shorthand for <code>RelRecordType(StructKind.FULLY_QUALIFIED, fields,
false)</code>.
+ */
public RelRecordType(List<RelDataTypeField> fields) {
- this(StructKind.FULLY_QUALIFIED, fields);
+ this(StructKind.FULLY_QUALIFIED, fields, false);
}
//~ Methods ----------------------------------------------------------------
@@ -53,7 +72,7 @@ public class RelRecordType extends RelDataTypeImpl implements
Serializable {
}
@Override public boolean isNullable() {
- return false;
+ return nullable;
}
@Override public int getPrecision() {
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 60e3325..6af99cf 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
@@ -17,13 +17,22 @@
package org.apache.calcite.sql.type;
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.RelRecordType;
import com.google.common.collect.Lists;
import org.junit.Test;
+import java.util.ArrayList;
+import java.util.List;
+
import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**
@@ -112,6 +121,26 @@ public class SqlTypeFactoryTest {
assertThat(SqlTypeUtil.comparePrecision(p1, p1), is(0));
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-2464";>[CALCITE-2464]
+ * Allow to set nullability for columns of structured types</a>. */
+ @Test
+ public void createStructTypeWithNullability() {
+ SqlTypeFixture f = new SqlTypeFixture();
+ RelDataTypeFactory typeFactory = f.typeFactory;
+ List<RelDataTypeField> fields = new ArrayList<>();
+ RelDataTypeField field0 = new RelDataTypeFieldImpl(
+ "i", 0, typeFactory.createSqlType(SqlTypeName.INTEGER));
+ RelDataTypeField field1 = new RelDataTypeFieldImpl(
+ "s", 1, typeFactory.createSqlType(SqlTypeName.VARCHAR));
+ fields.add(field0);
+ fields.add(field1);
+ final RelDataType recordType = new RelRecordType(fields); // nullable
false by default
+ final RelDataType copyRecordType =
typeFactory.createTypeWithNullability(recordType, true);
+ assertFalse(recordType.isNullable());
+ assertTrue(copyRecordType.isNullable());
+ }
+
}
// End SqlTypeFactoryTest.java
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index 77674a6..01ad5d5 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -4679,6 +4679,21 @@ public class JdbcTest {
"deptno=null; deptno=40");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-2464";>[CALCITE-2464]
+ * Allow to set nullability for columns of structured types</a>. */
+ @Test public void testLeftJoinWhereStructIsNotNull() {
+ CalciteAssert.hr()
+ .query("select e.\"deptno\", d.\"deptno\"\n"
+ + "from \"hr\".\"emps\" as e\n"
+ + " left join \"hr\".\"depts\" as d using (\"deptno\")"
+ + "where d.\"location\" is not null")
+ .returnsUnordered(
+ "deptno=10; deptno=10",
+ "deptno=10; deptno=10",
+ "deptno=10; deptno=10");
+ }
+
/** Various queries against EMP and DEPT, in particular involving composite
* join conditions in various flavors of outer join. Results are verified
* against MySQL (except full join, which MySQL does not support). */
diff --git
a/server/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
b/server/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
index 3550831..a6ba383 100644
--- a/server/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
+++ b/server/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
@@ -174,6 +174,10 @@ public class SqlCreateTable extends SqlCreate
CalciteSchema.TypeEntry typeEntry =
pairForType.left.getType(pairForType.right, false);
if (typeEntry != null) {
type = typeEntry.getType().apply(typeFactory);
+ if (d.dataType.getNullable() != null
+ && d.dataType.getNullable() != type.isNullable()) {
+ type = typeFactory.createTypeWithNullability(type,
d.dataType.getNullable());
+ }
}
}
builder.add(d.name.getSimple(), type);
diff --git a/server/src/test/java/org/apache/calcite/test/ServerTest.java
b/server/src/test/java/org/apache/calcite/test/ServerTest.java
index e4e2026..2d8b241 100644
--- a/server/src/test/java/org/apache/calcite/test/ServerTest.java
+++ b/server/src/test/java/org/apache/calcite/test/ServerTest.java
@@ -139,6 +139,14 @@ public class ServerTest {
assertThat(r.getInt(1), is(4));
assertThat(r.next(), is(false));
}
+
+ // CALCITE-2464: Allow to set nullability for columns of structured types
+ b = s.execute("create type mytype as (i int)");
+ assertThat(b, is(false));
+ b = s.execute("create table w (i int not null, j mytype)");
+ assertThat(b, is(false));
+ x = s.executeUpdate("insert into w values (1, NULL)");
+ assertThat(x, is(1));
}
}
diff --git a/server/src/test/resources/sql/type.iq
b/server/src/test/resources/sql/type.iq
index 7103be4..19eb1ac 100644
--- a/server/src/test/resources/sql/type.iq
+++ b/server/src/test/resources/sql/type.iq
@@ -52,7 +52,7 @@ select * from t;
# Create a table with complex structure type
# This is to test struct type inference in
-# <a href="https://issues.apache.org/jira/browse/CALCITE-1222";>[CALCITE-2468]
+# <a href="https://issues.apache.org/jira/browse/CALCITE-2468";>[CALCITE-2468]
create type mytype1 as (ii int not null);
(0 rows modified)
@@ -70,8 +70,40 @@ MYINT INTEGER(10) NOT NULL
MYSTRUCT STRUCT NOT NULL
!type
+
+# Create a table with nullable complex structure type
+# This is to test nullability for columns of structured types
+# <a href="https://issues.apache.org/jira/browse/CALCITE-2464";>[CALCITE-2464]
+
+# Create a complex table
+create table w (i int not null, j mytype1);
+(0 rows modified)
+
+!update
+
+select i AS myInt, j AS myNullableStruct from w;
+MYINT INTEGER(10) NOT NULL
+MYNULLABLESTRUCT STRUCT
+!type
+
+insert into w values (1, NULL);
+(1 row modified)
+
+!update
+
+select * from w;
++---+---+
+| I | J |
++---+---+
+| 1 | |
++---+---+
+(1 row)
+
+!ok
+
drop table t;
drop table v;
+drop table w;
(0 rows modified)
!update
