Skip to content

Commit

Permalink
Add metastore column properties
Browse files Browse the repository at this point in the history
  • Loading branch information
findepi committed Aug 1, 2023
1 parent 46c10fa commit 644fe76
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 15 deletions.
6 changes: 6 additions & 0 deletions .mvn/modernizer/violations.xml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@
<comment>Table type is nullable in Glue model, which is too easy to forget about. Prefer GlueToTrinoConverter.getTableType</comment>
</violation>

<violation>
<name>com/amazonaws/services/glue/model/Column.getParameters:()Ljava/util/Map;</name>
<version>1.1</version>
<comment>Column parameters map is nullable in Glue model, which is too easy to forget about. Prefer GlueToTrinoConverter.getColumnParameters</comment>
</violation>

<violation>
<name>com/amazonaws/services/glue/model/Table.getParameters:()Ljava/util/Map;</name>
<version>1.1</version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.annotations.Immutable;
import io.trino.plugin.hive.HiveType;

import java.util.Map;
import java.util.Objects;
import java.util.Optional;

Expand All @@ -30,16 +32,28 @@ public class Column
private final String name;
private final HiveType type;
private final Optional<String> comment;
private final Map<String, String> properties;

@Deprecated
public Column(
String name,
HiveType type,
Optional<String> comment)
{
this(name, type, comment, ImmutableMap.of());
}

@JsonCreator
public Column(
@JsonProperty("name") String name,
@JsonProperty("type") HiveType type,
@JsonProperty("comment") Optional<String> comment)
@JsonProperty("comment") Optional<String> comment,
@JsonProperty("properties") Map<String, String> properties)
{
this.name = requireNonNull(name, "name is null");
this.type = requireNonNull(type, "type is null");
this.comment = requireNonNull(comment, "comment is null");
this.properties = ImmutableMap.copyOf(requireNonNull(properties, "properties is null"));
}

@JsonProperty
Expand All @@ -60,6 +74,12 @@ public Optional<String> getComment()
return comment;
}

@JsonProperty
public Map<String, String> getProperties()
{
return properties;
}

@Override
public String toString()
{
Expand All @@ -82,12 +102,13 @@ public boolean equals(Object o)
Column column = (Column) o;
return Objects.equals(name, column.name) &&
Objects.equals(type, column.type) &&
Objects.equals(comment, column.comment);
Objects.equals(comment, column.comment) &&
Objects.equals(properties, column.properties);
}

@Override
public int hashCode()
{
return Objects.hash(name, type, comment);
return Objects.hash(name, type, comment, properties);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.annotations.Immutable;
import io.trino.plugin.hive.HiveType;

import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

Expand All @@ -32,16 +34,32 @@ public class Column
private final String name;
private final HiveType type;
private final Optional<String> comment;
private final Map<String, String> properties;

@JsonCreator
public Column(
@JsonProperty("name") String name,
@JsonProperty("type") HiveType type,
@JsonProperty("comment") Optional<String> comment)
@JsonProperty("comment") Optional<String> comment,
@JsonProperty("properties") Optional<Map<String, String>> properties)
{
this(
name,
type,
comment,
properties.orElse(ImmutableMap.of()));
}

public Column(
String name,
HiveType type,
Optional<String> comment,
Map<String, String> properties)
{
this.name = requireNonNull(name, "name is null");
this.type = requireNonNull(type, "type is null");
this.comment = requireNonNull(comment, "comment is null");
this.properties = ImmutableMap.copyOf(requireNonNull(properties, "properties is null"));
}

@JsonProperty
Expand All @@ -62,6 +80,12 @@ public Optional<String> getComment()
return comment;
}

@JsonProperty
public Map<String, String> getProperties()
{
return properties;
}

@Override
public String toString()
{
Expand All @@ -84,13 +108,14 @@ public boolean equals(Object o)
Column column = (Column) o;
return Objects.equals(name, column.name) &&
Objects.equals(type, column.type) &&
Objects.equals(comment, column.comment);
Objects.equals(comment, column.comment) &&
Objects.equals(properties, column.properties);
}

@Override
public int hashCode()
{
return Objects.hash(name, type, comment);
return Objects.hash(name, type, comment, properties);
}

public static List<Column> fromMetastoreModel(List<io.trino.plugin.hive.metastore.Column> metastoreColumns)
Expand All @@ -105,7 +130,8 @@ public static Column fromMetastoreModel(io.trino.plugin.hive.metastore.Column me
return new Column(
metastoreColumn.getName(),
metastoreColumn.getType(),
metastoreColumn.getComment());
metastoreColumn.getComment(),
metastoreColumn.getProperties());
}

