[PR] [core] Fix JDBC catalog drop, alter and create failure handling [paimon]

["LuciferYang (via GitHub)" <gi...@apache.org>]

LuciferYang opened a new pull request, #10270:
URL: https://github.com/apache/paimon/pull/10270

   ### Purpose
   
   `JdbcCatalog` mishandled three failure paths.
   
   1. Cascade `dropDatabaseImpl` removed only the four JDBC row sets and never deleted the database directory, leaking `warehouse/<db>.db/` (schema and data) and desyncing the catalog from the filesystem. Recreating the same database and table then collided with the stale `schema-0`. It now deletes the directory, matching `FileSystemCatalog`. This runs only after the `dropDatabase` override has already done the existence and cascade/empty checks, so the recursive delete is safe.
   
   2. `alterDatabaseImpl` did no existence check: altering a missing database with `ignoreIfNotExists = false` inserted a property row and returned success instead of throwing `DatabaseNotExistException`, and `databaseExists` then reported the database as present, materializing a phantom database. It now verifies existence first.
   
   3. `createTableImplWithLock` leaked the created directory when a create failed after the schema was committed to the filesystem but before the JDBC row was inserted. It now removes the directory on that path, guarded so it deletes only a directory this call created and then failed to register (a `schemaCreated` flag combined with the existing `registered` flag). A table that already exists on the filesystem but is absent from the JDBC catalog (the state `repairTable` recovers) is left untouched.
   
   This closes #10269.
   
   ### Tests
   
   - `testDropDatabaseCascadeDeletesWarehouseDirectory` pins that a cascade drop removes the database directory.
   - `testAlterDatabaseOnMissingDatabaseThrows` pins that altering a missing database throws instead of creating a phantom entry.
   - `testCreateTablePreservesUnregisteredOnDiskTableOnFailure` pins that a failed create does not delete a pre-existing on-disk table that is absent from the JDBC catalog, and that it stays recoverable via `repairTable`.
   
   ### API and Format
   
   No.
   
   ### Documentation
   
   No.
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [core] Fix JDBC catalog cascade drop and alter of missing database [paimon]

["LuciferYang (via GitHub)" <gi...@apache.org>]

LuciferYang closed pull request #10270: [core] Fix JDBC catalog cascade drop and alter of missing database
URL: https://github.com/apache/paimon/pull/10270


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [core] Fix JDBC catalog drop, alter and create failure handling [paimon]

["JingsongLi (via GitHub)" <gi...@apache.org>]

JingsongLi commented on PR #10270:
URL: https://github.com/apache/paimon/pull/10270#issuecomment-5936470741

   I found three introduced failure/isolation problems. The normal JDK 8 reactor run of `JdbcCatalogTest` passed all 81 tests; the additional reproductions below use actual local SQLite metadata and compare with the merge-base `JdbcCatalog` implementation.
   
   **[P1] Delete data only after metadata deletion succeeds — `JdbcCatalog.java:308`.** The directory is now removed before the first JDBC DELETE. If that DELETE fails (for example, permission denial or a storage/connection failure), the operation throws but registered tables have already lost their schema and data. I created and committed a real row `42`, then used a SQLite trigger to reject deletion from `paimon_tables`: HEAD reports the DROP failure and still lists the table, but its entire directory is gone; the same program with the merge-base catalog preserves the table and reads row `42`. Please preserve files when metadata deletion fails and perform cleanup after successful metadata removal. `dropTableImpl` already uses this ordering.
   
   **[P1] Restrict cleanup to this catalog's owned tables — `JdbcCatalog.java:308`.** JDBC SQL filters by `catalogKey`, but `newDatabasePath(name)` is only `warehouse/<name>.db`. Different catalog keys can therefore have independent databases and different tables in the same warehouse directory. In a real two-catalog test, A and B share the JDBC database and warehouse, both create `db`, and only B owns `b_table` with committed row `42`. A's `listTables("db")` is empty, so `A.dropDatabase("db", false, false)` succeeds, then HEAD deletes B's files while B still lists its registered table. The merge-base preserves and reads B's row. The documented catalog-key isolation must survive a DROP in another catalog; recursively deleting the shared directory is unsafe. Enumerate and clean this catalog's registered table paths, preserving other catalogs and recoverable unregistered tables.
   
   **[P2] A thrown INSERT call does not prove registration failed — `JdbcCatalog.java:585–586`.** `JdbcUtils.insertTable` executes an auto-committed INSERT inside try-with-resources. If the INSERT commits and `PreparedStatement.close()` throws, the assignment to `registered` never happens, so this new catch deletes a table that is already registered. I injected a close failure *after* the real SQLite INSERT/statement close: HEAD still lists the committed table row but deletes its schema; the merge-base leaves it registered and readable. A lost response after commit has the same ambiguous outcome. Please confirm persistent registration/transaction rollback before removing the created directory, rather than treating `registered == false` as proof that no catalog row exists.
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org