Skip to content

Commit

Permalink
Allow alternative implementations of table writability checks
Browse files Browse the repository at this point in the history
  • Loading branch information
arhimondr committed Mar 21, 2024
1 parent 2216cc6 commit 331e3d8
Show file tree
Hide file tree
Showing 15 changed files with 262 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.facebook.presto.hive.HivePartitionStats;
import com.facebook.presto.hive.HiveSplitManager;
import com.facebook.presto.hive.HiveStagingFileCommitter;
import com.facebook.presto.hive.HiveTableWritabilityChecker;
import com.facebook.presto.hive.HiveTestUtils;
import com.facebook.presto.hive.HiveTransactionManager;
import com.facebook.presto.hive.HiveTypeTranslator;
Expand Down Expand Up @@ -175,7 +176,8 @@ public S3SelectTestHelper(String host,
new HivePartitionStats(),
new HiveFileRenamer(),
columnConverterProvider,
new QuickStatsProvider(hdfsEnvironment, DO_NOTHING_DIRECTORY_LISTER, new HiveClientConfig(), new NamenodeStats(), ImmutableList.of()));
new QuickStatsProvider(hdfsEnvironment, DO_NOTHING_DIRECTORY_LISTER, new HiveClientConfig(), new NamenodeStats(), ImmutableList.of()),
new HiveTableWritabilityChecker(config));
transactionManager = new HiveTransactionManager();
splitManager = new HiveSplitManager(
transactionManager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ public void configure(Binder binder)
binder.bind(StagingFileCommitter.class).to(HiveStagingFileCommitter.class).in(Scopes.SINGLETON);
binder.bind(ZeroRowFileCreator.class).to(HiveZeroRowFileCreator.class).in(Scopes.SINGLETON);
binder.bind(PartitionObjectBuilder.class).to(HivePartitionObjectBuilder.class).in(Scopes.SINGLETON);
binder.bind(TableWritabilityChecker.class).to(HiveTableWritabilityChecker.class).in(Scopes.SINGLETON);
binder.bind(HiveTransactionManager.class).in(Scopes.SINGLETON);
binder.bind(ConnectorSplitManager.class).to(HiveSplitManager.class).in(Scopes.SINGLETON);
binder.bind(PartitionSkippabilityChecker.class).to(HivePartitionSkippabilityChecker.class).in(Scopes.SINGLETON);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@
import static com.facebook.presto.hive.HiveUtil.translateHiveUnsupportedTypeForTemporaryTable;
import static com.facebook.presto.hive.HiveUtil.translateHiveUnsupportedTypesForTemporaryTable;
import static com.facebook.presto.hive.HiveUtil.verifyPartitionTypeSupported;
import static com.facebook.presto.hive.HiveWriteUtils.checkTableIsWritable;
import static com.facebook.presto.hive.HiveWriteUtils.isFileCreatedByQuery;
import static com.facebook.presto.hive.HiveWriteUtils.isWritableType;
import static com.facebook.presto.hive.HiveWriterFactory.computeBucketedFileName;
Expand Down Expand Up @@ -412,7 +411,6 @@ public class HiveMetadata
private final TableParameterCodec tableParameterCodec;
private final JsonCodec<PartitionUpdate> partitionUpdateCodec;
private final SmileCodec<PartitionUpdate> partitionUpdateSmileCodec;
private final boolean writesToNonManagedTablesEnabled;
private final boolean createsOfNonManagedTablesEnabled;
private final int maxPartitionBatchSize;
private final TypeTranslator typeTranslator;
Expand All @@ -424,14 +422,14 @@ public class HiveMetadata
private final HiveEncryptionInformationProvider encryptionInformationProvider;
private final HivePartitionStats hivePartitionStats;
private final HiveFileRenamer hiveFileRenamer;
private final TableWritabilityChecker tableWritabilityChecker;

public HiveMetadata(
SemiTransactionalHiveMetastore metastore,
HdfsEnvironment hdfsEnvironment,
HivePartitionManager partitionManager,
DateTimeZone timeZone,
boolean allowCorruptWritesForTesting,
boolean writesToNonManagedTablesEnabled,
boolean createsOfNonManagedTablesEnabled,
int maxPartitionBatchSize,
TypeManager typeManager,
Expand All @@ -450,7 +448,8 @@ public HiveMetadata(
PartitionObjectBuilder partitionObjectBuilder,
HiveEncryptionInformationProvider encryptionInformationProvider,
HivePartitionStats hivePartitionStats,
HiveFileRenamer hiveFileRenamer)
HiveFileRenamer hiveFileRenamer,
TableWritabilityChecker tableWritabilityChecker)
{
this.allowCorruptWritesForTesting = allowCorruptWritesForTesting;

Expand All @@ -466,7 +465,6 @@ public HiveMetadata(
this.tableParameterCodec = requireNonNull(tableParameterCodec, "tableParameterCodec is null");
this.partitionUpdateCodec = requireNonNull(partitionUpdateCodec, "partitionUpdateCodec is null");
this.partitionUpdateSmileCodec = requireNonNull(partitionUpdateSmileCodec, "partitionUpdateSmileCodec is null");
this.writesToNonManagedTablesEnabled = writesToNonManagedTablesEnabled;
this.createsOfNonManagedTablesEnabled = createsOfNonManagedTablesEnabled;
this.maxPartitionBatchSize = maxPartitionBatchSize;
this.typeTranslator = requireNonNull(typeTranslator, "typeTranslator is null");
Expand All @@ -478,6 +476,7 @@ public HiveMetadata(
this.encryptionInformationProvider = requireNonNull(encryptionInformationProvider, "encryptionInformationProvider is null");
this.hivePartitionStats = requireNonNull(hivePartitionStats, "hivePartitionStats is null");
this.hiveFileRenamer = requireNonNull(hiveFileRenamer, "hiveFileRenamer is null");
this.tableWritabilityChecker = requireNonNull(tableWritabilityChecker, "tableWritabilityChecker is null");
}

public SemiTransactionalHiveMetastore getMetastore()
Expand Down Expand Up @@ -1874,7 +1873,12 @@ private HiveInsertTableHandle beginInsertInternal(ConnectorSession session, Conn
Table table = metastore.getTable(metastoreContext, tableName.getSchemaName(), tableName.getTableName())
.orElseThrow(() -> new TableNotFoundException(tableName));

checkTableIsWritable(table, writesToNonManagedTablesEnabled, metastore.getTableConstraints(metastoreContext, tableName.getSchemaName(), tableName.getTableName()));
tableWritabilityChecker.checkTableWritable(table);

List<TableConstraint<String>> constraints = metastore.getTableConstraints(metastoreContext, tableName.getSchemaName(), tableName.getTableName());
if (constraints.stream().anyMatch(TableConstraint::isEnforced)) {
throw new PrestoException(NOT_SUPPORTED, format("Cannot write to table %s since it has table constraints that are enforced", table.getSchemaTableName().toString()));
}

for (Column column : table.getDataColumns()) {
if (!isWritableType(column.getType())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public class HiveMetadataFactory
private final boolean skipDeletionForAlter;
private final boolean skipTargetCleanupOnRollback;
private final boolean undoMetastoreOperationsEnabled;
private final boolean writesToNonManagedTablesEnabled;
private final boolean createsOfNonManagedTablesEnabled;
private final int maxPartitionBatchSize;
private final long perTransactionCacheMaximumSize;
Expand Down Expand Up @@ -72,6 +71,7 @@ public class HiveMetadataFactory
private final HiveFileRenamer hiveFileRenamer;
private final ColumnConverterProvider columnConverterProvider;
private final QuickStatsProvider quickStatsProvider;
private final TableWritabilityChecker tableWritabilityChecker;

@Inject
@SuppressWarnings("deprecation")
Expand Down Expand Up @@ -99,7 +99,8 @@ public HiveMetadataFactory(
HivePartitionStats hivePartitionStats,
HiveFileRenamer hiveFileRenamer,
ColumnConverterProvider columnConverterProvider,
QuickStatsProvider quickStatsProvider)
QuickStatsProvider quickStatsProvider,
TableWritabilityChecker tableWritabilityChecker)
{
this(
metastore,
Expand All @@ -109,7 +110,6 @@ public HiveMetadataFactory(
hiveClientConfig.getAllowCorruptWritesForTesting(),
hiveClientConfig.isSkipDeletionForAlter(),
hiveClientConfig.isSkipTargetCleanupOnRollback(),
hiveClientConfig.getWritesToNonManagedTablesEnabled(),
hiveClientConfig.getCreatesOfNonManagedTablesEnabled(),
hiveClientConfig.isUndoMetastoreOperationsEnabled(),
hiveClientConfig.getMaxPartitionBatchSize(),
Expand All @@ -134,7 +134,8 @@ public HiveMetadataFactory(
hivePartitionStats,
hiveFileRenamer,
columnConverterProvider,
quickStatsProvider);
quickStatsProvider,
tableWritabilityChecker);
}

public HiveMetadataFactory(
Expand All @@ -145,7 +146,6 @@ public HiveMetadataFactory(
boolean allowCorruptWritesForTesting,
boolean skipDeletionForAlter,
boolean skipTargetCleanupOnRollback,
boolean writesToNonManagedTablesEnabled,
boolean createsOfNonManagedTablesEnabled,
boolean undoMetastoreOperationsEnabled,
int maxPartitionBatchSize,
Expand All @@ -170,12 +170,12 @@ public HiveMetadataFactory(
HivePartitionStats hivePartitionStats,
HiveFileRenamer hiveFileRenamer,
ColumnConverterProvider columnConverterProvider,
QuickStatsProvider quickStatsProvider)
QuickStatsProvider quickStatsProvider,
TableWritabilityChecker tableWritabilityChecker)
{
this.allowCorruptWritesForTesting = allowCorruptWritesForTesting;
this.skipDeletionForAlter = skipDeletionForAlter;
this.skipTargetCleanupOnRollback = skipTargetCleanupOnRollback;
this.writesToNonManagedTablesEnabled = writesToNonManagedTablesEnabled;
this.createsOfNonManagedTablesEnabled = createsOfNonManagedTablesEnabled;
this.undoMetastoreOperationsEnabled = undoMetastoreOperationsEnabled;
this.maxPartitionBatchSize = maxPartitionBatchSize;
Expand Down Expand Up @@ -205,6 +205,7 @@ public HiveMetadataFactory(
this.hiveFileRenamer = requireNonNull(hiveFileRenamer, "hiveFileRenamer is null");
this.columnConverterProvider = requireNonNull(columnConverterProvider, "columnConverterProvider is null");
this.quickStatsProvider = requireNonNull(quickStatsProvider, "quickStatsProvider is null");
this.tableWritabilityChecker = requireNonNull(tableWritabilityChecker, "tableWritabilityChecker is null");

if (!allowCorruptWritesForTesting && !timeZone.equals(DateTimeZone.getDefault())) {
log.warn("Hive writes are disabled. " +
Expand Down Expand Up @@ -232,7 +233,6 @@ public HiveMetadata get()
partitionManager,
timeZone,
allowCorruptWritesForTesting,
writesToNonManagedTablesEnabled,
createsOfNonManagedTablesEnabled,
maxPartitionBatchSize,
typeManager,
Expand All @@ -251,6 +251,7 @@ public HiveMetadata get()
partitionObjectBuilder,
encryptionInformationProvider,
hivePartitionStats,
hiveFileRenamer);
hiveFileRenamer,
tableWritabilityChecker);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* 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;

import com.facebook.presto.hive.metastore.PrestoTableType;
import com.facebook.presto.hive.metastore.Table;
import com.facebook.presto.spi.PrestoException;

import javax.inject.Inject;

import java.util.Optional;

import static com.facebook.presto.hive.HiveWriteUtils.checkWritable;
import static com.facebook.presto.hive.metastore.MetastoreUtil.getProtectMode;
import static com.facebook.presto.hive.metastore.PrestoTableType.MANAGED_TABLE;
import static com.facebook.presto.hive.metastore.PrestoTableType.MATERIALIZED_VIEW;
import static com.facebook.presto.hive.metastore.PrestoTableType.TEMPORARY_TABLE;
import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED;
import static java.util.Objects.requireNonNull;

public class HiveTableWritabilityChecker
implements TableWritabilityChecker
{
private final boolean writesToNonManagedTablesEnabled;

@Inject
public HiveTableWritabilityChecker(HiveClientConfig hiveClientConfig)
{
this(requireNonNull(hiveClientConfig, "hiveClientConfig is null").getWritesToNonManagedTablesEnabled());
}

public HiveTableWritabilityChecker(boolean writesToNonManagedTablesEnabled)
{
this.writesToNonManagedTablesEnabled = writesToNonManagedTablesEnabled;
}

@Override
public void checkTableWritable(Table table)
{
PrestoTableType tableType = table.getTableType();
if (!writesToNonManagedTablesEnabled
&& !tableType.equals(MANAGED_TABLE)
&& !tableType.equals(MATERIALIZED_VIEW)
&& !tableType.equals(TEMPORARY_TABLE)) {
throw new PrestoException(NOT_SUPPORTED, "Cannot write to non-managed Hive table");
}

checkWritable(
table.getSchemaTableName(),
Optional.empty(),
getProtectMode(table),
table.getParameters(),
table.getStorage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,13 @@
import com.facebook.presto.hive.metastore.Database;
import com.facebook.presto.hive.metastore.MetastoreContext;
import com.facebook.presto.hive.metastore.Partition;
import com.facebook.presto.hive.metastore.PrestoTableType;
import com.facebook.presto.hive.metastore.SemiTransactionalHiveMetastore;
import com.facebook.presto.hive.metastore.Storage;
import com.facebook.presto.hive.metastore.Table;
import com.facebook.presto.hive.s3.PrestoS3FileSystem;
import com.facebook.presto.spi.ConnectorSession;
import com.facebook.presto.spi.PrestoException;
import com.facebook.presto.spi.SchemaNotFoundException;
import com.facebook.presto.spi.SchemaTableName;
import com.facebook.presto.spi.constraints.TableConstraint;
import com.facebook.presto.spi.security.ConnectorIdentity;
import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Shorts;
Expand Down Expand Up @@ -118,9 +115,6 @@
import static com.facebook.presto.hive.metastore.MetastoreUtil.isUserDefinedTypeEncodingEnabled;
import static com.facebook.presto.hive.metastore.MetastoreUtil.pathExists;
import static com.facebook.presto.hive.metastore.MetastoreUtil.verifyOnline;
import static com.facebook.presto.hive.metastore.PrestoTableType.MANAGED_TABLE;
import static com.facebook.presto.hive.metastore.PrestoTableType.MATERIALIZED_VIEW;
import static com.facebook.presto.hive.metastore.PrestoTableType.TEMPORARY_TABLE;
import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED;
import static java.lang.Float.intBitsToFloat;
import static java.lang.Math.toIntExact;
Expand Down Expand Up @@ -301,28 +295,6 @@ else if (isRowType(type)) {
throw new IllegalArgumentException("unsupported type: " + type);
}

public static void checkTableIsWritable(Table table, boolean writesToNonManagedTablesEnabled, List<TableConstraint<String>> constraints)
{
PrestoTableType tableType = table.getTableType();
if (!writesToNonManagedTablesEnabled
&& !tableType.equals(MANAGED_TABLE)
&& !tableType.equals(MATERIALIZED_VIEW)
&& !tableType.equals(TEMPORARY_TABLE)) {
throw new PrestoException(NOT_SUPPORTED, "Cannot write to non-managed Hive table");
}

if (constraints.stream().anyMatch(TableConstraint::isEnforced)) {
throw new PrestoException(NOT_SUPPORTED, format("Cannot write to table %s since it has table constraints that are enforced", table.getSchemaTableName().toString()));
}

checkWritable(
table.getSchemaTableName(),
Optional.empty(),
getProtectMode(table),
table.getParameters(),
table.getStorage());
}

public static void checkPartitionIsWritable(String partitionName, Partition partition)
{
checkWritable(
Expand All @@ -333,7 +305,7 @@ public static void checkPartitionIsWritable(String partitionName, Partition part
partition.getStorage());
}

private static void checkWritable(
public static void checkWritable(
SchemaTableName tableName,
Optional<String> partitionName,
ProtectMode protectMode,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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;

import com.facebook.presto.hive.metastore.Table;

public interface TableWritabilityChecker
{
void checkTableWritable(Table table);
}
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,6 @@ protected final void setup(String databaseName, HiveClientConfig hiveClientConfi
true,
false,
false,
false,
true,
true,
getHiveClientConfig().getMaxPartitionBatchSize(),
Expand All @@ -1041,7 +1040,8 @@ protected final void setup(String databaseName, HiveClientConfig hiveClientConfi
new HivePartitionStats(),
new HiveFileRenamer(),
DEFAULT_COLUMN_CONVERTER_PROVIDER,
new QuickStatsProvider(HDFS_ENVIRONMENT, DO_NOTHING_DIRECTORY_LISTER, new HiveClientConfig(), new NamenodeStats(), ImmutableList.of()));
new QuickStatsProvider(HDFS_ENVIRONMENT, DO_NOTHING_DIRECTORY_LISTER, new HiveClientConfig(), new NamenodeStats(), ImmutableList.of()),
new HiveTableWritabilityChecker(false));

transactionManager = new HiveTransactionManager();
encryptionInformationProvider = new HiveEncryptionInformationProvider(ImmutableList.of());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ protected void setup(String host, int port, String databaseName, BiFunction<Hive
new HivePartitionStats(),
new HiveFileRenamer(),
columnConverterProvider,
new QuickStatsProvider(HDFS_ENVIRONMENT, DO_NOTHING_DIRECTORY_LISTER, new HiveClientConfig(), new NamenodeStats(), ImmutableList.of()));
new QuickStatsProvider(HDFS_ENVIRONMENT, DO_NOTHING_DIRECTORY_LISTER, new HiveClientConfig(), new NamenodeStats(), ImmutableList.of()),
new HiveTableWritabilityChecker(config));

transactionManager = new HiveTransactionManager();
splitManager = new HiveSplitManager(
Expand Down
Loading

0 comments on commit 331e3d8

Please sign in to comment.