Skip to content

Move already created check inside of metastore implementations #19969

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

Merged
dain merged 2 commits into from
Dec 2, 2023
Merged

Move already created check inside of metastore implementations #19969

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
9707977
Convert TestingThriftHiveMetastoreBuilder to TrinoFileSystem
dain Nov 30, 2023
4295ae8
Move already created check inside of metastore implementations
dain Nov 30, 2023
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
56 changes: 3 additions & 53 deletions plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@
import io.trino.plugin.deltalake.transactionlog.writer.TransactionLogWriter;
import io.trino.plugin.deltalake.transactionlog.writer.TransactionLogWriterFactory;
import io.trino.plugin.hive.HiveType;
import io.trino.plugin.hive.SchemaAlreadyExistsException;
import io.trino.plugin.hive.TableAlreadyExistsException;
import io.trino.plugin.hive.TrinoViewHiveMetastore;
import io.trino.plugin.hive.metastore.Column;
import io.trino.plugin.hive.metastore.Database;
Expand Down Expand Up @@ -836,18 +834,7 @@ public void createSchema(ConnectorSession session, String schemaName, Map<String
"Database '%s' does not have correct query id set",
database.getDatabaseName());

try {
metastore.createDatabase(database);
}
catch (SchemaAlreadyExistsException e) {
// Ignore SchemaAlreadyExistsException when database looks like created by us.
// This may happen when an actually successful metastore create call is retried
// e.g. because of a timeout on our side.
Optional<Database> existingDatabase = metastore.getDatabase(schemaName);
if (existingDatabase.isEmpty() || !isCreatedBy(existingDatabase.get(), queryId)) {
throw e;
}
}
metastore.createDatabase(database);
}

@Override
Expand Down Expand Up @@ -989,21 +976,7 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe
// As a precaution, clear the caches
statisticsAccess.invalidateCache(schemaTableName, Optional.of(location));
transactionLogAccess.invalidateCache(schemaTableName, Optional.of(location));
try {
metastore.createTable(
session,
table,
principalPrivileges);
}
catch (TableAlreadyExistsException e) {
// Ignore TableAlreadyExistsException when table looks like created by us.
// This may happen when an actually successful metastore create call is retried
// e.g. because of a timeout on our side.
Optional<Table> existingTable = metastore.getRawMetastoreTable(schemaName, tableName);
if (existingTable.isEmpty() || !isCreatedBy(existingTable.get(), queryId)) {
throw e;
}
}
metastore.createTable(session, table, principalPrivileges);
}

public static Table buildTable(ConnectorSession session, SchemaTableName schemaTableName, String location, boolean isExternal)
Expand Down Expand Up @@ -1317,18 +1290,7 @@ public Optional<ConnectorOutputMetadata> finishCreateTable(
// As a precaution, clear the caches
statisticsAccess.invalidateCache(schemaTableName, Optional.of(location));
transactionLogAccess.invalidateCache(schemaTableName, Optional.of(location));
try {
metastore.createTable(session, table, principalPrivileges);
}
catch (TableAlreadyExistsException e) {
// Ignore TableAlreadyExistsException when table looks like created by us.
// This may happen when an actually successful metastore create call is retried
// e.g. because of a timeout on our side.
Optional<Table> existingTable = metastore.getRawMetastoreTable(schemaName, tableName);
if (existingTable.isEmpty() || !isCreatedBy(existingTable.get(), queryId)) {
throw e;
}
}
metastore.createTable(session, table, principalPrivileges);
}
catch (Exception e) {
// Remove the transaction log entry if the table creation fails
Expand All @@ -1346,18 +1308,6 @@ public Optional<ConnectorOutputMetadata> finishCreateTable(
return Optional.empty();
}

private static boolean isCreatedBy(Database database, String queryId)
{
Optional<String> databaseQueryId = getQueryId(database);
return databaseQueryId.isPresent() && databaseQueryId.get().equals(queryId);
}

public static boolean isCreatedBy(Table table, String queryId)
{
Optional<String> tableQueryId = getQueryId(table);
return tableQueryId.isPresent() && tableQueryId.get().equals(queryId);
}

@Override
public void setTableComment(ConnectorSession session, ConnectorTableHandle tableHandle, Optional<String> comment)
{
Expand Down
18 changes: 1 addition & 17 deletions ...-delta-lake/src/main/java/io/trino/plugin/deltalake/procedure/RegisterTableProcedure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import io.trino.plugin.deltalake.statistics.CachingExtendedStatisticsAccess;
import io.trino.plugin.deltalake.transactionlog.TableSnapshot;
import io.trino.plugin.deltalake.transactionlog.TransactionLogAccess;
import io.trino.plugin.hive.TableAlreadyExistsException;
import io.trino.plugin.hive.metastore.PrincipalPrivileges;
import io.trino.plugin.hive.metastore.Table;
import io.trino.spi.TrinoException;
Expand All @@ -48,7 +47,6 @@
import static io.trino.plugin.deltalake.DeltaLakeErrorCode.DELTA_LAKE_INVALID_TABLE;
import static io.trino.plugin.deltalake.DeltaLakeMetadata.buildTable;
import static io.trino.plugin.deltalake.DeltaLakeMetadata.getQueryId;
import static io.trino.plugin.deltalake.DeltaLakeMetadata.isCreatedBy;
import static io.trino.plugin.deltalake.transactionlog.TransactionLogUtil.getTransactionLogDir;
import static io.trino.plugin.hive.metastore.MetastoreUtil.buildInitialPrivilegeSet;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
Expand Down Expand Up @@ -185,21 +183,7 @@ private void doRegisterTable(
getQueryId(table).orElseThrow(() -> new IllegalArgumentException("Query id is not present")).equals(queryId),
"Table '%s' does not have correct query id set",
table);
try {
metastore.createTable(
session,
table,
principalPrivileges);
}
catch (TableAlreadyExistsException e) {
// Ignore TableAlreadyExistsException when table looks like created by us.
// This may happen when an actually successful metastore create call is retried
// e.g. because of a timeout on our side.
Optional<Table> existingTable = metastore.getRawMetastoreTable(schemaName, tableName);
if (existingTable.isEmpty() || !isCreatedBy(existingTable.get(), queryId)) {
throw e;
}
}
metastore.createTable(session, table, principalPrivileges);
}
}
}
123 changes: 0 additions & 123 deletions ...-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeCreateSchemaInternalRetry.java
View file
Open in desktop

This file was deleted.

Loading