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.

Sign up for GitHub

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

Generalize storage formats in file metastore #17368

Merged
merged 2 commits into from
Mar 8, 2022
Merged

Generalize storage formats in file metastore #17368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
235e556
Add some tests on serialization for file metastore
7c00 Mar 4, 2022
92d46bd
Generalize storage format in file metastore
7c00 Mar 7, 2022
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
5 changes: 5 additions & 0 deletions presto-hive-metastore/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@
<artifactId>slice</artifactId>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
Expand Down
53 changes: 43 additions & 10 deletions ...ve-metastore/src/main/java/com/facebook/presto/hive/metastore/file/PartitionMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,17 @@
import com.facebook.presto.hive.metastore.StorageFormat;
import com.facebook.presto.hive.metastore.Table;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

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

import static com.facebook.presto.hive.HiveStorageFormat.getHiveStorageFormat;
import static com.facebook.presto.hive.metastore.MetastoreUtil.updateStatisticsParameters;
import static com.facebook.presto.hive.metastore.PrestoTableType.EXTERNAL_TABLE;
import static com.facebook.presto.hive.metastore.StorageFormat.VIEW_STORAGE_FORMAT;
Expand All @@ -44,7 +46,7 @@ public class PartitionMetadata
private final List<Column> columns;
private final Map<String, String> parameters;

