amitpoorab commented on issue #3758:
URL:
https://github.com/apache/iceberg-python/issues/3758#issuecomment-5234361294
I'd like to work on fixing this issue. I've reproduced it and traced the
root cause in detail.
## Root Cause (Summary)
The manifest pruning optimization (PR #3011) builds a deletion predicate by
aliasing partition **values** onto source **column names**. For temporal
transforms, this causes a type mismatch:
I've reproduced and debugged this issue step-by-step. The manifest pruning
optimization builds predicates by aliasing partition values onto source column
names, causing a cascade:
1. Predicate built: EqualTo(Reference('ts'), 20459) — mixing partition value
with source column
2. Silent coercion: LongLiteral(20459).to(TimestampType()) → interpreted as
microseconds
3. Transform re-applied: day(1970-01-01) → 0 instead of 20459
4. False negative: manifest evaluator says "no match" and skips it
5. Result: stale rows remain, untouched rows duplicate
This only affects temporal transforms because their large day-ordinals
(e.g., 20459) land far outside the manifest's bounds when misinterpreted as
microseconds.
## Proposed Fixes
I've prepared two approaches:
### Option A: Manifest Pruning Guard (Recommended for fast merge)
Restrict the manifest pruning optimization to identity-transform-only specs,
mirroring the guard already in `dynamic_partition_overwrite` (lines 569-574).
For non-identity transforms, fall back to pre-#3011 behavior (always open
manifest, exact file identity match — provably correct).
**Pros:** Small, diff (~15 lines); easy to review; mirrors existing pattern
**Cons:** Non-identity-partitioned upsert/overwrite loses #3011 perf
(reverts to 0.11.1 speed)
### Option B: Fix the Predicate (Better long-term)
Rework `Transaction._build_partition_predicate()` to express predicates
directly in partition-field space (`ts_day = 20455`) instead of aliasing onto
source columns. No type confusion, correct for all transforms, keeps the #3011
performance win everywhere.
**Pros:** Correct and fast for all transforms
**Cons:** Deeper change (~50 lines), more scrutiny needed to ensure
`_DeleteFiles` row-level filters aren't regressed
--
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]
