lasdf1234 opened a new issue, #14736: URL: https://github.com/apache/gravitino/issues/14736
### What would you like to be improved? `IcebergOrphanFileRemovalContent` stores an optional absolute `location`. `validate()` only checks URI syntax (absolute path, no `..`, no query/fragment, no encodings). Nothing checks the value against a table, so values like `file:///` or `s3://some-other-bucket/` save through policy create/alter. A policy reaches tables through tags. A tag on a catalog or schema carries the policy to every table below it. A policy with `location` set therefore lands on tables it cannot apply to. No wrong files are deleted: `IcebergRemoveOrphanFilesJob` still rejects any location outside the table via `OrphanFileLocationUtils.validateLocation`. But the policy saves fine and then fails on every run, on every table but at most one. Scenarios: - Policy with `location` linked to a tag on a schema or catalog → every table below gets it; runs outside the one matching table fail. - Policy with `location` reaching several tables through table-level tags → one absolute path matches at most one of them. - A table created or registered later under a tagged schema → the policy silently gains another table it cannot apply to. - Policy on one table whose `location` is outside that table → saves, then fails on every run. ### How should we improve? Keep the field name `location` and its Iceberg meaning: an absolute path within one table's location, as in Iceberg's `remove_orphan_files`. Restrict where it may be used: 1. An orphan-file-removal policy that sets `location` may only be bound **directly** to a **table**. It cannot be associated with a regular tag, and it has at most one direct binding. 2. `location` must be inside that table's location, using the same containment rule as `OrphanFileLocationUtils.validateLocation`. 3. Without `location`, behavior is unchanged: the policy can be associated with any tag at any level and scans the whole table location. Enforce on three writes (reject with an error naming the policy and, where involved, the table and its location): - policy create and content update (setting/changing `location` when the policy already has a tag association or more than one direct binding, or a value outside the bound table); - associating the policy with a tag; - binding the policy directly to a table. Do **not** resolve “which tables does this policy reach?” on tag assignment, tag value/selector changes, enabling a policy, or table creation. Inheritance must never carry a policy with `location`. Keep run-time `validateLocation` as the backstop when a table location changes outside Gravitino. **Why not a relative `scanSubPath`:** a catalog/schema-level relative path can quietly skip tables that write elsewhere (and “missing subpath = success with nothing removed” hides that); table layouts are independent; Iceberg / similar systems keep an absolute path for the single-table case. ### Test case (reproduces today, passes after the fix) Catalog `ice` with tables `s1.t1` at `file:///tmp/wh/s1/t1` and `s1.t2` at `file:///tmp/wh/s1/t2`. 1. Create policy `orphan_loc` with `location: file:///` → today accepted; after fix may exist unbound, but step 2 must reject. 2. Associate it with a tag → today 200; **expected: rejected** (policy with `location` cannot be tag-associated). 3. Assign that tag to schema `ice.s1` → today both tables show the policy and cleanup fails in `validateLocation`. Pass criteria: - Step 2 rejected with an error naming `orphan_loc`. - Direct binding of `orphan_loc` to `ice.s1.t2` rejected while `location` is `file:///`, accepted after updating to a path under `file:///tmp/wh/s1/t2`. - A second direct binding of `orphan_loc` to `ice.s1.t1` rejected. - Updating `location` to a path under `t1` while bound to `t2` rejected. - A policy without `location` still associates with a tag and appears on both tables. -- 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]
