nevzheng opened a new pull request, #14252:
URL: https://github.com/apache/gravitino/pull/14252
### What changes were proposed in this pull request?
- **Iceberg REST:** a new package-private `IcebergReservedPropertyValidator`
rejects `gravitino.identifier`
with 400 (`IllegalArgumentException`, like the sibling validators). It
runs first in createTable,
updateTable (SetProperties), createView, replaceView, createNamespace and
updateNamespace.
- **Core:** a new `OperationDispatcher.canReuseStringId(...)`, used by
import in
`Table/View/SchemaOperationDispatcher`.
- An import keeps the id it carries only when a sibling entity already has
that id and either has the
same name or has an old name that is gone from the catalog (an external
rename).
- Otherwise the import gets a fresh id. Overwriting a stale row under the
same name still works, always
with that fresh id, so the upsert can never land on another entity's row.
### Why are the changes needed?
**An Iceberg REST client could set `gravitino.identifier` to another
object's id.** Import then reused
that id, the store renamed or moved the other object's row, and the hook
made the caller its owner. For
tables and schemas this worked across catalogs.
Fix: #14244
### Does this PR introduce _any_ user-facing change?
Yes, two:
- Iceberg REST create and update requests that set `gravitino.identifier`
now get 400.
- An object moved between schemas outside Gravitino is now imported under a
fresh id, so its grants do
not follow it.
`registerTable` and `registerView` still accept the property, because
metadata files written by
Gravitino carry it. Those paths rely on the core check.
### How was this patch tested?
- **`TestImportStringIdentifierReuse` (real H2 store, 4 cases):** a view in
the same schema, a table
across catalogs, a schema across catalogs, and a legitimate external
rename.
- **On `main`:** the 3 reuse cases fail, and the rename case passes.
- **With the fix:** all 4 pass.
- **`TestIcebergReservedPropertyRejection` (7 cases):** on `main` the 6
rejection cases fail; with the fix
all 7 pass.
- **Regressions:** all `Test*OperationDispatcher` tests in core and the
`TestIceberg*` dispatcher tests in
iceberg-rest-server pass.
--
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]
