From d2e43d9d590fbdca04710617c9bd7d5a696bca5e Mon Sep 17 00:00:00 2001 From: Viggo Date: Tue, 28 May 2024 20:51:43 +0800 Subject: [PATCH 1/9] feat(isthmus): support PrecisionTimestamp in isthmus --- .../java/io/substrait/isthmus/SubstraitTypeSystem.java | 3 ++- .../main/java/io/substrait/isthmus/TypeConverter.java | 9 +++++++-- .../test/java/io/substrait/isthmus/CalciteTypeTest.java | 6 ++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java index 76106f5f..cef6b4bf 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java @@ -25,9 +25,10 @@ public int getMaxPrecision(final SqlTypeName typeName) { case INTERVAL_YEAR_MONTH: case TIME: case TIME_WITH_LOCAL_TIME_ZONE: + // TODO: change it case TIMESTAMP: case TIMESTAMP_WITH_LOCAL_TIME_ZONE: - return 6; + return 9; } return super.getMaxPrecision(typeName); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index 92ccecbe..4c6d55c7 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -97,11 +97,11 @@ private Type toSubstrait(RelDataType type, List names) { yield creator.TIME; } case TIMESTAMP -> { - if (type.getPrecision() != 6) { + if (type.getPrecision() > 9) { throw new UnsupportedOperationException( "unsupported timestamp precision " + type.getPrecision()); } - yield creator.TIMESTAMP; + yield creator.precisionTimestamp(type.getPrecision()); } case TIMESTAMP_WITH_LOCAL_TIME_ZONE -> { if (type.getPrecision() != 6) { @@ -241,6 +241,11 @@ public RelDataType visit(Type.Timestamp expr) { return t(n(expr), SqlTypeName.TIMESTAMP, 6); } + @Override + public RelDataType visit(Type.PrecisionTimestamp expr) { + return t(n(expr), SqlTypeName.TIMESTAMP, expr.precision()); + } + @Override public RelDataType visit(Type.IntervalYear expr) { return typeFactory.createTypeWithNullability( diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index 36fae668..fcce354d 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -78,6 +78,12 @@ void timestamp(boolean nullable) { testType(Type.withNullability(nullable).TIMESTAMP, SqlTypeName.TIMESTAMP, nullable, 6); } + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void precisionTimeStamp(boolean nullable) { + testType(Type.withNullability(nullable).precisionTimestamp(9), SqlTypeName.TIMESTAMP, nullable, 9); + } + @ParameterizedTest @ValueSource(booleans = {true, false}) void timestamptz(boolean nullable) { From eecd9f9ccbab9fe1921f41783161a31f66d7efb2 Mon Sep 17 00:00:00 2001 From: Viggo Date: Wed, 29 May 2024 15:16:59 +0800 Subject: [PATCH 2/9] feat(isthmus): spotless --- .../main/java/io/substrait/isthmus/SubstraitTypeSystem.java | 1 - .../src/test/java/io/substrait/isthmus/CalciteTypeTest.java | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java index cef6b4bf..4a87fcec 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java @@ -25,7 +25,6 @@ public int getMaxPrecision(final SqlTypeName typeName) { case INTERVAL_YEAR_MONTH: case TIME: case TIME_WITH_LOCAL_TIME_ZONE: - // TODO: change it case TIMESTAMP: case TIMESTAMP_WITH_LOCAL_TIME_ZONE: return 9; diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index fcce354d..ae6ad5be 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -81,7 +81,8 @@ void timestamp(boolean nullable) { @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimeStamp(boolean nullable) { - testType(Type.withNullability(nullable).precisionTimestamp(9), SqlTypeName.TIMESTAMP, nullable, 9); + testType( + Type.withNullability(nullable).precisionTimestamp(9), SqlTypeName.TIMESTAMP, nullable, 9); } @ParameterizedTest From 6dfc75edf47fe2ab11e84556ef0c7845acd9037d Mon Sep 17 00:00:00 2001 From: Viggo Date: Wed, 29 May 2024 16:59:48 +0800 Subject: [PATCH 3/9] feat(isthmus): support PrecisionTimestampTZ --- .../io/substrait/isthmus/TypeConverter.java | 29 +++++-------------- .../io/substrait/isthmus/CalciteTypeTest.java | 12 ++------ 2 files changed, 11 insertions(+), 30 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index 4c6d55c7..e11159f7 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -89,27 +89,9 @@ private Type toSubstrait(RelDataType type, List names) { } case SYMBOL -> creator.STRING; case DATE -> creator.DATE; - case TIME -> { - if (type.getPrecision() != 6) { - throw new UnsupportedOperationException( - "unsupported time precision " + type.getPrecision()); - } - yield creator.TIME; - } - case TIMESTAMP -> { - if (type.getPrecision() > 9) { - throw new UnsupportedOperationException( - "unsupported timestamp precision " + type.getPrecision()); - } - yield creator.precisionTimestamp(type.getPrecision()); - } - case TIMESTAMP_WITH_LOCAL_TIME_ZONE -> { - if (type.getPrecision() != 6) { - throw new UnsupportedOperationException( - "unsupported timestamptz precision " + type.getPrecision()); - } - yield creator.TIMESTAMP_TZ; - } + case TIME -> creator.TIME; + case TIMESTAMP -> creator.precisionTimestamp(type.getPrecision()); + case TIMESTAMP_WITH_LOCAL_TIME_ZONE -> creator.precisionTimestampTZ(type.getPrecision()); case INTERVAL_YEAR, INTERVAL_YEAR_MONTH, INTERVAL_MONTH -> creator.INTERVAL_YEAR; case INTERVAL_DAY, INTERVAL_DAY_HOUR, @@ -246,6 +228,11 @@ public RelDataType visit(Type.PrecisionTimestamp expr) { return t(n(expr), SqlTypeName.TIMESTAMP, expr.precision()); } + @Override + public RelDataType visit(Type.PrecisionTimestampTZ expr) throws RuntimeException { + return t(n(expr), SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, expr.precision()); + } + @Override public RelDataType visit(Type.IntervalYear expr) { return typeFactory.createTypeWithNullability( diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index ae6ad5be..01dd15f3 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -72,12 +72,6 @@ void time(boolean nullable) { testType(Type.withNullability(nullable).TIME, SqlTypeName.TIME, nullable, 6); } - @ParameterizedTest - @ValueSource(booleans = {true, false}) - void timestamp(boolean nullable) { - testType(Type.withNullability(nullable).TIMESTAMP, SqlTypeName.TIMESTAMP, nullable, 6); - } - @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimeStamp(boolean nullable) { @@ -87,12 +81,12 @@ void precisionTimeStamp(boolean nullable) { @ParameterizedTest @ValueSource(booleans = {true, false}) - void timestamptz(boolean nullable) { + void precisionTimestamptz(boolean nullable) { testType( - Type.withNullability(nullable).TIMESTAMP_TZ, + Type.withNullability(nullable).precisionTimestampTZ(9), SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, nullable, - 6); + 9); } @ParameterizedTest From 32f957b63801748e6b42be15be95510c57ec2f45 Mon Sep 17 00:00:00 2001 From: Viggo Date: Fri, 31 May 2024 14:06:09 +0800 Subject: [PATCH 4/9] feat(isthmus): support precisionTimestamp & PrecisionTimestamp --- .../io/substrait/isthmus/SubstraitTypeSystem.java | 2 +- .../io/substrait/isthmus/CalciteTypeTest.java | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java index 4a87fcec..76106f5f 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java @@ -27,7 +27,7 @@ public int getMaxPrecision(final SqlTypeName typeName) { case TIME_WITH_LOCAL_TIME_ZONE: case TIMESTAMP: case TIMESTAMP_WITH_LOCAL_TIME_ZONE: - return 9; + return 6; } return super.getMaxPrecision(typeName); } diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index 01dd15f3..e19f6165 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -75,18 +75,19 @@ void time(boolean nullable) { @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimeStamp(boolean nullable) { - testType( - Type.withNullability(nullable).precisionTimestamp(9), SqlTypeName.TIMESTAMP, nullable, 9); + for (int precision : new int[]{0, 3, 6}) { + testType( + Type.withNullability(nullable).precisionTimestamp(precision), SqlTypeName.TIMESTAMP, nullable, precision); + } } @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimestamptz(boolean nullable) { - testType( - Type.withNullability(nullable).precisionTimestampTZ(9), - SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, - nullable, - 9); + for (int precision : new int[]{0, 3, 6}) { + testType( + Type.withNullability(nullable).precisionTimestampTZ(precision), SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, nullable, precision); + } } @ParameterizedTest From df4a4861868e214b61c7d3ace9ae899dad8fcfd5 Mon Sep 17 00:00:00 2001 From: Viggo Date: Sat, 1 Jun 2024 09:11:52 +0800 Subject: [PATCH 5/9] chore: spotless --- .../java/io/substrait/isthmus/CalciteTypeTest.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index 8ce36611..20cbd962 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -84,18 +84,24 @@ void time(boolean nullable) { @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimeStamp(boolean nullable) { - for (int precision : new int[]{0, 3, 6}) { + for (int precision : new int[] {0, 3, 6}) { testType( - Type.withNullability(nullable).precisionTimestamp(precision), SqlTypeName.TIMESTAMP, nullable, precision); + Type.withNullability(nullable).precisionTimestamp(precision), + SqlTypeName.TIMESTAMP, + nullable, + precision); } } @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimestamptz(boolean nullable) { - for (int precision : new int[]{0, 3, 6}) { + for (int precision : new int[] {0, 3, 6}) { testType( - Type.withNullability(nullable).precisionTimestampTZ(precision), SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, nullable, precision); + Type.withNullability(nullable).precisionTimestampTZ(precision), + SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, + nullable, + precision); } } From ca93f9803f7c4629024ea7b44a2652760f2e5610 Mon Sep 17 00:00:00 2001 From: Viggo Date: Tue, 11 Jun 2024 22:41:07 +0800 Subject: [PATCH 6/9] feat(isthmus): check max precision in ToRelDataType --- .../main/java/io/substrait/isthmus/TypeConverter.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index 263afff4..4257596f 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -225,11 +225,21 @@ public RelDataType visit(Type.Timestamp expr) { @Override public RelDataType visit(Type.PrecisionTimestamp expr) { + int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); + if (expr.precision() > maxPrecision) { + throw new UnsupportedOperationException( + "unsupported timestamp precision " + expr.precision() + ", max precision in calcite type system is " + maxPrecision); + } return t(n(expr), SqlTypeName.TIMESTAMP, expr.precision()); } @Override public RelDataType visit(Type.PrecisionTimestampTZ expr) throws RuntimeException { + int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); + if (expr.precision() > maxPrecision) { + throw new UnsupportedOperationException( + "unsupported timestamp_tz precision " + expr.precision() + ", max precision in calcite type system is " + maxPrecision); + } return t(n(expr), SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, expr.precision()); } From 679e8bb5ad746a7bd2e4ec89462d240bbc1a47b7 Mon Sep 17 00:00:00 2001 From: Victor Barua Date: Tue, 11 Jun 2024 13:16:36 -0700 Subject: [PATCH 7/9] refactor: use String.format + apply spotless --- .../src/main/java/io/substrait/isthmus/TypeConverter.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index 4257596f..fb65291f 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -228,7 +228,9 @@ public RelDataType visit(Type.PrecisionTimestamp expr) { int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); if (expr.precision() > maxPrecision) { throw new UnsupportedOperationException( - "unsupported timestamp precision " + expr.precision() + ", max precision in calcite type system is " + maxPrecision); + String.format( + "unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s", + expr.precision(), maxPrecision)); } return t(n(expr), SqlTypeName.TIMESTAMP, expr.precision()); } @@ -238,7 +240,9 @@ public RelDataType visit(Type.PrecisionTimestampTZ expr) throws RuntimeException int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); if (expr.precision() > maxPrecision) { throw new UnsupportedOperationException( - "unsupported timestamp_tz precision " + expr.precision() + ", max precision in calcite type system is " + maxPrecision); + String.format( + "unsupported precision_timestamp_tz precision %s, max precision in Calcite type system is set to %s", + expr.precision(), maxPrecision)); } return t(n(expr), SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, expr.precision()); } From 46d0e7df880c3ea275ea32322e6f271bd15c3146 Mon Sep 17 00:00:00 2001 From: Victor Barua Date: Tue, 11 Jun 2024 13:17:53 -0700 Subject: [PATCH 8/9] fix: use correct SqlTypeName to fetch precision_timestamp_tz Calcite precision --- isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index fb65291f..57c77ed3 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -237,7 +237,7 @@ public RelDataType visit(Type.PrecisionTimestamp expr) { @Override public RelDataType visit(Type.PrecisionTimestampTZ expr) throws RuntimeException { - int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); + int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE); if (expr.precision() > maxPrecision) { throw new UnsupportedOperationException( String.format( From 4aea043f7b872e07c05175a8e27d5bbf03e5900a Mon Sep 17 00:00:00 2001 From: Victor Barua Date: Tue, 11 Jun 2024 14:43:57 -0700 Subject: [PATCH 9/9] refactor: spotless --- isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index 57c77ed3..e4ce6700 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -237,7 +237,8 @@ public RelDataType visit(Type.PrecisionTimestamp expr) { @Override public RelDataType visit(Type.PrecisionTimestampTZ expr) throws RuntimeException { - int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE); + int maxPrecision = + typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE); if (expr.precision() > maxPrecision) { throw new UnsupportedOperationException( String.format(