private final Optional<HiveStorageFormat> storageFormat;
private final StorageFormat storageFormat;
private final Optional<HiveBucketProperty> bucketProperty;
private final Map<String, String> storageParameters;
private final Map<String, String> serdeParameters;
Expand All @@ -59,7 +61,8 @@ public class PartitionMetadata
public PartitionMetadata(
@JsonProperty("columns") List<Column> columns,
@JsonProperty("parameters") Map<String, String> parameters,
@JsonProperty("storageFormat") Optional<HiveStorageFormat> storageFormat,
@JsonDeserialize(using = StorageFormatCompatDeserializer.class)
@JsonProperty("storageFormat") StorageFormat storageFormat,
@JsonProperty("bucketProperty") Optional<HiveBucketProperty> bucketProperty,
@JsonProperty("storageParameters") Map<String, String> storageParameters,
@JsonProperty("serdeParameters") Map<String, String> serdeParameters,
Expand All @@ -71,7 +74,7 @@ public PartitionMetadata(
this.columns = ImmutableList.copyOf(requireNonNull(columns, "columns is null"));
this.parameters = ImmutableMap.copyOf(requireNonNull(parameters, "parameters is null"));

this.storageFormat = requireNonNull(storageFormat, "storageFormat is null");
this.storageFormat = storageFormat == null ? VIEW_STORAGE_FORMAT : storageFormat;
this.bucketProperty = requireNonNull(bucketProperty, "bucketProperty is null");
this.storageParameters = ImmutableMap.copyOf(firstNonNull(storageParameters, ImmutableMap.of()));
this.serdeParameters = requireNonNull(serdeParameters, "serdeParameters is null");
Expand All @@ -82,6 +85,32 @@ public PartitionMetadata(
this.sealedPartition = sealedPartition;
}

@Deprecated
public PartitionMetadata(
List<Column> columns,
Map<String, String> parameters,
Optional<HiveStorageFormat> storageFormat,
Optional<HiveBucketProperty> bucketProperty,
Map<String, String> storageParameters,
Map<String, String> serdeParameters,
Optional<String> externalLocation,
Map<String, HiveColumnStatistics> columnStatistics,
boolean eligibleToIgnore,
boolean sealedPartition)
{
this(
columns,
parameters,
storageFormat.map(StorageFormat::fromHiveStorageFormat).orElse(VIEW_STORAGE_FORMAT),
bucketProperty,
storageParameters,
serdeParameters,
externalLocation,
columnStatistics,
eligibleToIgnore,
sealedPartition);
}

public PartitionMetadata(Table table, PartitionWithStatistics partitionWithStatistics)
{
Partition partition = partitionWithStatistics.getPartition();
Expand All @@ -90,10 +119,7 @@ public PartitionMetadata(Table table, PartitionWithStatistics partitionWithStati
this.columns = partition.getColumns();
this.parameters = updateStatisticsParameters(partition.getParameters(), statistics.getBasicStatistics());

StorageFormat tableFormat = partition.getStorage().getStorageFormat();
storageFormat = Arrays.stream(HiveStorageFormat.values())
.filter(format -> tableFormat.equals(StorageFormat.fromHiveStorageFormat(format)))
.findFirst();
storageFormat = partition.getStorage().getStorageFormat();

if (table.getTableType().equals(EXTERNAL_TABLE)) {
externalLocation = Optional.of(partition.getStorage().getLocation());
Expand Down Expand Up @@ -122,8 +148,15 @@ public Map<String, String> getParameters()
return parameters;
}

@JsonProperty
@Deprecated
@JsonIgnore
public Optional<HiveStorageFormat> getStorageFormat()
{
return getHiveStorageFormat(storageFormat);
}

@JsonProperty("storageFormat")
public StorageFormat getPartitionStorageFormat()
{
return storageFormat;
}
Expand Down Expand Up @@ -188,7 +221,7 @@ public Partition toPartition(String databaseName, String tableName, List<String>
values,
Storage.builder()
.setLocation(externalLocation.orElse(location))
.setStorageFormat(storageFormat.map(StorageFormat::fromHiveStorageFormat).orElse(VIEW_STORAGE_FORMAT))
.setStorageFormat(storageFormat)
.setBucketProperty(bucketProperty)
.setSerdeParameters(serdeParameters)
.setParameters(parameters)
Expand Down
44 changes: 44 additions & 0 deletions ...rc/main/java/com/facebook/presto/hive/metastore/file/StorageFormatCompatDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.facebook.presto.hive.metastore.file;

import com.facebook.presto.hive.HiveStorageFormat;
import com.facebook.presto.hive.metastore.StorageFormat;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;

import java.io.IOException;

import static com.facebook.presto.hive.metastore.StorageFormat.fromHiveStorageFormat;
import static com.fasterxml.jackson.core.JsonToken.VALUE_STRING;

public class StorageFormatCompatDeserializer
extends JsonDeserializer<StorageFormat>
{
@Override
public StorageFormat deserialize(JsonParser p, DeserializationContext ctxt)
throws IOException, JsonProcessingException
{
// Prior to version 0.271, HiveStorageFormat was used for storage format;
// this deserializer is to ensure backward compatibility
if (p.currentToken() == VALUE_STRING) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment here, prior to version 0.271 this was HiveStorageFormat and this class is to ensure back ward compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

HiveStorageFormat format = p.readValueAs(HiveStorageFormat.class);
return fromHiveStorageFormat(format);
}
return p.readValueAs(StorageFormat.class);
}
}
59 changes: 49 additions & 10 deletions ...o-hive-metastore/src/main/java/com/facebook/presto/hive/metastore/file/TableMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@
import com.facebook.presto.hive.metastore.StorageFormat;
import com.facebook.presto.hive.metastore.Table;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

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

import static com.facebook.presto.hive.HiveStorageFormat.getHiveStorageFormat;
import static com.facebook.presto.hive.metastore.PrestoTableType.EXTERNAL_TABLE;
import static com.facebook.presto.hive.metastore.StorageFormat.VIEW_STORAGE_FORMAT;
import static com.google.common.base.MoreObjects.firstNonNull;
Expand All @@ -45,7 +47,7 @@ public class TableMetadata
private final List<Column> partitionColumns;
private final Map<String, String> parameters;

private final Optional<HiveStorageFormat> storageFormat;
private final StorageFormat storageFormat;
private final Optional<HiveBucketProperty> bucketProperty;
private final Map<String, String> storageParameters;
private final Map<String, String> serdeParameters;
Expand All @@ -64,7 +66,8 @@ public TableMetadata(
@JsonProperty("dataColumns") List<Column> dataColumns,
@JsonProperty("partitionColumns") List<Column> partitionColumns,
@JsonProperty("parameters") Map<String, String> parameters,
@JsonProperty("storageFormat") Optional<HiveStorageFormat> storageFormat,
@JsonDeserialize(using = StorageFormatCompatDeserializer.class)
@JsonProperty("storageFormat") StorageFormat storageFormat,
@JsonProperty("bucketProperty") Optional<HiveBucketProperty> bucketProperty,
@JsonProperty("storageParameters") Map<String, String> storageParameters,
@JsonProperty("serdeParameters") Map<String, String> serdeParameters,
Expand All @@ -79,7 +82,7 @@ public TableMetadata(
this.partitionColumns = ImmutableList.copyOf(requireNonNull(partitionColumns, "partitionColumns is null"));
this.parameters = ImmutableMap.copyOf(requireNonNull(parameters, "parameters is null"));
this.storageParameters = ImmutableMap.copyOf(firstNonNull(storageParameters, ImmutableMap.of()));
this.storageFormat = requireNonNull(storageFormat, "storageFormat is null");
this.storageFormat = storageFormat == null ? VIEW_STORAGE_FORMAT : storageFormat;
this.bucketProperty = requireNonNull(bucketProperty, "bucketProperty is null");
this.serdeParameters = requireNonNull(serdeParameters, "serdeParameters is null");
this.externalLocation = requireNonNull(externalLocation, "externalLocation is null");
Expand All @@ -96,6 +99,38 @@ public TableMetadata(
checkArgument(partitionColumns.isEmpty() || columnStatistics.isEmpty(), "column statistics cannot be set for partitioned table");
}

@Deprecated
public TableMetadata(
String owner,
PrestoTableType tableType,
List<Column> dataColumns,
List<Column> partitionColumns,
Map<String, String> parameters,
Optional<HiveStorageFormat> storageFormat,
Optional<HiveBucketProperty> bucketProperty,
Map<String, String> storageParameters,
Map<String, String> serdeParameters,
Optional<String> externalLocation,
Optional<String> viewOriginalText,
Optional<String> viewExpandedText,
Map<String, HiveColumnStatistics> columnStatistics)
{
this(
owner,
tableType,
dataColumns,
partitionColumns,
parameters,
storageFormat.map(StorageFormat::fromHiveStorageFormat).orElse(VIEW_STORAGE_FORMAT),
bucketProperty,
storageParameters,
serdeParameters,
externalLocation,
viewOriginalText,
viewExpandedText,
columnStatistics);
}

public TableMetadata(Table table)
{
this(table, ImmutableMap.of());
Expand All @@ -109,10 +144,7 @@ public TableMetadata(Table table, Map<String, HiveColumnStatistics> columnStatis
partitionColumns = table.getPartitionColumns();
parameters = table.getParameters();

StorageFormat tableFormat = table.getStorage().getStorageFormat();
storageFormat = Arrays.stream(HiveStorageFormat.values())
.filter(format -> tableFormat.equals(StorageFormat.fromHiveStorageFormat(format)))
.findFirst();
storageFormat = table.getStorage().getStorageFormat();
bucketProperty = table.getStorage().getBucketProperty();
storageParameters = table.getStorage().getParameters();
serdeParameters = table.getStorage().getSerdeParameters();
Expand Down Expand Up @@ -174,8 +206,15 @@ public Map<String, String> getParameters()
return parameters;
}

@JsonProperty
@Deprecated
@JsonIgnore
public Optional<HiveStorageFormat> getStorageFormat()
{
return getHiveStorageFormat(storageFormat);
}

@JsonProperty("storageFormat")
public StorageFormat getTableStorageFormat()
{
return storageFormat;
}
Expand Down Expand Up @@ -285,7 +324,7 @@ public Table toTable(String databaseName, String tableName, String location)
tableType,
Storage.builder()
.setLocation(externalLocation.orElse(location))
.setStorageFormat(storageFormat.map(StorageFormat::fromHiveStorageFormat).orElse(VIEW_STORAGE_FORMAT))
.setStorageFormat(storageFormat)
.setBucketProperty(bucketProperty)
.setParameters(storageParameters)
.setSerdeParameters(serdeParameters)
Expand Down
Loading