voonhous commented on code in PR #20012:
URL: https://github.com/apache/hudi/pull/20012#discussion_r4194836082
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/HudiMetadata.java:
##########
@@ -304,6 +325,224 @@ public Optional<Object> getInfo(ConnectorSession session,
ConnectorTableHandle t
return Optional.of(new HudiTableInfo(table.getSchemaTableName(),
table.getTableType().name(), table.getBasePath()));
}
+ /**
+ * Creates an empty table: its {@code .hoodie} directory on storage and
its catalog entry.
+ * <p>
+ * This is the single-call path only. {@code beginCreateTable}/{@code
finishCreateTable} and a
+ * page sink are deliberately absent, so {@code CREATE TABLE AS} and
{@code INSERT} still fail as
+ * unsupported; no rows are written here.
+ * <p>
+ * An explicit {@code location} makes the table external, matching Hudi's
Spark SQL behaviour. An
+ * omitted one makes it managed, at {@code <schemaLocation>/<tableName>},
which is what decides
+ * whether {@code DROP TABLE} later deletes the data.
+ */
+ @Override
+ public void createTable(ConnectorSession session, ConnectorTableMetadata
tableMetadata, SaveMode saveMode)
+ {
+ SchemaTableName schemaTableName = tableMetadata.getTable();
+ if (saveMode == SaveMode.REPLACE) {
+ // Replacing a table means deciding what happens to the rows
already in it, which is the
+ // write path this connector does not yet have.
+ throw new TrinoException(NOT_SUPPORTED, "This connector does not
support replacing tables");
+ }
+ Database database =
metastore.getDatabase(schemaTableName.getSchemaName())
+ .orElseThrow(() -> new
SchemaNotFoundException(schemaTableName.getSchemaName()));
+ if (metastore.getTable(schemaTableName.getSchemaName(),
schemaTableName.getTableName()).isPresent()) {
+ if (saveMode == SaveMode.IGNORE) {
+ return;
+ }
+ throw new TrinoException(ALREADY_EXISTS, "Table already exists: "
+ schemaTableName);
+ }
+
+ // Everything that can be rejected is rejected before storage is
touched, so a bad statement
+ // leaves nothing behind.
+ HudiTableValidation.validateCreateTable(tableMetadata);
+
+ Map<String, Object> properties = tableMetadata.getProperties();
+ Optional<String> explicitLocation = getTableLocation(properties);
+ boolean external = explicitLocation.isPresent();
+ String basePath = explicitLocation.orElseGet(() ->
defaultTableLocation(database, schemaTableName));
+
+ TrinoFileSystem fileSystem = fileSystemFactory.create(session);
+ checkLocationIsEmpty(fileSystem, basePath);
+
+ // One schema object produces both hoodie.table.create.schema and the
metastore column list,
+ // so the two cannot disagree (HUDI-9435).
+ HoodieSchema tableSchema = HudiSchemaConverter.toTableSchema(
+ tableMetadata.getColumns(), schemaTableName.getTableName());
+ Table table = Table.builder(HudiMetastoreTables.buildTable(
+ schemaTableName,
+ basePath,
+ getTableType(properties),
+ tableSchema,
+ getPartitionedBy(properties),
+ external,
+ Optional.of(session.getUser()),
+ tableMetadata.getComment()))
+ .setParameter(TRINO_QUERY_ID_NAME, session.getQueryId())
+ .setParameter(TRINO_MANAGED_TABLE_PARAMETER,
Boolean.toString(!external))
+ .build();
+ try {
+ HudiTableInitializer.initializeTable(fileSystem, basePath,
tableMetadata, tableSchema);
+ metastore.createTable(table, NO_PRIVILEGES);
+ }
+ catch (RuntimeException e) {
+ Optional<Table> registeredTable;
+ try {
+ registeredTable =
metastore.getTable(schemaTableName.getSchemaName(),
schemaTableName.getTableName());
Review Comment:
**major:** Confirming this, and the consequence is destructive.
`createPerTransactionCache` caches misses for every `ObjectType`, and only
`createTable`'s `finally` invalidates (`CachingHiveMetastore.java:133-141`,
`633-641` in trino-metastore 484). So if `initializeTable` fails with anything
but `FileAlreadyExistsException` (e.g. a transient S3 error;
`S3OutputFile.create()` never throws FAE), this lookup returns the miss cached
at line 350 and line 416 deletes the `.hoodie` that a concurrent same-name
CREATE has already registered.
Could we invalidate the entry before this lookup (keep the
`CachingHiveMetastore` reference and call `invalidateTable`), and add a test
that wraps the proxy metastore in `createPerTransactionCache`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
