OjashKush commented on code in PR #20012:
URL: https://github.com/apache/hudi/pull/20012#discussion_r4116770444


##########
hudi-trino/src/main/java/io/trino/plugin/hudi/HudiMetadata.java:
##########
@@ -304,6 +321,202 @@ 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 = HudiMetastoreTables.buildTable(
+                schemaTableName,
+                basePath,
+                getTableType(properties),
+                tableSchema,
+                getPartitionedBy(properties),
+                external,
+                Optional.of(session.getUser()),
+                tableMetadata.getComment());
+        try {
+            HudiTableInitializer.initializeTable(fileSystem, basePath, 
tableMetadata, tableSchema);
+            metastore.createTable(table, NO_PRIVILEGES);
+        }
+        catch (TableAlreadyExistsException e) {
+            // Another CREATE TABLE may have initialized the same managed 
location and won the
+            // metastore race. Its catalog entry now owns .hoodie, so deleting 
it would corrupt the
+            // live table. A different location still belongs to this failed 
attempt and is safe to
+            // clean up.
+            if (!isTableRegisteredAtLocation(schemaTableName, basePath, e)) {
+                cleanupTableMetadata(fileSystem, basePath, e);
+            }
+            throw e;
+        }
+        catch (RuntimeException e) {
+            // Initialization may have written some or all of .hoodie, but no 
catalog entry from
+            // this call references it. Left there it would make a retry fail 
the emptiness check.
+            //
+            // Only .hoodie is removed, never the base path: the base path may 
have been created by
+            // someone else, and this connector did not create it. On object 
storage there is no
+            // directory to delete in any case -- deleteDirectory removes the 
objects under the
+            // prefix, which is exactly the set initTable wrote or may have 
partially written.
+            cleanupTableMetadata(fileSystem, basePath, e);
+            throw e;
+        }
+    }
+
+    private boolean isTableRegisteredAtLocation(SchemaTableName tableName, 
String basePath, RuntimeException failure)
+    {
+        try {
+            return metastore.getTable(tableName.getSchemaName(), 
tableName.getTableName())
+                    .map(table -> 
table.getStorage().getLocation().equals(basePath))

Review Comment:
   fix done:
   
     1. Add trino_query_id to CREATE TABLE parameters.
     2. After any create failure, re-read the metastore table.
     3. If its query ID matches, treat the retried create as successful.
     4. If another table is registered, preserve .hoodie regardless of location.
     5. Clean .hoodie only when the metastore confirms no table is registered.



-- 
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]

Reply via email to