From 2ff3b99c19f2bbba8218764f1c352d943ed2045b Mon Sep 17 00:00:00 2001 From: Padraig O'Sullivan Date: Fri, 1 Oct 2021 17:28:48 -0400 Subject: [PATCH] Add extra_properties to hive table properties --- .../trino/sql/rewrite/ShowQueriesRewrite.java | 10 ++ .../io/trino/plugin/hive/HiveMetadata.java | 17 ++++ .../plugin/hive/HiveTableProperties.java | 22 ++++- .../plugin/hive/BaseHiveConnectorTest.java | 94 +++++++++++++++++++ 4 files changed, 141 insertions(+), 2 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java b/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java index a33509b08deb..13024e41928c 100644 --- a/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java +++ b/core/trino-main/src/main/java/io/trino/sql/rewrite/ShowQueriesRewrite.java @@ -64,6 +64,7 @@ import io.trino.sql.tree.Explain; import io.trino.sql.tree.ExplainAnalyze; import io.trino.sql.tree.Expression; +import io.trino.sql.tree.FunctionCall; import io.trino.sql.tree.Identifier; import io.trino.sql.tree.LikePredicate; import io.trino.sql.tree.LongLiteral; @@ -75,6 +76,7 @@ import io.trino.sql.tree.QualifiedName; import io.trino.sql.tree.Query; import io.trino.sql.tree.Relation; +import io.trino.sql.tree.Row; import io.trino.sql.tree.ShowCatalogs; import io.trino.sql.tree.ShowColumns; import io.trino.sql.tree.ShowCreate; @@ -154,6 +156,7 @@ import static io.trino.sql.tree.ShowCreate.Type.TABLE; import static io.trino.sql.tree.ShowCreate.Type.VIEW; import static java.lang.String.format; +import static java.util.Arrays.asList; import static java.util.Locale.ENGLISH; import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toList; @@ -563,6 +566,13 @@ private static Expression toExpression(Object value) .collect(toList())); } + if (value instanceof Map) { + Map map = (Map) value; + return new FunctionCall(QualifiedName.of("map_from_entries"), ImmutableList.of(new Array(map.entrySet().stream() + .map(entry -> new Row(asList(toExpression(entry.getKey()), toExpression(entry.getValue())))) + .collect(toImmutableList())))); + } + throw new TrinoException(INVALID_TABLE_PROPERTY, format("Failed to convert object of type %s to expression: %s", value.getClass().getName(), value)); } 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 0d3c694e469f..e4677ab81e76 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 @@ -159,6 +159,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.Iterables.concat; import static com.google.common.collect.Iterables.getOnlyElement; +import static io.airlift.json.JsonCodec.mapJsonCodec; import static io.trino.hdfs.ConfigurationUtils.toJobConf; import static io.trino.plugin.hive.HiveAnalyzeProperties.getColumnNames; import static io.trino.plugin.hive.HiveAnalyzeProperties.getPartitionList; @@ -213,6 +214,7 @@ import static io.trino.plugin.hive.HiveTableProperties.CSV_QUOTE; import static io.trino.plugin.hive.HiveTableProperties.CSV_SEPARATOR; import static io.trino.plugin.hive.HiveTableProperties.EXTERNAL_LOCATION_PROPERTY; +import static io.trino.plugin.hive.HiveTableProperties.EXTRA_PROPERTIES; import static io.trino.plugin.hive.HiveTableProperties.NULL_FORMAT_PROPERTY; import static io.trino.plugin.hive.HiveTableProperties.ORC_BLOOM_FILTER_COLUMNS; import static io.trino.plugin.hive.HiveTableProperties.ORC_BLOOM_FILTER_FPP; @@ -227,6 +229,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; @@ -685,6 +688,11 @@ private ConnectorTableMetadata doGetTableMetadata(ConnectorSession session, Sche // Partition Projection specific properties properties.putAll(partitionProjectionService.getPartitionProjectionTrinoTableProperties(table)); + String extraProperties = table.getParameters().get(EXTRA_PROPERTIES); + if (extraProperties != null) { + properties.put(EXTRA_PROPERTIES, mapJsonCodec(String.class, String.class).fromJson(extraProperties)); + } + return new ConnectorTableMetadata(tableName, columns.build(), properties.buildOrThrow(), comment); } @@ -1090,6 +1098,15 @@ else if (avroSchemaLiteral != null) { tableProperties.put("numFiles", "-1"); tableProperties.put("totalSize", "-1"); + // Extra properties + Map extraProperties = getExtraProperties(tableMetadata.getProperties()); + if (extraProperties != null) { + tableProperties.put(EXTRA_PROPERTIES, mapJsonCodec(String.class, String.class).toJson(extraProperties)); + for (Map.Entry extraProperty : extraProperties.entrySet()) { + tableProperties.put(extraProperty.getKey(), extraProperty.getValue()); + } + } + // Table comment property tableMetadata.getComment().ifPresent(value -> tableProperties.put(TABLE_COMMENT, value)); 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 ed505a19e584..d51faf321757 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; @@ -67,13 +69,15 @@ public class HiveTableProperties public static final String CSV_ESCAPE = "csv_escape"; 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( @@ -169,7 +173,16 @@ 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, + false, + value -> ((Map) value), + value -> value)); } public List> getTableProperties() @@ -297,4 +310,9 @@ public static Optional isAutoPurge(Map tableProperties) { return Optional.ofNullable((Boolean) tableProperties.get(AUTO_PURGE)); } + + public static Map getExtraProperties(Map tableProperties) + { + return (Map) tableProperties.get(EXTRA_PROPERTIES); + } } 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 40f4ea8e4cdc..310e1faf1be0 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 @@ -29,6 +29,7 @@ import io.trino.metadata.QualifiedObjectName; import io.trino.metadata.TableHandle; import io.trino.metadata.TableMetadata; +import io.trino.spi.QueryId; import io.trino.spi.connector.CatalogSchemaTableName; import io.trino.spi.connector.ColumnHandle; import io.trino.spi.connector.ColumnMetadata; @@ -54,6 +55,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; @@ -8453,6 +8455,98 @@ 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 = format("%s.%s.test_extra_properties", getSession().getCatalog().get(), getSession().getSchema().get()); + @Language("SQL") String createTableSql = format(""" + CREATE TABLE %s ( + c1 integer) + WITH ( + extra_properties = MAP(ARRAY['extra.property'], ARRAY['true']), + format = 'ORC' + )""", + tableName); + MaterializedResultWithQueryId result = getDistributedQueryRunner().executeWithQueryId(getSession(), createTableSql); + QueryId queryId = result.getQueryId(); + String nodeVersion = (String) computeScalar("SELECT node_version FROM system.runtime.nodes WHERE coordinator"); + assertQuery( + "SELECT * FROM \"test_extra_properties$properties\"", + "SELECT 'workaround for potential lack of HIVE-12730', 'false', 'true', '{\n \"extra.property\" : \"true\"\n}', '0', '0', '" + queryId + "', '" + nodeVersion + "', '0', '0', 'false'"); + MaterializedResult actualResult = computeActual("SHOW CREATE TABLE " + tableName); + String expectedShowCreateTableSql = "CREATE TABLE hive.tpch.test_extra_properties (\n" + + " c1 integer\n" + + ")\n" + + "WITH (\n" + + " extra_properties = map_from_entries(ARRAY[ROW('extra.property', 'true')]),\n" + + " format = 'ORC'\n" + + ")"; + assertEquals(getOnlyElement(actualResult.getOnlyColumnAsSet()), expectedShowCreateTableSql); + assertUpdate("DROP TABLE " + tableName); + } + + @Test + public void testMultipleExtraProperties() + { + String tableName = format("%s.%s.test_multiple_extra_properties", getSession().getCatalog().get(), getSession().getSchema().get()); + @Language("SQL") String createTableSql = format(""" + CREATE TABLE %s ( + c1 integer) + WITH ( + extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two']), + format = 'ORC' + )""", + tableName); + MaterializedResultWithQueryId result = getDistributedQueryRunner().executeWithQueryId(getSession(), createTableSql); + QueryId queryId = result.getQueryId(); + String nodeVersion = (String) computeScalar("SELECT node_version FROM system.runtime.nodes WHERE coordinator"); + assertQuery( + "SELECT * FROM \"test_multiple_extra_properties$properties\"", + "SELECT 'workaround for potential lack of HIVE-12730', 'false', 'one', 'two', '{\n \"extra.property.one\" : \"one\",\n \"extra.property.two\" : \"two\"\n}', '0', '0', '" + queryId + "', '" + nodeVersion + "', '0', '0', 'false'"); + MaterializedResult actualResult = computeActual("SHOW CREATE TABLE " + tableName); + String expectedShowCreateTableSql = "CREATE TABLE hive.tpch.test_multiple_extra_properties (\n" + + " c1 integer\n" + + ")\n" + + "WITH (\n" + + " extra_properties = map_from_entries(ARRAY[ROW('extra.property.one', 'one'),ROW('extra.property.two', 'two')]),\n" + + " format = 'ORC'\n" + + ")"; + assertEquals(getOnlyElement(actualResult.getOnlyColumnAsSet()), expectedShowCreateTableSql); + assertUpdate("DROP TABLE " + tableName); + } + + @Test + public void testDuplicateExtraProperties() + { + String tableName = format("%s.%s.test_duplicate_extra_properties", getSession().getCatalog().get(), getSession().getSchema().get()); + @Language("SQL") String createTableSql = format(""" + CREATE TABLE %s ( + c1 integer) + WITH ( + extra_properties = MAP(ARRAY['extra.property', 'extra.property'], ARRAY['true', 'false']), + format = 'ORC' + )""", + tableName); + assertQueryFails(createTableSql, "Invalid value for catalog 'hive' table property 'extra_properties': Cannot convert.*"); + } + + @Test + public void testOverwriteExistingPropertyWithExtraProperties() + { + String tableName = format("%s.%s.test_overwrite_extra_properties", getSession().getCatalog().get(), getSession().getSchema().get()); + @Language("SQL") String createTableSql = format(""" + CREATE TABLE %s ( + c1 integer) + WITH ( + extra_properties = MAP(ARRAY['transactional'], ARRAY['true']), + format = 'ORC' + )""", + tableName); + assertThatThrownBy(() -> assertUpdate(createTableSql)) + .isInstanceOf(QueryFailedException.class) + .hasMessage("Multiple entries with same key: transactional=true and transactional=false"); + } + private static final Set NAMED_COLUMN_ONLY_FORMATS = ImmutableSet.of(HiveStorageFormat.AVRO, HiveStorageFormat.JSON); @DataProvider