markhoerth opened a new pull request, #14440:
URL: https://github.com/apache/gravitino/pull/14440
Backport to branch-1.3 of the fix for #14438 (same change as the PR to
main). On branch-1.3 the OSS register path does not support overwrite, so the
register test checks that the UUID check refuses another table's metadata and
otherwise leaves the request to the backend.
### What changes were proposed in this pull request?
- `IcebergNamespaceHookDispatcher.createNamespace`: refuse a level
containing `.` or a full name longer than `EntityFieldLimits.MAX_NAME_LENGTH`
with 400 before creating anything. If the Gravitino import still fails, drop
the namespace again and report the failure.
- `IcebergTableHookDispatcher`: for a table create and a staged-create
commit, drop the table again (without purge) when Gravitino cannot import it;
an `UnsupportedOperationException` from the import (an unsupported column type)
is reported as 400. Register is not rolled back, since it may have overwritten
an existing table.
- `CatalogWrapperForREST.registerTable`: an overwrite whose metadata has a
different table UUID is refused with 400 before anything is written.
- New `IcebergCommitValidation`, added by
`CatalogWrapperForREST.updateTable` as an extra `UpdateRequirement`. Iceberg
evaluates requirements against the metadata the commit is built on, on every
retry, so the checks cannot be bypassed by a concurrent commit:
- `main` cannot be set to a tag (400);
- `add-snapshot` needs a known operation (`append`, `replace`,
`overwrite`, `delete`), a summary on v2+ tables, and a schema id the table has
(400);
- `remove-snapshots` of a snapshot a branch or tag references is refused
unless the same commit moves or removes that ref (409, so a client such as
Spark's `expire_snapshots` refreshes and recomputes).
- `IcebergRESTUtils.cloneIcebergRESTObject`: when Iceberg cannot serialize a
valid request for an event copy (a partition field without `field-id`), return
the original object instead of failing a request that has already been applied.
One case is not covered: when a snapshot summary omits `operation`,
Iceberg's `SnapshotParser` fills in `overwrite` while parsing, so the server
cannot tell it from a request that sent `overwrite`.
### Why are the changes needed?
Clients are told requests failed when they were applied, which leaves
unowned namespaces and tables. Unchecked commits leave tables that cannot be
written or loaded, and `remove-snapshots` can silently delete refs, including
under normal concurrent use (expire racing a tag or rollback).
Fixes #14438
### Does this PR introduce _any_ user-facing change?
Yes. The requests above are refused (400, or 409 for removing a referenced
snapshot) instead of being applied or half-applied.
### How was this patch tested?
New unit tests:
- `TestIcebergCommitValidation`: through Iceberg's
`CatalogHandlers.updateTable` on an in-memory catalog. Covers main as a tag,
invalid snapshots, removing referenced snapshots, removing with the ref in the
same commit, and a tag created after the client read the table.
- `TestIcebergNamespaceHookDispatcher` and `TestIcebergTableHookDispatcher`:
name checks before create, rollback on import failure, and a failing rollback
still reporting the original error.
- `TestCatalogWrapperForREST`: register overwrite with another table's
metadata on a JDBC (SQLite) catalog.
- `TestIcebergRESTUtils`: event copy of a request with a partition field
without `field-id`.
`./gradlew :iceberg:iceberg-rest-server:test -PskipITs`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Eu6fb9pLfmrSJzGQDVLi1g
--
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]
