From 901e7146e2825057e00d7cc699351ec641862a58 Mon Sep 17 00:00:00 2001 From: Marius Grama Date: Fri, 24 Feb 2023 14:19:41 +0100 Subject: [PATCH] Respect `hive.timestamp-precision` in Hive views --- .../io/trino/plugin/hive/HiveMetadata.java | 7 ++++-- .../plugin/hive/HiveMetadataFactory.java | 11 +++++++--- .../io/trino/plugin/hive/ViewReaderUtil.java | 22 ++++++++++++------- .../trino/plugin/hive/AbstractTestHive.java | 3 ++- .../plugin/hive/BaseHiveConnectorTest.java | 18 --------------- .../product/hive/AbstractTestHiveViews.java | 20 +++++------------ 6 files changed, 35 insertions(+), 46 deletions(-) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java index 4e6ce6340c8c..44b890974ba2 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java @@ -386,6 +386,7 @@ public class HiveMetadata private final PartitionProjectionService partitionProjectionService; private final boolean allowTableRename; private final long maxPartitionDropsPerQuery; + private final HiveTimestampPrecision hiveViewsTimestampPrecision; public HiveMetadata( CatalogName catalogName, @@ -412,7 +413,8 @@ public HiveMetadata( DirectoryLister directoryLister, PartitionProjectionService partitionProjectionService, boolean allowTableRename, - long maxPartitionDropsPerQuery) + long maxPartitionDropsPerQuery, + HiveTimestampPrecision hiveViewsTimestampPrecision) { this.catalogName = requireNonNull(catalogName, "catalogName is null"); this.metastore = requireNonNull(metastore, "metastore is null"); @@ -439,6 +441,7 @@ public HiveMetadata( this.partitionProjectionService = requireNonNull(partitionProjectionService, "partitionProjectionService is null"); this.allowTableRename = allowTableRename; this.maxPartitionDropsPerQuery = maxPartitionDropsPerQuery; + this.hiveViewsTimestampPrecision = requireNonNull(hiveViewsTimestampPrecision, "hiveViewsTimestampPrecision is null"); } @Override @@ -2622,7 +2625,7 @@ private Optional toConnectorViewDefinition(ConnectorSes throw new HiveViewNotSupportedException(viewName); } - ConnectorViewDefinition definition = createViewReader(metastore, session, view, typeManager, this::redirectTable, metadataProvider, hiveViewsRunAsInvoker) + ConnectorViewDefinition definition = createViewReader(metastore, session, view, typeManager, this::redirectTable, metadataProvider, hiveViewsRunAsInvoker, hiveViewsTimestampPrecision) .decodeViewData(view.getViewOriginalText().get(), view, catalogName); // use owner from table metadata if it exists if (view.getOwner().isPresent() && !definition.isRunAsInvoker()) { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadataFactory.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadataFactory.java index fe033ebb4c25..69027b8f6644 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadataFactory.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadataFactory.java @@ -79,6 +79,7 @@ public class HiveMetadataFactory private final long perTransactionFileStatusCacheMaximumSize; private final PartitionProjectionService partitionProjectionService; private final boolean allowTableRename; + private final HiveTimestampPrecision hiveViewsTimestampPrecision; @Inject public HiveMetadataFactory( @@ -138,7 +139,8 @@ public HiveMetadataFactory( directoryLister, hiveConfig.getPerTransactionFileStatusCacheMaximumSize(), partitionProjectionService, - allowTableRename); + allowTableRename, + hiveConfig.getTimestampPrecision()); } public HiveMetadataFactory( @@ -175,7 +177,8 @@ public HiveMetadataFactory( DirectoryLister directoryLister, long perTransactionFileStatusCacheMaximumSize, PartitionProjectionService partitionProjectionService, - boolean allowTableRename) + boolean allowTableRename, + HiveTimestampPrecision hiveViewsTimestampPrecision) { this.catalogName = requireNonNull(catalogName, "catalogName is null"); this.skipDeletionForAlter = skipDeletionForAlter; @@ -218,6 +221,7 @@ public HiveMetadataFactory( this.perTransactionFileStatusCacheMaximumSize = perTransactionFileStatusCacheMaximumSize; this.partitionProjectionService = requireNonNull(partitionProjectionService, "partitionProjectionService is null"); this.allowTableRename = allowTableRename; + this.hiveViewsTimestampPrecision = requireNonNull(hiveViewsTimestampPrecision, "hiveViewsTimestampPrecision is null"); } @Override @@ -266,6 +270,7 @@ public TransactionalMetadata create(ConnectorIdentity identity, boolean autoComm directoryLister, partitionProjectionService, allowTableRename, - maxPartitionDropsPerQuery); + maxPartitionDropsPerQuery, + hiveViewsTimestampPrecision); } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java index 9b0bbd44660d..643397f41494 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java @@ -79,7 +79,8 @@ public static ViewReader createViewReader( TypeManager typeManager, BiFunction> tableRedirectionResolver, MetadataProvider metadataProvider, - boolean runHiveViewRunAsInvoker) + boolean runHiveViewRunAsInvoker, + HiveTimestampPrecision hiveViewsTimestampPrecision) { if (isPrestoView(table)) { return new PrestoViewReader(); @@ -91,7 +92,8 @@ public static ViewReader createViewReader( return new HiveViewReader( new CoralSemiTransactionalHiveMSCAdapter(metastore, coralTableRedirectionResolver(session, tableRedirectionResolver, metadataProvider)), typeManager, - runHiveViewRunAsInvoker); + runHiveViewRunAsInvoker, + hiveViewsTimestampPrecision); } private static CoralTableRedirectionResolver coralTableRedirectionResolver( @@ -198,12 +200,14 @@ public static class HiveViewReader private final HiveMetastoreClient metastoreClient; private final TypeManager typeManager; private final boolean hiveViewsRunAsInvoker; + private final HiveTimestampPrecision hiveViewsTimestampPrecision; - public HiveViewReader(HiveMetastoreClient hiveMetastoreClient, TypeManager typeManager, boolean hiveViewsRunAsInvoker) + public HiveViewReader(HiveMetastoreClient hiveMetastoreClient, TypeManager typeManager, boolean hiveViewsRunAsInvoker, HiveTimestampPrecision hiveViewsTimestampPrecision) { this.metastoreClient = requireNonNull(hiveMetastoreClient, "hiveMetastoreClient is null"); this.typeManager = requireNonNull(typeManager, "typeManager is null"); this.hiveViewsRunAsInvoker = hiveViewsRunAsInvoker; + this.hiveViewsTimestampPrecision = requireNonNull(hiveViewsTimestampPrecision, "hiveViewsTimestampPrecision is null"); } @Override @@ -218,7 +222,7 @@ public ConnectorViewDefinition decodeViewData(String viewSql, Table table, Catal List columns = rowType.getFieldList().stream() .map(field -> new ViewColumn( field.getName(), - typeManager.fromSqlType(getTypeString(field.getType())).getTypeId(), + typeManager.fromSqlType(getTypeString(field.getType(), hiveViewsTimestampPrecision)).getTypeId(), Optional.empty())) .collect(toImmutableList()); return new ConnectorViewDefinition( @@ -241,7 +245,7 @@ public ConnectorViewDefinition decodeViewData(String viewSql, Table table, Catal // Calcite does not provide correct type strings for non-primitive types. // We add custom code here to make it work. Goal is for calcite/coral to handle this - private static String getTypeString(RelDataType type) + private static String getTypeString(RelDataType type, HiveTimestampPrecision timestampPrecision) { switch (type.getSqlTypeName()) { case ROW: { @@ -250,7 +254,7 @@ private static String getTypeString(RelDataType type) // We add the Coral function here to parse data types successfully. // Goal is to use data type mapping instead of translating to strings return type.getFieldList().stream() - .map(field -> quoteWordIfNotQuoted(field.getName().toLowerCase(Locale.ENGLISH)) + " " + getTypeString(field.getType())) + .map(field -> quoteWordIfNotQuoted(field.getName().toLowerCase(Locale.ENGLISH)) + " " + getTypeString(field.getType(), timestampPrecision)) .collect(joining(",", "row(", ")")); } case CHAR: @@ -263,14 +267,16 @@ private static String getTypeString(RelDataType type) case MAP: { RelDataType keyType = type.getKeyType(); RelDataType valueType = type.getValueType(); - return "map(" + getTypeString(keyType) + "," + getTypeString(valueType) + ")"; + return "map(" + getTypeString(keyType, timestampPrecision) + "," + getTypeString(valueType, timestampPrecision) + ")"; } case ARRAY: { - return "array(" + getTypeString(type.getComponentType()) + ")"; + return "array(" + getTypeString(type.getComponentType(), timestampPrecision) + ")"; } case DECIMAL: { return "decimal(" + type.getPrecision() + "," + type.getScale() + ")"; } + case TIMESTAMP: + return "timestamp(" + timestampPrecision.getPrecision() + ")"; default: return type.getSqlTypeName().toString(); } diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java index 7a793c320e2c..26dc0d01abd5 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java @@ -880,7 +880,8 @@ public Optional getMaterializedView(Connect countingDirectoryLister, 1000, new PartitionProjectionService(hiveConfig, ImmutableMap.of(), new TestingTypeManager()), - true); + true, + HiveTimestampPrecision.DEFAULT_PRECISION); transactionManager = new HiveTransactionManager(metadataFactory); splitManager = new HiveSplitManager( transactionManager, diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java index 7d0dee8a3387..06ba3f8001af 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java @@ -8241,47 +8241,29 @@ public void testSelectFromPrestoViewReferencingHiveTableWithTimestamps() assertThat(query(defaultSession, "SELECT ts FROM " + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(defaultSession, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123456789'") assertThat(query(defaultSession, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'"); assertThat(query(millisSession, "SELECT ts FROM " + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'"); assertThat(query(millisSession, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(nanosSessions, "SELECT ts FROM " + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123456789'") assertThat(query(nanosSessions, "SELECT ts FROM " + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(nanosSessions, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123456789'") assertThat(query(nanosSessions, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameDefault)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'"); // Presto view created with config property set to MILLIS and session property set to NANOS String prestoViewNameNanos = "presto_view_ts_nanos_" + randomNameSuffix(); assertUpdate(nanosSessions, "CREATE VIEW " + prestoViewNameNanos + " AS SELECT * FROM " + tableName); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(defaultSession, "SELECT ts FROM " + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'") assertThat(query(defaultSession, "SELECT ts FROM " + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123000000'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(defaultSession, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123456789'") assertThat(query(defaultSession, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123000000'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(millisSession, "SELECT ts FROM " + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'") assertThat(query(millisSession, "SELECT ts FROM " + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123000000'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(millisSession, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123'") assertThat(query(millisSession, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123000000'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(nanosSessions, "SELECT ts FROM " + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123456789'") assertThat(query(nanosSessions, "SELECT ts FROM " + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123000000'"); - // TODO(https://github.com/trinodb/trino/issues/6295) Presto view schema is fixed on creation - // should be: assertThat(query(nanosSessions, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123456789'") assertThat(query(nanosSessions, "SELECT ts FROM hive_timestamp_nanos.tpch." + prestoViewNameNanos)).matches("VALUES TIMESTAMP '1990-01-02 12:13:14.123000000'"); } diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/AbstractTestHiveViews.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/AbstractTestHiveViews.java index 29dc87b976e0..ae0d15c52af2 100644 --- a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/AbstractTestHiveViews.java +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/AbstractTestHiveViews.java @@ -482,30 +482,22 @@ public void testTimestampHiveView() unsetSessionProperty("hive_timestamp_nanos.timestamp_precision"); assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM timestamp_hive_view")).containsOnly(row("1990-01-02 12:13:14.123")); - assertThatThrownBy( - // TODO(https://github.com/trinodb/trino/issues/6295) it is not possible to query Hive view with timestamps if hive.timestamp-precision=NANOSECONDS - () -> assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM hive_timestamp_nanos.default.timestamp_hive_view")).containsOnly(row("1990-01-02 12:13:14.123456789")) - ).hasMessageContaining("timestamp(9) projected from query view at position 0 cannot be coerced to column [ts] of type timestamp(3) stored in view definition"); + assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM hive_timestamp_nanos.default.timestamp_hive_view")) + .containsOnly(row("1990-01-02 12:13:14.123456789")); setSessionProperty("hive.timestamp_precision", "'MILLISECONDS'"); setSessionProperty("hive_timestamp_nanos.timestamp_precision", "'MILLISECONDS'"); assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM timestamp_hive_view")).containsOnly(row("1990-01-02 12:13:14.123")); - assertThatThrownBy( - // TODO(https://github.com/trinodb/trino/issues/6295) it is not possible to query Hive view with timestamps if hive.timestamp-precision=NANOSECONDS - () -> assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM hive_timestamp_nanos.default.timestamp_hive_view")).containsOnly(row("1990-01-02 12:13:14.123")) - ).hasMessageContaining("timestamp(9) projected from query view at position 0 cannot be coerced to column [ts] of type timestamp(3) stored in view definition"); + assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM hive_timestamp_nanos.default.timestamp_hive_view")) + .containsOnly(row("1990-01-02 12:13:14.123456789")); setSessionProperty("hive.timestamp_precision", "'NANOSECONDS'"); setSessionProperty("hive_timestamp_nanos.timestamp_precision", "'NANOSECONDS'"); - // TODO(https://github.com/trinodb/trino/issues/6295) timestamp_precision has no effect on Hive views - // should be: assertThat(query("SELECT CAST(ts AS varchar) FROM timestamp_hive_view")).containsOnly(row("1990-01-02 12:13:14.123456789")) assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM timestamp_hive_view")).containsOnly(row("1990-01-02 12:13:14.123")); - assertThatThrownBy( - // TODO(https://github.com/trinodb/trino/issues/6295) it is not possible to query Hive view with timestamps if hive.timestamp-precision=NANOSECONDS - () -> assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM hive_timestamp_nanos.default.timestamp_hive_view")).containsOnly(row("1990-01-02 12:13:14.123456789")) - ).hasMessageContaining("timestamp(9) projected from query view at position 0 cannot be coerced to column [ts] of type timestamp(3) stored in view definition"); + assertThat(onTrino().executeQuery("SELECT CAST(ts AS varchar) FROM hive_timestamp_nanos.default.timestamp_hive_view")) + .containsOnly(row("1990-01-02 12:13:14.123456789")); } @Test(groups = HIVE_VIEWS)