lasdf1234 commented on PR #14552: URL: https://github.com/apache/gravitino/pull/14552#issuecomment-6599664241
Thanks for the thorough review against Iceberg 1.11.0 — addressed in 208abb2. 1. **[P1] Residual names:** `prepareTasksForResponse` remaps residuals (bound by field id, unbound by name) from the snapshot schema back onto the request/current schema before returning tasks. Added residual evaluation + plan-path assertions for the rename + name-reuse case. 2. **[P1] Client deserialization:** Sanitizing `specs-by-id` alone cannot help the client, which parses with its local `table.specs()`. Tasks whose partition type still has `unknown` are rewritten onto an existing unpartitioned spec so round-trip parse with the client's broken map succeeds. Covered with a serialize-with-sanitized / parse-with-client-specs test. 3. **[P2] Branch movement:** Filters on columns added after the snapshot are no longer rejected in the no-branch fallback. They become always-true for planning (same as `useRef` / null-column semantics) and are reattached via `postSnapshotResidual` so behavior does not depend on whether a branch still points at the snapshot. 4. **[P2] Source types:** Dropped sources are restored from historical table schemas (and identity partition data only when the schema type is missing). Transform result types and the binary guess are no longer used; specs that cannot be fully restored are left unchanged. 5. **[P2] Dedupe:** Restoration deduplicates by source id so `identity(s), bucket(s, …)` no longer adds duplicate `_dropped_source_*` fields. Also fixed `select: ["*"]` so the fallback rewrite treats it as project-all, matching Iceberg schema selection. -- 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]
