laserninja commented on code in PR #10757:
URL: https://github.com/apache/gravitino/pull/10757#discussion_r3231383919
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoMetadata.java:
##########
@@ -230,12 +230,19 @@ public ConnectorOutputTableHandle beginCreateTable(
ConnectorTableMetadata tableMetadata,
Optional<ConnectorTableLayout> layout,
RetryMode retryMode,
- boolean noExistingData) {
- // First, create the table in the Gravitino catalog
+ boolean replace) {
+ SchemaTableName tableName = tableMetadata.getTable();
+
+ // CREATE OR REPLACE TABLE AS SELECT: drop the existing table first if
present.
+ if (replace
+ && catalogConnectorMetadata.tableExists(
+ tableName.getSchemaName(), tableName.getTableName())) {
+ catalogConnectorMetadata.dropTable(tableName);
Review Comment:
Thanks for the feedback. The CREATE OR REPLACE TABLE AS SELECT path was
removed rather than kept with a workaround because of a fundamental
incompatibility with the Iceberg internal connector's table-identity tracking.
Root cause: The internal Iceberg connector caches the table's UUID at
query-plan time. The CTAS transaction flow is:
beginCreateTable → beginInsert → commitTransaction.
When replace=true, we dropped and recreated the table inside
beginCreateTable. But beginInsert then calls beginTransaction → refresh(),
which compares the cached UUID against the new table and throws
IllegalStateException: Table UUID does not match: current=<old> !=
refreshed=<new>. There is no public API in the internal connector to reset this
cache.
Throwing NOT_SUPPORTED is the standard Trino pattern for operations the
connector genuinely cannot execute, it's what the default ConnectorMetadata
base class does for any unimplemented method. The error message surfaces
cleanly to the user: "This connector does not support replacing a table".
I'm happy to add an inline comment in the code explaining the UUID cache
incompatibility so future maintainers understand why replace=true is rejected
rather than implemented. I can also add a note to the docs if there's a
connector-limitations section to update, please point me to the right file.
Would that address the concern?
--
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]