osscm commented on code in PR #11041: URL: https://github.com/apache/iceberg/pull/11041#discussion_r4099228449
########## format/view-spec.md: ########## @@ -82,9 +97,12 @@ Each version in `versions` is a struct with the following fields: | _required_ | `representations` | A list of [representations](#representations) for the view definition | | _optional_ | `default-catalog` | Catalog name to use when a reference in the SELECT does not contain a catalog | | _required_ | `default-namespace` | Namespace to use when a reference in the SELECT is a single identifier | +| _optional_ | `storage-table` | A [storage table identifier](#storage-table-identifier) of the storage table | Review Comment: > Placing `storage-table` inside the immutable `Version` struct creates a few unaddressed cases: > > * A new view version can carry a different `storage-table` identifier than the previous version. Is that allowed? > * A new version can omit `storage-table`, which (per line 104) reverts the entity to a common view. Is that allowed? > * On rollback to a prior version, what storage table is current? > > Is there any production SQL engine allows converting a regular view to a materialized view (or vice versa) in place? if not, should the `storage-table` be a top-level view metadata field? Any view/MV conversion have to go through the drop and re-create path in this case. Sanity-checking something with people who know the catalog implementations better than I do: `ViewCatalog.renameView()` is a required method, and HiveCatalog implements it as a single atomic alter_table call - but for REST-backed catalogs, `RESTSessionCatalog.renameView()` looks like it's gated behind server-advertised endpoint support rather than guaranteed. If that reading is correct, a producer can't unconditionally rely on renameView() being available for a materialized view's rename, since a given REST catalog server might simply not implement that endpoint - so an atomic "view + storage-table" rename could silently degrade to something else (drop+recreate, or fail) depending on which catalog implementation is behind the scenes. Separately, on the rollback case: if current-version-id rolls back to a version with a different (or absent) storage-table, is the previous storage table expected to be dropped/orphaned by the producer, or does it just sit unreferenced until GC'd? Wasn't sure if that's out of scope for v1 or needs explicit spec language to avoid silently leaking storage tables. I think the `ALTER...` should not allow to change the Storage table. -- 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]
