Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metastore-level test for column properties #18567

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,22 @@ public void testStorePartitionWithStatistics()
// used to ingest data into partitioned hive tables.
testStorePartitionWithStatistics(STATISTICS_PARTITIONED_TABLE_COLUMNS, STATISTICS_1, STATISTICS_2, STATISTICS_1_1, PartitionStatistics.empty());
}

@Override
public void testDataColumnProperties()
{
// Column properties are currently not supported in ThriftHiveMetastore
assertThatThrownBy(super::testDataColumnProperties)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Persisting column properties is not supported: Column{name=id, type=bigint}");
}

@Override
public void testPartitionColumnProperties()
{
// Column properties are currently not supported in ThriftHiveMetastore
assertThatThrownBy(super::testPartitionColumnProperties)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Persisting column properties is not supported: Column{name=part_key, type=varchar(256)}");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ public static PrincipalType fromMetastoreApiPrincipalType(io.trino.hive.thrift.m

public static FieldSchema toMetastoreApiFieldSchema(Column column)
{
checkArgument(column.getProperties().isEmpty(), "Persisting column properties is not supported: %s", column);
return new FieldSchema(column.getName(), column.getType().getHiveTypeName().toString(), column.getComment().orElse(null));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.metastore.TableType;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.joda.time.DateTime;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
Expand Down Expand Up @@ -3412,6 +3413,92 @@ public void testUpdatePartitionColumnStatisticsEmptyOptionalFields()
}
}

@Test
public void testDataColumnProperties()
throws Exception
{
SchemaTableName tableName = temporaryTable("test_column_properties");
HiveMetastoreClosure metastoreClient = new HiveMetastoreClosure(getMetastoreClient());
try {
doCreateEmptyTable(tableName, ORC, List.of(new ColumnMetadata("id", BIGINT), new ColumnMetadata("part_key", createVarcharType(256))));

Table table = metastoreClient.getTable(tableName.getSchemaName(), tableName.getTableName()).orElseThrow();
assertThat(table.getDataColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEmpty();
assertThat(table.getPartitionColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEmpty();

String columnPropertyValue = "data column value ,;.!??? \" ' {} [] non-printable \000 \001 spaces \n\r\t\f hiragana だ emoji 🤷‍♂️ x";
metastoreClient.replaceTable(
tableName.getSchemaName(),
tableName.getTableName(),
Table.builder(table)
.setDataColumns(List.of(new Column("id", HIVE_LONG, Optional.empty(), Map.of("data prop", columnPropertyValue))))
.build(),
NO_PRIVILEGES);

table = metastoreClient.getTable(tableName.getSchemaName(), tableName.getTableName()).orElseThrow();
assertThat(table.getDataColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEqualTo(Map.of("data prop", columnPropertyValue));
assertThat(table.getPartitionColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEmpty();
}
finally {
dropTable(tableName);
}
}

@Test
public void testPartitionColumnProperties()
throws Exception
{
SchemaTableName tableName = temporaryTable("test_column_properties");
HiveMetastoreClosure metastoreClient = new HiveMetastoreClosure(getMetastoreClient());
try {
doCreateEmptyTable(tableName, ORC, List.of(new ColumnMetadata("id", BIGINT), new ColumnMetadata("part_key", createVarcharType(256))));

Table table = metastoreClient.getTable(tableName.getSchemaName(), tableName.getTableName()).orElseThrow();
assertThat(table.getDataColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEmpty();
assertThat(table.getPartitionColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEmpty();

String columnPropertyValue = "partition column value ,;.!??? \" ' {} [] non-printable \000 \001 spaces \n\r\t\f hiragana だ emoji 🤷‍♂️ x";
metastoreClient.replaceTable(
tableName.getSchemaName(),
tableName.getTableName(),
Table.builder(table)
.setPartitionColumns(List.of(new Column("part_key", HiveType.valueOf("varchar(256)"), Optional.empty(), Map.of("partition prop", columnPropertyValue))))
.build(),
NO_PRIVILEGES);

table = metastoreClient.getTable(tableName.getSchemaName(), tableName.getTableName()).orElseThrow();
assertThat(table.getDataColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEmpty();
assertThat(table.getPartitionColumns())
.singleElement()
.extracting(Column::getProperties, InstanceOfAssertFactories.MAP)
.isEqualTo(Map.of("partition prop", columnPropertyValue));
}
finally {
dropTable(tableName);
}
}

@Test
public void testInputInfoWhenTableIsPartitioned()
throws Exception
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

import java.io.File;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

// staging directory is shared mutable state
@Test(singleThreaded = true)
public class TestHiveInMemoryMetastore
Expand Down Expand Up @@ -67,4 +69,22 @@ public void testDisallowQueryingOfIcebergTables()
{
throw new SkipException("not supported");
}

@Override
public void testDataColumnProperties()
{
// Column properties are currently not supported in ThriftHiveMetastore
assertThatThrownBy(super::testDataColumnProperties)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Persisting column properties is not supported: Column{name=id, type=bigint}");
}

@Override
public void testPartitionColumnProperties()
{
// Column properties are currently not supported in ThriftHiveMetastore
assertThatThrownBy(super::testPartitionColumnProperties)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Persisting column properties is not supported: Column{name=part_key, type=varchar(256)}");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,15 @@ public void testInvalidColumnStatisticsMetadata()
}
}

@Override
public void testPartitionColumnProperties()
{
// Glue currently does not support parameters on the partitioning columns
assertThatThrownBy(super::testPartitionColumnProperties)
.isInstanceOf(TrinoException.class)
.hasMessageStartingWith("Parameters not supported for partition columns (Service: AWSGlue; Status Code: 400; Error Code: InvalidInputException;");
}

@Test
public void testGlueObjectsWithoutStorageDescriptor()
{
Expand Down