Skip to content

Commit

Permalink
Add extra_properties to hive table properties
Browse files Browse the repository at this point in the history
  • Loading branch information
@Praveen2112
Praveen2112 committed May 10, 2023
1 parent 0df1476 commit e0b7da9
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 4 deletions.
5 changes: 5 additions & 0 deletions docs/src/main/sphinx/connector/hive.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,11 @@ to the connector using a :doc:`WITH </sql/create-table-as>` clause::
``s3a://test/name=${name}/``. Mapped from the AWS Athena table property
`storage.location.template <https://docs.aws.amazon.com/athena/latest/ug/partition-projection-setting-up.html#partition-projection-specifying-custom-s3-storage-locations>`_
- ``${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:

Expand Down
24 changes: 23 additions & 1 deletion plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1168,7 +1170,27 @@ else if (avroSchemaLiteral != null) {
// Partition Projection specific properties
tableProperties.putAll(partitionProjectionService.getPartitionProjectionHiveTableProperties(tableMetadata));

return tableProperties.buildOrThrow();
Map<String, String> baseProperties = tableProperties.buildOrThrow();

// Extra properties
Map<String, String> extraProperties = getExtraProperties(tableMetadata.getProperties())
.orElseGet(ImmutableMap::of);
Set<String> illegalExtraProperties = Sets.intersection(
ImmutableSet.<String>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.<String, String>builder()
.putAll(baseProperties)
.putAll(extraProperties)
.buildOrThrow();
}

private static void checkFormatForProperty(HiveStorageFormat actualStorageFormat, HiveStorageFormat expectedStorageFormat, String propertyName)
Expand Down
31 changes: 29 additions & 2 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveTableProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<PropertyMetadata<?>> tableProperties;

@Inject
public HiveTableProperties(
HiveConfig config,
OrcWriterConfig orcWriterConfig)
OrcWriterConfig orcWriterConfig,
TypeManager typeManager)
{
tableProperties = ImmutableList.of(
stringProperty(
Expand Down Expand Up @@ -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<String, String> extraProperties = (Map<String, String>) 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<PropertyMetadata<?>> getTableProperties()
Expand Down Expand Up @@ -311,4 +333,9 @@ public static Optional<Boolean> isAutoPurge(Map<String, Object> tableProperties)
{
return Optional.ofNullable((Boolean) tableProperties.get(AUTO_PURGE));
}

public static Optional<Map<String, String>> getExtraProperties(Map<String, Object> tableProperties)
{
return Optional.ofNullable((Map<String, String>) tableProperties.get(EXTRA_PROPERTIES));
}
}
2 changes: 1 addition & 1 deletion ...n/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> STATS_PROPERTIES = ImmutableSet.of(NUM_FILES, NUM_ROWS, RAW_DATA_SIZE, TOTAL_SIZE);
public static final Set<String> STATS_PROPERTIES = ImmutableSet.of(NUM_FILES, NUM_ROWS, RAW_DATA_SIZE, TOTAL_SIZE);

private ThriftMetastoreUtil() {}

Expand Down
105 changes: 105 additions & 0 deletions plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<HiveStorageFormat> NAMED_COLUMN_ONLY_FORMATS = ImmutableSet.of(HiveStorageFormat.AVRO, HiveStorageFormat.JSON);

@DataProvider
Expand Down

0 comments on commit e0b7da9

Please sign in to comment.