Repository: calcite Updated Branches: refs/heads/master 439ca73b8 -> 18caf38f8
[CALCITE-2714] Make BasicSqlType immutable, and now SqlTypeFactory.createWithNullability can reuse existing type if possible (Ruben Quesada Lopez) Close apache/calcite#947 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/08aefb09 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/08aefb09 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/08aefb09 Branch: refs/heads/master Commit: 08aefb09c596c3a3292b68b695b1038fd4f64b92 Parents: 439ca73 Author: rubenada <[email protected]> Authored: Thu Nov 29 11:08:03 2018 +0100 Committer: Julian Hyde <[email protected]> Committed: Fri Nov 30 17:53:31 2018 -0800 ---------------------------------------------------------------------- .../calcite/sql/type/AbstractSqlType.java | 3 +- .../apache/calcite/sql/type/BasicSqlType.java | 94 ++++++++++---------- .../calcite/sql/type/SqlTypeFactoryImpl.java | 5 +- 3 files changed, 53 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/08aefb09/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java b/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java index 4baa04d..2e316f5 100644 --- a/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java +++ b/core/src/main/java/org/apache/calcite/sql/type/AbstractSqlType.java @@ -24,6 +24,7 @@ import org.apache.calcite.rel.type.RelDataTypePrecedenceList; import java.io.Serializable; import java.util.List; +import java.util.Objects; /** * Abstract base class for SQL implementations of {@link RelDataType}. @@ -50,7 +51,7 @@ public abstract class AbstractSqlType boolean isNullable, List<? extends RelDataTypeField> fields) { super(fields); - this.typeName = typeName; + this.typeName = Objects.requireNonNull(typeName); this.isNullable = isNullable || (typeName == SqlTypeName.NULL); } http://git-wip-us.apache.org/repos/asf/calcite/blob/08aefb09/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java b/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java index 1c0ac32..72e4777 100644 --- a/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java +++ b/core/src/main/java/org/apache/calcite/sql/type/BasicSqlType.java @@ -23,10 +23,13 @@ import org.apache.calcite.util.SerializableCharset; import com.google.common.base.Preconditions; import java.nio.charset.Charset; +import java.util.Objects; /** * BasicSqlType represents a standard atomic SQL type (excluding interval * types). + * + * <p>Instances of this class are immutable. */ public class BasicSqlType extends AbstractSqlType { //~ Static fields/initializers --------------------------------------------- @@ -36,8 +39,8 @@ public class BasicSqlType extends AbstractSqlType { private final int precision; private final int scale; private final RelDataTypeSystem typeSystem; - private SqlCollation collation; - private SerializableCharset wrappedCharset; + private final SqlCollation collation; + private final SerializableCharset wrappedCharset; //~ Constructors ----------------------------------------------------------- @@ -45,39 +48,51 @@ public class BasicSqlType extends AbstractSqlType { * Constructs a type with no parameters. This should only be called from a * factory method. * + * @param typeSystem Type system * @param typeName Type name */ public BasicSqlType(RelDataTypeSystem typeSystem, SqlTypeName typeName) { this(typeSystem, typeName, false, PRECISION_NOT_SPECIFIED, - SCALE_NOT_SPECIFIED); - assert typeName.allowsPrecScale(false, false) - : "typeName.allowsPrecScale(false,false), typeName=" + typeName.name(); - computeDigest(); + SCALE_NOT_SPECIFIED, null, null); + checkPrecScale(typeName, false, false); + } + + /** Throws if {@code typeName} does not allow the given combination of + * precision and scale. */ + protected static void checkPrecScale(SqlTypeName typeName, + boolean precisionSpecified, boolean scaleSpecified) { + if (!typeName.allowsPrecScale(precisionSpecified, scaleSpecified)) { + throw new AssertionError("typeName.allowsPrecScale(" + + precisionSpecified + ", " + scaleSpecified + "): " + typeName); + } } /** * Constructs a type with precision/length but no scale. * + * @param typeSystem Type system * @param typeName Type name + * @param precision Precision (called length for some types) */ public BasicSqlType(RelDataTypeSystem typeSystem, SqlTypeName typeName, int precision) { - this(typeSystem, typeName, false, precision, SCALE_NOT_SPECIFIED); - assert typeName.allowsPrecScale(true, false) - : "typeName.allowsPrecScale(true, false)"; - computeDigest(); + this(typeSystem, typeName, false, precision, SCALE_NOT_SPECIFIED, null, + null); + checkPrecScale(typeName, true, false); } /** * Constructs a type with precision/length and scale. * + * @param typeSystem Type system * @param typeName Type name + * @param precision Precision (called length for some types) + * @param scale Scale */ public BasicSqlType(RelDataTypeSystem typeSystem, SqlTypeName typeName, int precision, int scale) { - this(typeSystem, typeName, false, precision, scale); - assert typeName.allowsPrecScale(true, true); - computeDigest(); + this(typeSystem, typeName, false, precision, scale, null, null); + checkPrecScale(typeName, true, true); } /** Internal constructor. */ @@ -86,61 +101,52 @@ public class BasicSqlType extends AbstractSqlType { SqlTypeName typeName, boolean nullable, int precision, - int scale) { + int scale, + SqlCollation collation, + SerializableCharset wrappedCharset) { super(typeName, nullable, null); - this.typeSystem = typeSystem; + this.typeSystem = Objects.requireNonNull(typeSystem); this.precision = precision; this.scale = scale; + this.collation = collation; + this.wrappedCharset = wrappedCharset; + computeDigest(); } //~ Methods ---------------------------------------------------------------- /** - * Constructs a type with nullablity + * Constructs a type with nullablity. */ BasicSqlType createWithNullability(boolean nullable) { - BasicSqlType ret; - try { - ret = (BasicSqlType) this.clone(); - } catch (CloneNotSupportedException e) { - throw new AssertionError(e); + if (nullable == this.isNullable) { + return this; } - ret.isNullable = nullable; - ret.computeDigest(); - return ret; + return new BasicSqlType(this.typeSystem, this.typeName, nullable, + this.precision, this.scale, this.collation, this.wrappedCharset); } /** * Constructs a type with charset and collation. * - * <p>This must be a character tyoe. + * <p>This must be a character type. */ - BasicSqlType createWithCharsetAndCollation( - Charset charset, + BasicSqlType createWithCharsetAndCollation(Charset charset, SqlCollation collation) { Preconditions.checkArgument(SqlTypeUtil.inCharFamily(this)); - BasicSqlType ret; - try { - ret = (BasicSqlType) this.clone(); - } catch (CloneNotSupportedException e) { - throw new AssertionError(e); - } - ret.wrappedCharset = SerializableCharset.forCharset(charset); - ret.collation = collation; - ret.computeDigest(); - return ret; + return new BasicSqlType(this.typeSystem, this.typeName, this.isNullable, + this.precision, this.scale, collation, + SerializableCharset.forCharset(charset)); } - // implement RelDataType - public int getPrecision() { + @Override public int getPrecision() { if (precision == PRECISION_NOT_SPECIFIED) { return typeSystem.getDefaultPrecision(typeName); } return precision; } - // implement RelDataType - public int getScale() { + @Override public int getScale() { if (scale == SCALE_NOT_SPECIFIED) { switch (typeName) { case TINYINT: @@ -156,13 +162,11 @@ public class BasicSqlType extends AbstractSqlType { return scale; } - // implement RelDataType - public Charset getCharset() { + @Override public Charset getCharset() { return wrappedCharset == null ? null : wrappedCharset.getCharset(); } - // implement RelDataType - public SqlCollation getCollation() { + @Override public SqlCollation getCollation() { return collation; } http://git-wip-us.apache.org/repos/asf/calcite/blob/08aefb09/core/src/main/java/org/apache/calcite/sql/type/SqlTypeFactoryImpl.java ---------------------------------------------------------------------- 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 91556ca..b1ee8dc 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 @@ -192,10 +192,9 @@ public class SqlTypeFactoryImpl extends RelDataTypeFactoryImpl { @Override public RelDataType createTypeWithNullability( final RelDataType type, final boolean nullable) { - RelDataType newType; + final RelDataType newType; if (type instanceof BasicSqlType) { - BasicSqlType sqlType = (BasicSqlType) type; - newType = sqlType.createWithNullability(nullable); + newType = ((BasicSqlType) type).createWithNullability(nullable); } else if (type instanceof MapSqlType) { newType = copyMapType(type, nullable); } else if (type instanceof ArraySqlType) {