public static List<io.trino.plugin.hive.metastore.Column> toMetastoreModel(List<Column> fileMetastoreColumns)
Expand All @@ -120,6 +146,7 @@ public static io.trino.plugin.hive.metastore.Column toMetastoreModel(Column file
return new io.trino.plugin.hive.metastore.Column(
fileMetastoreColumn.getName(),
fileMetastoreColumn.getType(),
fileMetastoreColumn.getComment());
fileMetastoreColumn.getComment(),
fileMetastoreColumn.getProperties());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ public synchronized void commentColumn(String databaseName, String tableName, St
ImmutableList.Builder<Column> newDataColumns = ImmutableList.builder();
for (Column fieldSchema : oldTable.getDataColumns()) {
if (fieldSchema.getName().equals(columnName)) {
newDataColumns.add(new Column(columnName, fieldSchema.getType(), comment));
newDataColumns.add(new Column(columnName, fieldSchema.getType(), comment, fieldSchema.getProperties()));
}
else {
newDataColumns.add(fieldSchema);
Expand All @@ -702,7 +702,7 @@ public synchronized void addColumn(String databaseName, String tableName, String
currentVersion,
ImmutableList.<Column>builder()
.addAll(oldTable.getDataColumns())
.add(new Column(columnName, columnType, Optional.ofNullable(columnComment)))
.add(new Column(columnName, columnType, Optional.ofNullable(columnComment), ImmutableMap.of()))
.build());
});
}
Expand All @@ -727,7 +727,7 @@ public synchronized void renameColumn(String databaseName, String tableName, Str
ImmutableList.Builder<Column> newDataColumns = ImmutableList.builder();
for (Column fieldSchema : oldTable.getDataColumns()) {
if (fieldSchema.getName().equals(oldColumnName)) {
newDataColumns.add(new Column(newColumnName, fieldSchema.getType(), fieldSchema.getComment()));
newDataColumns.add(new Column(newColumnName, fieldSchema.getType(), fieldSchema.getComment(), fieldSchema.getProperties()));
}
else {
newDataColumns.add(fieldSchema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ private static com.amazonaws.services.glue.model.Column convertColumn(Column tri
return new com.amazonaws.services.glue.model.Column()
.withName(trinoColumn.getName())
.withType(trinoColumn.getType().toString())
.withComment(trinoColumn.getComment().orElse(null));
.withComment(trinoColumn.getComment().orElse(null))
.withParameters(trinoColumn.getProperties());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ public final class GlueToTrinoConverter

private GlueToTrinoConverter() {}

@SuppressModernizer // Usage of `Column.getParameters` is not allowed. Only this method can call that.
public static Map<String, String> getColumnParameters(com.amazonaws.services.glue.model.Column glueColumn)
{
return firstNonNull(glueColumn.getParameters(), ImmutableMap.of());
}

public static String getTableType(com.amazonaws.services.glue.model.Table glueTable)
{
// Athena treats missing table type as EXTERNAL_TABLE.
Expand Down Expand Up @@ -132,7 +138,7 @@ public static Table convertTable(com.amazonaws.services.glue.model.Table glueTab
// Iceberg tables do not need to read the StorageDescriptor field, but we still need to return dummy properties for compatibility
// Delta Lake tables only need to provide a dummy properties if a StorageDescriptor was not explicitly configured.
// Materialized views do not need to read the StorageDescriptor, but we still need to return dummy properties for compatibility
tableBuilder.setDataColumns(ImmutableList.of(new Column("dummy", HIVE_INT, Optional.empty())));
tableBuilder.setDataColumns(ImmutableList.of(new Column("dummy", HIVE_INT, Optional.empty(), ImmutableMap.of())));
tableBuilder.getStorageBuilder().setStorageFormat(StorageFormat.fromHiveStorageFormat(HiveStorageFormat.PARQUET));
}
else {
Expand All @@ -159,9 +165,9 @@ private static Column convertColumn(SchemaTableName table, com.amazonaws.service
// to string to avoid cast exceptions.
if (HiveStorageFormat.CSV.getSerde().equals(serde)) {
//TODO(https://github.com/trinodb/trino/issues/7240) Add tests
return new Column(glueColumn.getName(), HiveType.HIVE_STRING, Optional.ofNullable(glueColumn.getComment()));
return new Column(glueColumn.getName(), HiveType.HIVE_STRING, Optional.ofNullable(glueColumn.getComment()), getColumnParameters(glueColumn));
}
return new Column(glueColumn.getName(), convertType(table, glueColumn), Optional.ofNullable(glueColumn.getComment()));
return new Column(glueColumn.getName(), convertType(table, glueColumn), Optional.ofNullable(glueColumn.getComment()), getColumnParameters(glueColumn));
}

private static HiveType convertType(SchemaTableName table, com.amazonaws.services.glue.model.Column column)
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

0 comments on commit 644fe76

Please sign in to comment.