ayushtkn commented on code in PR #6872: URL: https://github.com/apache/hive/pull/6872#discussion_r4274401596
########## iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergMetaHook.java: ########## Review Comment: I looked at moving the commit flag to the start of the `icebergTable == null` block (table-only path). That would scope the fix to tables and avoid running table rollback logic on the view branch, but it would also mean that on a failed view create we’d leave an orphan HMS entry again: `delegate.createTable` still creates the HMS row, then `createOrReplaceView` fails, and with the flag unset rollback would no-op — the same failure mode this JIRA fixes for tables. For this PR I’m keeping the flag at the top of `commitCreateTable` so rollback runs for both paths and the HMS name can be cleared when Iceberg commit fails. The cleanup still goes through `Catalogs.dropTable`, which is the table catalog API; for a fully committed Iceberg view the right catalog call would be `HiveCatalog#dropView` (view metadata + HMS). That said, normal Hive `DROP VIEW` also goes through `HookEnabledMetaStoreClient#dropTable` and HMS `dropTable` — there is no separate `dropView` hook on the metastore client — and `HiveIcebergMetaHook#commitDropTable` doesn’t call `dropView` for typical view drops either. So for failed create rollback, dropTable is imperfect at the Iceberg layer but aligned with “remove the HMS object” for the common stub / failed-commit case. I think there would similar issue with Views, I would look into views as a followup if that can be improved, or if this OOTB doesn't fix it, I will be upset though, if it doesn't, atleast that was my quick guess... -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
