For the three types that carry a length across the Substrait boundary — {@link + * SqlTypeName#CHAR}, {@link SqlTypeName#VARCHAR} and {@link SqlTypeName#BINARY}, holding {@code + * fixedchar}, {@code varchar} and {@code fixedbinary} — this is Substrait's own limit: those + * lengths are 32-bit integers. Calcite's default of 65536 is narrower, and the type factory caps + * a converted type at it rather than reporting that it cannot represent the declared width. + * + *
{@link SqlTypeName#VARBINARY} is raised with them even though Substrait's {@code binary}
+ * carries no length of its own, because the cap bites inside Calcite's own type unification:
+ * {@link #shouldConvertRaggedUnionTypesToVarying()} is true here, so a union of fixed-width
+ * binaries of different widths is unified as a {@code VARBINARY} of the widest. Left at 65536,
+ * the least restrictive type of {@code BINARY(100000)} and {@code BINARY(5)} is {@code
+ * VARBINARY(65536)} -- narrower than one of its own inputs, and the cap this method removes
+ * reimposed.
+ *
* @param typeName The {@link SqlTypeName} for which precision is requested.
* @return Maximum precision for the type.
*/
@Override
public int getMaxPrecision(final SqlTypeName typeName) {
switch (typeName) {
+ case CHAR:
+ case VARCHAR:
+ case BINARY:
+ case VARBINARY:
+ return Integer.MAX_VALUE;
case INTERVAL_DAY:
case INTERVAL_YEAR:
case INTERVAL_YEAR_MONTH:
diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java
index 754387694..fb2779bd2 100644
--- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java
+++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java
@@ -31,6 +31,9 @@
*/
public class TypeConverter {
+ /** The widest precision the spec gives a decimal: {@code DECIMAL A negative length is refused before the factory is asked, because asking tells us nothing:
+ * with assertions off the factory stores the negative and reports it back, so the width below
+ * certifies itself, and with them on Calcite raises a bare {@code AssertionError} in place of
+ * this message. A length of -1 is Calcite's unspecified precision besides, so the factory
+ * answers with an unparameterised type whose precision equals what was asked for. A zero length
+ * is left to the factory: the spec puts a fixedchar's width at 1 or more, but Calcite types the
+ * empty character literal as a {@code CHAR(0)}, so plans carrying one exist.
+ *
+ * @param nullable whether the type is nullable
+ * @param typeName the Calcite type name to build
+ * @param length the declared length
+ * @return the built type
+ * @throws IllegalArgumentException if the length is negative, or the factory built a type of
+ * another length
+ */
+ private RelDataType withLength(boolean nullable, SqlTypeName typeName, int length) {
+ if (length < 0) {
+ throw new IllegalArgumentException(
+ String.format(
+ "A %s cannot declare a negative length, and this one is %d", typeName, length));
+ }
+ RelDataType type = t(nullable, typeName, length);
+ if (type.getPrecision() != length) {
+ throw new IllegalArgumentException(
+ String.format(
+ "The type factory cannot hold %s(%d), which it narrowed to %s; its type system"
+ + " allows up to %d",
+ typeName, length, type, typeFactory.getTypeSystem().getMaxPrecision(typeName)));
+ }
+ return type;
}
@Override
public RelDataType visit(Type.Decimal expr) {
+ // Before the factory, for the reason the lengths are: -1 is Calcite's unspecified precision,
+ // so a negative one is answered with an unparameterised DECIMAL whose precision reads back as
+ // the type system's maximum. A zero or negative scale Calcite reports itself.
+ if (expr.precision() < 0) {
+ throw new IllegalArgumentException(
+ String.format(
+ "A decimal cannot declare a negative precision, and this one is %d",
+ expr.precision()));
+ }
+ // The spec's own ceiling, not just the factory's: handed a type system whose DECIMAL maximum
+ // is above it, the factory builds the type and the outbound conversion above then refuses it,
+ // so the type would convert in and have no way back.
+ if (expr.precision() > MAX_DECIMAL_PRECISION) {
+ throw new IllegalArgumentException(
+ String.format(
+ "A decimal cannot declare a precision of %d, above the %d the spec allows",
+ expr.precision(), MAX_DECIMAL_PRECISION));
+ }
+ SubstraitTypeSystem.requireSupportedPrecision(
+ typeFactory.getTypeSystem(), SqlTypeName.DECIMAL, "decimal", expr.precision());
+ // The spec puts a decimal's scale in [0..P]. No factory reports a scale above the precision:
+ // Calcite builds the type as asked, and its own maximum cannot catch it either, since both
+ // type systems here set maxScale equal to maxPrecision.
+ if (expr.scale() > expr.precision()) {
+ throw new IllegalArgumentException(
+ String.format(
+ "A decimal cannot declare a scale of %d above its precision of %d",
+ expr.scale(), expr.precision()));
+ }
return t(n(expr), SqlTypeName.DECIMAL, expr.precision(), expr.scale());
}
diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java
index 50199128f..167375636 100644
--- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java
+++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java
@@ -15,6 +15,7 @@
import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
class CalciteTypeTest extends CalciteObjs {
@@ -220,6 +221,44 @@ void aJavaCharColumnTakesCalcitesDefaultWidth() {
TypeConverter.DEFAULT.toSubstrait(javaTypeFactory.createJavaType(Character.class)));
}
+ /**
+ * A width above Calcite's default 65536 cap, for each type whose length crosses the Substrait
+ * boundary. The expected precision is asserted directly rather than through {@link #testType},
+ * whose expectation is built with the same type factory and would be narrowed alongside the value
+ * under test.
+ */
+ @ParameterizedTest
+ @CsvSource({
+ "CHAR, 65537",
+ "CHAR, 2147483647",
+ "VARCHAR, 65537",
+ "VARCHAR, 2147483647",
+ "BINARY, 65537",
+ "BINARY, 2147483647"
+ })
+ void wideLengthCarryingTypesKeepTheirLength(SqlTypeName typeName, int length) {
+ TypeExpression substrait;
+ switch (typeName) {
+ case CHAR:
+ substrait = TypeCreator.REQUIRED.fixedChar(length);
+ break;
+ case VARCHAR:
+ substrait = TypeCreator.REQUIRED.varChar(length);
+ break;
+ case BINARY:
+ substrait = TypeCreator.REQUIRED.fixedBinary(length);
+ break;
+ default:
+ throw new IllegalArgumentException("no Substrait type mapped for " + typeName);
+ }
+
+ RelDataType calcite = TypeConverter.DEFAULT.toCalcite(type, substrait, null);
+
+ assertEquals(typeName, calcite.getSqlTypeName());
+ assertEquals(length, calcite.getPrecision());
+ assertEquals(substrait, TypeConverter.DEFAULT.toSubstrait(calcite));
+ }
+
@ParameterizedTest
@ValueSource(booleans = {true, false})
void decimal(boolean nullable) {
diff --git a/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java b/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java
index 01e7bd0c7..03dd4c77f 100644
--- a/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java
+++ b/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java
@@ -1,10 +1,21 @@
package io.substrait.isthmus;
import static io.substrait.isthmus.SubstraitTypeSystem.TYPE_FACTORY;
+import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import io.substrait.isthmus.sql.SubstraitCreateStatementParser;
+import io.substrait.plan.Plan;
+import io.substrait.type.TypeCreator;
+import java.util.List;
+import org.apache.calcite.prepare.CalciteCatalogReader;
import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeSystem;
+import org.apache.calcite.rel.type.RelDataTypeSystemImpl;
+import org.apache.calcite.sql.type.SqlTypeFactoryImpl;
import org.apache.calcite.sql.type.SqlTypeName;
import org.junit.jupiter.api.Test;
@@ -42,6 +53,114 @@ void timeMaxPrecision() {
assertEquals(6, typeSystem.getMaxPrecision(SqlTypeName.TIME));
}
+ @Test
+ void lengthCarryingTypesMaxPrecisionIsSubstraitsOwnLimit() {
+ assertEquals(Integer.MAX_VALUE, typeSystem.getMaxPrecision(SqlTypeName.VARCHAR));
+ assertEquals(Integer.MAX_VALUE, typeSystem.getMaxPrecision(SqlTypeName.CHAR));
+ assertEquals(Integer.MAX_VALUE, typeSystem.getMaxPrecision(SqlTypeName.BINARY));
+ // Substrait's binary carries no length of its own, but Calcite unifies a ragged binary union
+ // through this type, so a cap here caps the union.
+ assertEquals(Integer.MAX_VALUE, typeSystem.getMaxPrecision(SqlTypeName.VARBINARY));
+ }
+
+ /**
+ * Calcite's default caps a character type at 65536, which is narrower than the {@code int} length
+ * Substrait declares, so a wider converted type would be silently narrowed by the type factory.
+ */
+ @Test
+ void lengthCarryingTypesMaxPrecisionDiffersFromDefaultTypeSystem() {
+ assertEquals(65536, RelDataTypeSystem.DEFAULT.getMaxPrecision(SqlTypeName.VARCHAR));
+ assertEquals(65536, RelDataTypeSystem.DEFAULT.getMaxPrecision(SqlTypeName.CHAR));
+ assertEquals(65536, RelDataTypeSystem.DEFAULT.getMaxPrecision(SqlTypeName.BINARY));
+ }
+
+ @Test
+ void canCreateCharacterTypesWiderThanTheCalciteDefault() {
+ assertEquals(100_000, TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR, 100_000).getPrecision());
+ assertEquals(100_000, TYPE_FACTORY.createSqlType(SqlTypeName.CHAR, 100_000).getPrecision());
+ assertEquals(100_000, TYPE_FACTORY.createSqlType(SqlTypeName.BINARY, 100_000).getPrecision());
+ }
+
+ /**
+ * The conversion takes whatever type factory it is handed, and one built on Calcite's default
+ * type system cannot hold these widths. Narrowing them is what this fix is about, so a factory
+ * that would narrow is reported rather than followed.
+ */
+ @Test
+ void aFactoryThatCannotHoldTheDeclaredLengthIsReported() {
+ RelDataTypeFactory defaultFactory = new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ defaultFactory, TypeCreator.REQUIRED.varChar(100_000), null));
+ assertTrue(e.getMessage().contains("allows up to 65536"), e.getMessage());
+
+ assertEquals(
+ 100_000,
+ TypeConverter.DEFAULT
+ .toCalcite(TYPE_FACTORY, TypeCreator.REQUIRED.varChar(100_000), null)
+ .getPrecision());
+ }
+
+ /**
+ * Asking the factory first cannot tell a negative width from a width it holds: with assertions
+ * off it stores the negative and reports it back, and -1 is its unspecified precision besides, so
+ * the answer equals what was asked for either way.
+ */
+ @Test
+ void aNegativeLengthIsRefusedBeforeTheFactoryIsAsked() {
+ assertAll(
+ () -> {
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ TYPE_FACTORY, TypeCreator.REQUIRED.varChar(-5), null));
+ assertTrue(
+ e.getMessage().contains("negative length, and this one is -5"), e.getMessage());
+ },
+ () -> {
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ TYPE_FACTORY, TypeCreator.REQUIRED.fixedChar(-1), null));
+ assertTrue(
+ e.getMessage().contains("negative length, and this one is -1"), e.getMessage());
+ },
+ () -> {
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ TYPE_FACTORY, TypeCreator.REQUIRED.fixedBinary(-5), null));
+ assertTrue(
+ e.getMessage().contains("negative length, and this one is -5"), e.getMessage());
+ });
+ }
+
+ /**
+ * A union of fixed-width binaries of different widths is unified as a VARBINARY, so leaving that
+ * type at Calcite's default would reimpose the cap the wide types are raised past -- on a type
+ * wider than one of the union's own inputs.
+ */
+ @Test
+ void aRaggedBinaryUnionKeepsTheWidestWidth() {
+ RelDataType wide = TYPE_FACTORY.createSqlType(SqlTypeName.BINARY, 100_000);
+ RelDataType narrow = TYPE_FACTORY.createSqlType(SqlTypeName.BINARY, 5);
+
+ RelDataType unified = TYPE_FACTORY.leastRestrictive(List.of(wide, narrow));
+
+ assertEquals(SqlTypeName.VARBINARY, unified.getSqlTypeName());
+ assertEquals(100_000, unified.getPrecision());
+ }
+
@Test
void canCreateDecimalWithMaxPrecision() {
RelDataType decimalType = TYPE_FACTORY.createSqlType(SqlTypeName.DECIMAL, 38, 10);
@@ -60,4 +179,134 @@ void decimalMaxPrecisionAndScaleDifferentFromDefaultTypeSystem() {
assertEquals(38, typeSystem.getMaxPrecision(SqlTypeName.DECIMAL));
assertEquals(38, typeSystem.getMaxScale(SqlTypeName.DECIMAL));
}
+
+ /**
+ * A type system whose DECIMAL maximum is above the spec's 38 would let a wider decimal through
+ * the factory, and {@code toSubstrait} refuses it on the way back -- so the type would convert in
+ * with no way out. The bound is the spec's rather than the factory's for that reason.
+ */
+ @Test
+ void aDecimalPrecisionAboveTheSpecsCeilingIsRefused() {
+ RelDataTypeFactory wideFactory =
+ new SqlTypeFactoryImpl(
+ new RelDataTypeSystemImpl() {
+ @Override
+ public int getMaxPrecision(SqlTypeName typeName) {
+ return typeName == SqlTypeName.DECIMAL ? 76 : super.getMaxPrecision(typeName);
+ }
+
+ @Override
+ public int getMaxScale(SqlTypeName typeName) {
+ return typeName == SqlTypeName.DECIMAL ? 76 : super.getMaxScale(typeName);
+ }
+ });
+
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ wideFactory, TypeCreator.REQUIRED.decimal(45, 2), null));
+
+ assertTrue(e.getMessage().contains("above the 38 the spec allows"), e.getMessage());
+ }
+
+ /**
+ * Two ends the factory does not report. A precision of -1 is Calcite's unspecified precision, so
+ * it answers with an unparameterised DECIMAL whose precision reads back as the type system's
+ * maximum. A scale above the precision is outside the spec's {@code 0 <= S <= P} and Calcite
+ * builds it anyway, where its own maximum would not catch it: both type systems here set {@code
+ * maxScale} equal to {@code maxPrecision}, so a scale within the precision is within that too.
+ * Calcite reports a zero or negative scale and a zero precision itself.
+ */
+ @Test
+ void aDecimalParameterOutsideItsDeclaredBoundsIsRefused() {
+ assertAll(
+ () -> {
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ TYPE_FACTORY, TypeCreator.REQUIRED.decimal(-1, 0), null));
+ assertTrue(
+ e.getMessage().contains("negative precision, and this one is -1"), e.getMessage());
+ },
+ () -> {
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ TYPE_FACTORY, TypeCreator.REQUIRED.decimal(19, 25), null));
+ assertTrue(e.getMessage().contains("scale of 25 above its precision"), e.getMessage());
+ },
+ () ->
+ assertEquals(
+ 19,
+ TypeConverter.DEFAULT
+ .toCalcite(TYPE_FACTORY, TypeCreator.REQUIRED.decimal(19, 19), null)
+ .getScale()));
+ }
+
+ /**
+ * The raised maximum is also Calcite's overflow threshold for concatenation, in {@code
+ * ReturnTypes.DYADIC_STRING_SUM_PRECISION}: a sum of widths past it falls back to an
+ * unparameterised type. So two columns nowhere near the old cap decide the result type between
+ * them, and the conversion emitted a {@code string} for them before.
+ */
+ @Test
+ void concatenatingTwoVarcharsKeepsTheSumOfTheirWidths() throws Exception {
+ CalciteCatalogReader catalog =
+ SubstraitCreateStatementParser.processCreateStatementsToCatalog(
+ "CREATE TABLE t (a VARCHAR(40000), b VARCHAR(40000))");
+
+ Plan plan = new SqlToSubstrait().convert("SELECT a || b FROM t", catalog);
+
+ assertEquals(
+ List.of(TypeCreator.NULLABLE.varChar(80000)),
+ plan.getRoots().get(0).getInput().getRecordType().fields());
+ }
+
+ /**
+ * A decimal loses its precision to a foreign factory the same silent way a length does: Calcite's
+ * default type system caps it at 19, and the narrowed type reads back as one the plan never
+ * declared.
+ */
+ @Test
+ void aFactoryThatCannotHoldTheDeclaredPrecisionIsReported() {
+ RelDataTypeFactory defaultFactory = new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ TypeConverter.DEFAULT.toCalcite(
+ defaultFactory, TypeCreator.REQUIRED.decimal(38, 10), null));
+ assertTrue(e.getMessage().contains("is set to 19"), e.getMessage());
+
+ assertEquals(
+ 38,
+ TypeConverter.DEFAULT
+ .toCalcite(TYPE_FACTORY, TypeCreator.REQUIRED.decimal(38, 10), null)
+ .getPrecision());
+ }
+
+ /**
+ * The cap reaches the conversion from SQL as well. Calcite narrows a declared width to its
+ * maximum silently rather than reporting that it cannot hold it, so before this a cast wider than
+ * the default came out of the conversion as a {@code varchar<65536>}.
+ */
+ @Test
+ void aWideVarcharDeclaredInSqlKeepsItsLength() throws Exception {
+ CalciteCatalogReader catalog =
+ SubstraitCreateStatementParser.processCreateStatementsToCatalog(
+ "CREATE TABLE t (a VARCHAR(10))");
+
+ Plan plan = new SqlToSubstrait().convert("SELECT CAST(a AS VARCHAR(100000)) FROM t", catalog);
+
+ assertEquals(
+ List.of(TypeCreator.NULLABLE.varChar(100000)),
+ plan.getRoots().get(0).getInput().getRecordType().fields());
+ }
}