From af6090ab58287bd1a711d9243f9e60610f1d6091 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 7 Jun 2022 11:45:58 +0200 Subject: [PATCH 1/5] Add Analyzer test for time travel Add tests `StatementAnalyzer` validation for the `FOR ... AS OF ...` syntax. --- .../io/trino/sql/analyzer/TestAnalyzer.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java index a7375e7f8e46..ebda089cef4b 100644 --- a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java +++ b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java @@ -366,6 +366,95 @@ public void testSelectAllColumns() .hasErrorCode(COLUMN_NOT_FOUND); } + @Test + public void testTemporalTableVersion() + { + // valid temporal version pointer + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF DATE '2022-01-01'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF TIMESTAMP '2022-01-01'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF TIMESTAMP '2022-01-01 01:02:03.123456789012'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF TIMESTAMP '2022-01-01 UTC'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF TIMESTAMP '2022-01-01 01:02:03.123456789012 Asia/Kathmandu'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + + // wrong type + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF '2022-01-01'") + .hasErrorCode(TYPE_MISMATCH) + .hasMessage("line 1:18: Type varchar(10) invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date."); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF '2022-01-01 01:02:03'") + .hasErrorCode(TYPE_MISMATCH) + .hasMessage("line 1:18: Type varchar(19) invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date."); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF '2022-01-01 01:02:03 UTC'") + .hasErrorCode(TYPE_MISMATCH) + .hasMessage("line 1:18: Type varchar(23) invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date."); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF 1654594283421") + .hasErrorCode(TYPE_MISMATCH) + .hasMessage("line 1:18: Type bigint invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date."); + + // null value with right type + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF CAST(NULL AS date)") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF CAST(NULL AS timestamp(3))") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF CAST(NULL AS timestamp(3) with time zone)") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + + // null value with wrong type + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF NULL") + .hasErrorCode(TYPE_MISMATCH) + .hasMessage("UNKNOWN is not a valid type"); + assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF CAST(NULL AS bigint)") + .hasErrorCode(TYPE_MISMATCH) + .hasMessage("line 1:18: Type bigint invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date."); + } + + @Test + public void testRangeIdTableVersion() + { + // integer + assertFails("SELECT * FROM t1 FOR VERSION AS OF 123") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + + // bigint + assertFails("SELECT * FROM t1 FOR VERSION AS OF BIGINT '123'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + + // varchar + assertFails("SELECT * FROM t1 FOR VERSION AS OF '2022-01-01'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + + // date + assertFails("SELECT * FROM t1 FOR VERSION AS OF DATE '2022-01-01'") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + + // null value + assertFails("SELECT * FROM t1 FOR VERSION AS OF NULL") + .hasErrorCode(TYPE_MISMATCH) + .hasMessage("UNKNOWN is not a valid type"); + assertFails("SELECT * FROM t1 FOR VERSION AS OF CAST(NULL AS bigint)") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + assertFails("SELECT * FROM t1 FOR VERSION AS OF CAST(NULL AS varchar)") + .hasErrorCode(NOT_SUPPORTED) + .hasMessage("This connector does not support versioned tables"); + } + @Test public void testGroupByWithWildcard() { From 44b743d5a3115a9176f50ed3400cab7e02c096a1 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 7 Jun 2022 11:58:57 +0200 Subject: [PATCH 2/5] Extract version pointer validation method --- .../trino/sql/analyzer/StatementAnalyzer.java | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java index 34878e643eeb..6d7f1ac43a4d 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java @@ -4563,34 +4563,27 @@ private Optional extractTableVersion(Table table, QualifiedObjectN PointerType pointerType = toPointerType(table.getQueryPeriod().get().getRangeType()); Object evaluatedVersion = evaluateConstantExpression(version.get(), versionType, plannerContext, session, accessControl, ImmutableMap.of()); TableVersion extractedVersion = new TableVersion(pointerType, versionType, evaluatedVersion); + validateVersionPointer(tableName, table.getQueryPeriod().get(), extractedVersion); + return Optional.of(extractedVersion); + } - // Before checking if the connector supports the version type, verify that version is a valid time-based type + private void validateVersionPointer(QualifiedObjectName tableName, QueryPeriod queryPeriod, TableVersion extractedVersion) + { + Type type = extractedVersion.getObjectType(); if (extractedVersion.getPointerType() == PointerType.TEMPORAL) { - if (!isValidTemporalType(extractedVersion.getObjectType())) { - throw semanticException( - TYPE_MISMATCH, - table.getQueryPeriod().get(), - format( - "Type %s invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date.", - extractedVersion.getObjectType().getDisplayName())); + // Before checking if the connector supports the version type, verify that version is a valid time-based type + if (!(type instanceof TimestampWithTimeZoneType || + type instanceof TimestampType || + type instanceof DateType)) { + throw semanticException(TYPE_MISMATCH, queryPeriod, format( + "Type %s invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date.", + type.getDisplayName())); } } if (!metadata.isValidTableVersion(session, tableName, extractedVersion)) { - throw semanticException( - TYPE_MISMATCH, - table.getQueryPeriod().get(), - format("Type %s not supported by this connector.", extractedVersion.getObjectType().getDisplayName())); + throw semanticException(TYPE_MISMATCH, queryPeriod, format("Type %s not supported by this connector.", type.getDisplayName())); } - - return Optional.of(extractedVersion); - } - - private boolean isValidTemporalType(Type type) - { - return (type instanceof TimestampWithTimeZoneType || - type instanceof TimestampType || - type instanceof DateType); } private PointerType toPointerType(QueryPeriod.RangeType type) From f5bd46aafb3cba30d374ad8ee14d093a2d64ac86 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 7 Jun 2022 12:05:20 +0200 Subject: [PATCH 3/5] Throw proper error for AS OF NULL Previously query would fail with "UNKNOWN is not a valid type", which can be confusing, since user did not specify "UNKNOWN" anywhere in the query. --- .../java/io/trino/sql/analyzer/StatementAnalyzer.java | 3 +++ .../src/test/java/io/trino/sql/analyzer/TestAnalyzer.java | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java index 6d7f1ac43a4d..30a82fcd38c9 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java @@ -4561,6 +4561,9 @@ private Optional extractTableVersion(Table table, QualifiedObjectN // Once the range value is analyzed, we can evaluate it Type versionType = expressionAnalysis.getType(version.get()); PointerType pointerType = toPointerType(table.getQueryPeriod().get().getRangeType()); + if (versionType == UNKNOWN) { + throw semanticException(INVALID_ARGUMENTS, table.getQueryPeriod().get(), "Pointer value cannot be NULL"); + } Object evaluatedVersion = evaluateConstantExpression(version.get(), versionType, plannerContext, session, accessControl, ImmutableMap.of()); TableVersion extractedVersion = new TableVersion(pointerType, versionType, evaluatedVersion); validateVersionPointer(tableName, table.getQueryPeriod().get(), extractedVersion); diff --git a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java index ebda089cef4b..d19a9d607b36 100644 --- a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java +++ b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java @@ -413,8 +413,8 @@ public void testTemporalTableVersion() // null value with wrong type assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF NULL") - .hasErrorCode(TYPE_MISMATCH) - .hasMessage("UNKNOWN is not a valid type"); + .hasErrorCode(INVALID_ARGUMENTS) + .hasMessage("line 1:18: Pointer value cannot be NULL"); assertFails("SELECT * FROM t1 FOR TIMESTAMP AS OF CAST(NULL AS bigint)") .hasErrorCode(TYPE_MISMATCH) .hasMessage("line 1:18: Type bigint invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date."); @@ -445,8 +445,8 @@ public void testRangeIdTableVersion() // null value assertFails("SELECT * FROM t1 FOR VERSION AS OF NULL") - .hasErrorCode(TYPE_MISMATCH) - .hasMessage("UNKNOWN is not a valid type"); + .hasErrorCode(INVALID_ARGUMENTS) + .hasMessage("line 1:18: Pointer value cannot be NULL"); assertFails("SELECT * FROM t1 FOR VERSION AS OF CAST(NULL AS bigint)") .hasErrorCode(NOT_SUPPORTED) .hasMessage("This connector does not support versioned tables"); From e25508f4dd496e362a720961435695bbe9ae02da Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Tue, 7 Jun 2022 19:52:30 +0200 Subject: [PATCH 4/5] empty From c766746ad6ada85c31862b2b2c9e1da3e4b0055b Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Wed, 8 Jun 2022 17:01:26 +0200 Subject: [PATCH 5/5] empty