diff --git a/docs/src/main/sphinx/connector/hive.rst b/docs/src/main/sphinx/connector/hive.rst index ca6acf31b9bb..29f1a23cde1e 100644 --- a/docs/src/main/sphinx/connector/hive.rst +++ b/docs/src/main/sphinx/connector/hive.rst @@ -1141,6 +1141,11 @@ to the connector using a :doc:`WITH ` clause:: ``s3a://test/name=${name}/``. Mapped from the AWS Athena table property `storage.location.template `_ - ``${table_location}/${partition_name}`` + * - ``extra_properties`` + - Additional properties added to a Hive table. The properties are not used by Trino, + and are available in the ``$properties`` metadata table. + The properties are not included in the output of ``SHOW CREATE TABLE`` statements. + - .. _hive_special_tables: 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 50e9cdd94725..856bf1143ad3 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 @@ -236,6 +236,7 @@ import static io.trino.plugin.hive.HiveTableProperties.getAvroSchemaUrl; import static io.trino.plugin.hive.HiveTableProperties.getBucketProperty; import static io.trino.plugin.hive.HiveTableProperties.getExternalLocation; +import static io.trino.plugin.hive.HiveTableProperties.getExtraProperties; import static io.trino.plugin.hive.HiveTableProperties.getFooterSkipCount; import static io.trino.plugin.hive.HiveTableProperties.getHeaderSkipCount; import static io.trino.plugin.hive.HiveTableProperties.getHiveStorageFormat; @@ -277,6 +278,7 @@ import static io.trino.plugin.hive.metastore.SemiTransactionalHiveMetastore.cleanExtraOutputFiles; import static io.trino.plugin.hive.metastore.StorageFormat.VIEW_STORAGE_FORMAT; import static io.trino.plugin.hive.metastore.StorageFormat.fromHiveStorageFormat; +import static io.trino.plugin.hive.metastore.thrift.ThriftMetastoreUtil.STATS_PROPERTIES; import static io.trino.plugin.hive.type.Category.PRIMITIVE; import static io.trino.plugin.hive.util.AcidTables.deltaSubdir; import static io.trino.plugin.hive.util.AcidTables.isFullAcidTable; @@ -1168,7 +1170,27 @@ else if (avroSchemaLiteral != null) { // Partition Projection specific properties tableProperties.putAll(partitionProjectionService.getPartitionProjectionHiveTableProperties(tableMetadata)); - return tableProperties.buildOrThrow(); + Map baseProperties = tableProperties.buildOrThrow(); + + // Extra properties + Map extraProperties = getExtraProperties(tableMetadata.getProperties()) + .orElseGet(ImmutableMap::of); + Set illegalExtraProperties = Sets.intersection( + ImmutableSet.builder() + .addAll(baseProperties.keySet()) + .addAll(STATS_PROPERTIES) + .build(), + extraProperties.keySet()); + if (!illegalExtraProperties.isEmpty()) { + throw new TrinoException( + INVALID_TABLE_PROPERTY, + "Illegal keys in extra_properties: " + illegalExtraProperties); + } + + return ImmutableMap.builder() + .putAll(baseProperties) + .putAll(extraProperties) + .buildOrThrow(); } private static void checkFormatForProperty(HiveStorageFormat actualStorageFormat, HiveStorageFormat expectedStorageFormat, String propertyName) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveTableProperties.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveTableProperties.java index 328f494cbbfc..b5f8fc6a95fa 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveTableProperties.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveTableProperties.java @@ -21,6 +21,8 @@ import io.trino.spi.TrinoException; import io.trino.spi.session.PropertyMetadata; import io.trino.spi.type.ArrayType; +import io.trino.spi.type.MapType; +import io.trino.spi.type.TypeManager; import javax.inject.Inject; @@ -69,13 +71,15 @@ public class HiveTableProperties public static final String REGEX_CASE_INSENSITIVE = "regex_case_insensitive"; public static final String TRANSACTIONAL = "transactional"; public static final String AUTO_PURGE = "auto_purge"; + public static final String EXTRA_PROPERTIES = "extra_properties"; private final List> tableProperties; @Inject public HiveTableProperties( HiveConfig config, - OrcWriterConfig orcWriterConfig) + OrcWriterConfig orcWriterConfig, + TypeManager typeManager) { tableProperties = ImmutableList.of( stringProperty( @@ -173,7 +177,25 @@ public HiveTableProperties( PARTITION_PROJECTION_LOCATION_TEMPLATE, "Partition projection location template", null, - false)); + false), + new PropertyMetadata<>( + EXTRA_PROPERTIES, + "Extra table properties", + new MapType(VARCHAR, VARCHAR, typeManager.getTypeOperators()), + Map.class, + null, + true, // currently not shown in SHOW CREATE TABLE + value -> { + Map extraProperties = (Map) value; + if (extraProperties.containsValue(null)) { + throw new TrinoException(INVALID_TABLE_PROPERTY, format("Extra table property value cannot be null '%s'", extraProperties)); + } + if (extraProperties.containsKey(null)) { + throw new TrinoException(INVALID_TABLE_PROPERTY, format("Extra table property key cannot be null '%s'", extraProperties)); + } + return extraProperties; + }, + value -> value)); } public List> getTableProperties() @@ -311,4 +333,9 @@ public static Optional isAutoPurge(Map tableProperties) { return Optional.ofNullable((Boolean) tableProperties.get(AUTO_PURGE)); } + + public static Optional> getExtraProperties(Map tableProperties) + { + return Optional.ofNullable((Map) tableProperties.get(EXTRA_PROPERTIES)); + } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreUtil.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreUtil.java index 07d2b270ba38..6b73c3ae1f0d 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreUtil.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreUtil.java @@ -151,7 +151,7 @@ public final class ThriftMetastoreUtil private static final String NUM_FILES = "numFiles"; private static final String RAW_DATA_SIZE = "rawDataSize"; private static final String TOTAL_SIZE = "totalSize"; - private static final Set STATS_PROPERTIES = ImmutableSet.of(NUM_FILES, NUM_ROWS, RAW_DATA_SIZE, TOTAL_SIZE); + public static final Set STATS_PROPERTIES = ImmutableSet.of(NUM_FILES, NUM_ROWS, RAW_DATA_SIZE, TOTAL_SIZE); private ThriftMetastoreUtil() {} 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 7b546d2d3f32..7307ed4b1046 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 @@ -54,6 +54,7 @@ import io.trino.testing.MaterializedResult; import io.trino.testing.MaterializedResultWithQueryId; import io.trino.testing.MaterializedRow; +import io.trino.testing.QueryFailedException; import io.trino.testing.QueryRunner; import io.trino.testing.TestingConnectorBehavior; import io.trino.testing.sql.TestTable; @@ -8487,6 +8488,110 @@ public void testCreateAcidTableUnsupported() assertQueryFails("CREATE TABLE acid_unsupported WITH (transactional = true) AS SELECT 123 x", "FileHiveMetastore does not support ACID tables"); } + @Test + public void testExtraProperties() + { + String tableName = "create_table_with_multiple_extra_properties_" + randomNameSuffix(); + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two']))".formatted(tableName)); + + assertQuery( + "SELECT \"extra.property.one\", \"extra.property.two\" FROM \"%s$properties\"".formatted(tableName), + "SELECT 'one', 'two'"); + assertThat(computeActual("SHOW CREATE TABLE %s".formatted(tableName)).getOnlyValue()) + .isEqualTo("CREATE TABLE hive.tpch.%s (\n".formatted(tableName) + + " c1 integer\n" + + ")\n" + + "WITH (\n" + + " format = 'ORC'\n" + + ")"); + assertUpdate("DROP TABLE %s".formatted(tableName)); + } + + @Test + public void testExtraPropertiesWithCtas() + { + String tableName = "create_table_ctas_with_multiple_extra_properties_" + randomNameSuffix(); + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two']))".formatted(tableName)); + + assertQuery( + "SELECT \"extra.property.one\", \"extra.property.two\" FROM \"%s$properties\"".formatted(tableName), + "SELECT 'one', 'two'"); + assertThat(computeActual("SHOW CREATE TABLE %s".formatted(tableName)).getOnlyValue()) + .isEqualTo("CREATE TABLE hive.tpch.%s (\n".formatted(tableName) + + " c1 integer\n" + + ")\n" + + "WITH (\n" + + " format = 'ORC'\n" + + ")"); + + assertUpdate("DROP TABLE %s".formatted(tableName)); + } + + @Test + public void testShowCreateWithExtraProperties() + { + String tableName = format("%s.%s.show_create_table_with_extra_properties_%s", getSession().getCatalog().get(), getSession().getSchema().get(), randomNameSuffix()); + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two']))".formatted(tableName)); + + assertThat(computeActual("SHOW CREATE TABLE " + tableName).getOnlyValue()) + .isEqualTo("CREATE TABLE %s (\n".formatted(tableName) + + " c1 integer\n" + + ")\n" + + "WITH (\n" + + " format = 'ORC'\n" + + ")"); + + assertUpdate("DROP TABLE %s".formatted(tableName)); + } + + @Test + public void testDuplicateExtraProperties() + { + assertQueryFails( + "CREATE TABLE create_table_with_duplicate_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property', 'extra.property'], ARRAY['true', 'false']))", + "Invalid value for catalog 'hive' table property 'extra_properties': Cannot convert.*"); + assertQueryFails( + "CREATE TABLE create_table_select_as_with_duplicate_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['extra.property', 'extra.property'], ARRAY['true', 'false']))", + "Invalid value for catalog 'hive' table property 'extra_properties': Cannot convert.*"); + } + + @Test + public void testOverwriteExistingPropertyWithExtraProperties() + { + assertThatThrownBy(() -> assertUpdate("CREATE TABLE create_table_with_overwrite_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['transactional'], ARRAY['true']))")) + .isInstanceOf(QueryFailedException.class) + .hasMessage("Illegal keys in extra_properties: [transactional]"); + + assertThatThrownBy(() -> assertUpdate("CREATE TABLE create_table_as_select_with_extra_properties WITH (extra_properties = MAP(ARRAY['rawDataSize'], ARRAY['1'])) AS SELECT 1 as c1")) + .isInstanceOf(QueryFailedException.class) + .hasMessage("Illegal keys in extra_properties: [rawDataSize]"); + } + + @Test + public void testNullExtraProperty() + { + assertQueryFails( + "CREATE TABLE create_table_with_duplicate_extra_properties (c1 integer) WITH (extra_properties = MAP(ARRAY['null.property'], ARRAY[null]))", + ".*Extra table property value cannot be null '\\{null.property=null}'.*"); + assertQueryFails( + "CREATE TABLE create_table_as_select_with_extra_properties WITH (extra_properties = MAP(ARRAY['null.property'], ARRAY[null])) AS SELECT 1 as c1", + ".*Extra table property value cannot be null '\\{null.property=null}'.*"); + } + + @Test + public void testCollidingMixedCaseProperty() + { + String tableName = "create_table_with_mixed_case_extra_properties" + randomNameSuffix(); + + assertUpdate("CREATE TABLE %s (c1 integer) WITH (extra_properties = MAP(ARRAY['one', 'ONE'], ARRAY['one', 'ONE']))".formatted(tableName)); + // TODO: (https://github.com/trinodb/trino/issues/17) This should run successfully + assertThatThrownBy(() -> query("SELECT * FROM \"%s$properties\"".formatted(tableName))) + .isInstanceOf(QueryFailedException.class) + .hasMessageContaining("Multiple entries with same key: one=one and one=one"); + + assertUpdate("DROP TABLE %s".formatted(tableName)); + } + private static final Set NAMED_COLUMN_ONLY_FORMATS = ImmutableSet.of(HiveStorageFormat.AVRO, HiveStorageFormat.JSON); @DataProvider