You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "paulcaron16k (via GitHub)" <gi...@apache.org> on 2026/08/06 21:23:52 UTC
[I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
paulcaron16k opened a new issue, #3758:
URL: https://github.com/apache/iceberg-python/issues/3758
### Apache Iceberg version
main (reproduced at `154288fb`; the affected files are byte-identical at `7d0f5031`, which is `version = "0.12.0"`)
### Please describe the bug π
On a **partitioned** table, `upsert` leaves the row it replaced *and* duplicates a row it never touched. No error is raised.
`0.11.1` is correct. An unpartitioned table is correct on both, so the partition spec is required to reproduce.
## Reproduction
```python
import tempfile, pathlib, datetime as dt
import pyarrow as pa
from pyiceberg.catalog.sql import SqlCatalog
from pyiceberg.schema import Schema
from pyiceberg.partitioning import PartitionField, PartitionSpec
from pyiceberg.transforms import DayTransform
from pyiceberg.types import NestedField, StringType, IntegerType, TimestampType
wh = pathlib.Path(tempfile.mkdtemp())
cat = SqlCatalog("c", uri=f"sqlite:///{wh}/c.db", warehouse=f"file://{wh}")
cat.create_namespace("db")
schema = Schema(
NestedField(1, "k", StringType(), required=False),
NestedField(2, "v", IntegerType(), required=False),
NestedField(3, "ts", TimestampType(), required=False),
)
spec = PartitionSpec(
PartitionField(source_id=3, field_id=1000, transform=DayTransform(), name="ts_day")
)
t = cat.create_table("db.t", schema=schema, partition_spec=spec,
properties={"format-version": "2"})
arrow = pa.schema([pa.field("k", pa.string()), pa.field("v", pa.int32()),
pa.field("ts", pa.timestamp("us"))])
base = dt.datetime(2026, 1, 6, 12)
def rows(pairs):
return pa.table({"k": [p[0] for p in pairs],
"v": pa.array([p[1] for p in pairs], type=pa.int32()),
"ts": [base] * len(pairs)}, schema=arrow)
t.append(rows([("a", 1), ("b", 1)]))
t.refresh()
t.upsert(rows([("a", 2)]), join_cols=["k"])
t.refresh()
arr = t.scan().to_arrow()
print(sorted(zip(arr["k"].to_pylist(), arr["v"].to_pylist())))
print("data files:", len(t.inspect.files().to_pylist()))
```
| version | output | data files |
|---|---|---|
| 0.11.1 | `[('a', 2), ('b', 1)]` β correct | 2 |
| main | `[('a', 1), ('a', 2), ('b', 1), ('b', 1)]` | 3 |
Two problems in one result: the superseded `('a', 1)` survives beside its replacement, and `('b', 1)` β untouched by the upsert β is duplicated.
## Suspected cause
`_OverwriteFiles._existing_manifests` keeps a manifest its evaluator does not match, whole:
```python
manifest_evaluators: dict[int, Callable[[ManifestFile], bool]] = KeyDefaultDict(self._build_manifest_evaluator)
...
if not manifest_evaluators[manifest_file.partition_spec_id](manifest_file):
existing_files.append(manifest_file)
continue
```
The predicate comes from `_build_delete_files_partition_predicate`, built from the deleted files' partition values. For this upsert the join column is `k`, which is not a partition field, so the resulting predicate cannot select the manifest holding the rows being replaced. That manifest is appended intact, while the rewritten file and the new row are added alongside it, so a reader sees both copies.
I have not tried to determine whether the right fix is to widen the predicate, to fall back to a full scan when the predicate cannot be projected onto a spec, or to match deleted entries directly β happy to test a patch.
## Knock-on effect
The symptom most users will hit first is the *next* upsert, which fails in `upsert_util.get_rows_to_update`:
```
ValueError: Target table has duplicate rows, aborting upsert
```
That guard is doing its job. It is detecting real duplicates β just ones the previous `upsert` created.
## Why this may be worth looking at before 0.12
`version` on main is already `0.12.0`, and the failure is silent: a merge-style ingest into a partitioned table accumulates duplicate rows with no error until a later upsert happens to trip the duplicate check.
### Willingness to contribute
Would be interested in contributing a fix to Apache Iceberg.
--
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: issues-unsubscribe@iceberg.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "amitpoorab (via GitHub)" <gi...@apache.org>.
amitpoorab commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5244948132
I've opened a PR to fix this: #3775
The fix restricts the manifest pruning optimization to identity-transform-only specs,
preventing the false negatives that were causing data corruption on temporal-partitioned tables.
Added parametrized regression tests covering all transform types. Ready for review.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5337964757
this issue is resolved by https://github.com/apache/iceberg-python/pull/3320/changes#diff-23e8153e0fd497a9212215bd2067068f3b56fa071770c7ef326db3d3d03cee9bR833
a regression test was added in https://github.com/apache/iceberg-python/pull/3780/changes#diff-0a8709ee278f0979ecdcff40f5ff7e215076867112c820b633e3e6f17c827a2eR720
Thank you @paulcaron16k for reporting this issue. And big thanks to everyone for working on it!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu closed issue #3758: upsert duplicates rows on a partitioned table (regression vs 0.11.1)
URL: https://github.com/apache/iceberg-python/issues/3758
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "amitpoorab (via GitHub)" <gi...@apache.org>.
amitpoorab commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5242770243
I have first change ready, will send PR soon.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "gabeiglio (via GitHub)" <gi...@apache.org>.
gabeiglio commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5254664882
Hi thanks for reporting this issue. The root cause (as correctly identified) is that manifest partition values already hold partitioned values and the _build_partition_predicate method I introduced referenced it back to the source column which made the value be transformed a second time which evaluated to 0 and skipped the manifest being replaced when pruning the manifests.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5338109727
last thing to do:
Since #3780 was opened, #3320 added commit retries. For file-level overwrites, a retry may not detect that another writer deleted the file being replaced, potentially reintroducing deleted data. Iβll add file-existence validation and regression coverage in a follow-up (#3811).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "paulcaron16k (via GitHub)" <gi...@apache.org>.
paulcaron16k commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5211634083
Retested against `7d0f5031` (`version = "0.12.0"`). Still reproduces, and I was able to narrow it usefully.
## It is specific to temporal transforms
Same script, only the partition transform changed. `append([("a",1),("b",1)])` then `upsert([("a",2)])`:
| partition spec | result |
|---|---|
| `identity(ts)` | β
`[('a', 2), ('b', 1)]` |
| `truncate(k, 1)` | β
`[('a', 2), ('b', 1)]` |
| `bucket(k, 4)` | β
`[('a', 2), ('b', 1)]` |
| `year(ts)` | β `[('a', 1), ('a', 2), ('b', 1), ('b', 1)]` |
| `month(ts)` | β same |
| `day(ts)` | β same |
| `hour(ts)` | β same |
So it is not "partitioned" in general, and not "non-identity" either β `truncate` and `bucket` are both fine. Only the four temporal transforms fail, which are the ones whose partition value is an integer ordinal derived from a timestamp source column.
That fits the predicate I mentioned above. Traced during a run partitioned by `day(ts)`:
```
Or(EqualTo(term=Reference(name='ts'), literal=LongLiteral(20455)), ...)
```
The *source column* (`ts`, a timestamp) compared against a *partition ordinal* (`20455`, days since epoch). For `identity` the two coincide, and for `truncate`/`bucket` the projection is evidently still usable; for the temporal transforms the resulting predicate cannot select the manifest, so `_existing_manifests` keeps it whole.
## Minimum condition
One manifest containing **both** a row being replaced and a row that is not.
- append 1 row, upsert it β β
correct. Every entry in the manifest is deleted, so the manifest is dropped entirely and the stale path is never taken.
- append 2 rows, upsert 1 β β one entry survives, so the manifest has to be rewritten, the evaluator does not match, and it is appended verbatim with **both** entries β beside the newly written ones.
## Minimal reproduction
```python
import datetime as dt
import tempfile
import pyarrow as pa
from pyiceberg.catalog.sql import SqlCatalog
from pyiceberg.partitioning import PartitionField, PartitionSpec
from pyiceberg.schema import Schema
from pyiceberg.transforms import DayTransform
from pyiceberg.types import IntegerType, NestedField, StringType, TimestampType
warehouse = tempfile.mkdtemp()
catalog = SqlCatalog("c", uri=f"sqlite:///{warehouse}/c.db", warehouse=f"file://{warehouse}")
catalog.create_namespace("db")
table = catalog.create_table(
"db.t",
schema=Schema(
NestedField(1, "k", StringType(), required=False),
NestedField(2, "v", IntegerType(), required=False),
NestedField(3, "ts", TimestampType(), required=False),
),
partition_spec=PartitionSpec(PartitionField(3, 1000, DayTransform(), "p")),
properties={"format-version": "2"},
)
schema = pa.schema(
[pa.field("k", pa.string()), pa.field("v", pa.int32()), pa.field("ts", pa.timestamp("us"))]
)
when = dt.datetime(2026, 1, 6, 12)
def batch(pairs):
return pa.table(
{
"k": [k for k, _ in pairs],
"v": pa.array([v for _, v in pairs], type=pa.int32()),
"ts": [when] * len(pairs),
},
schema=schema,
)
table.append(batch([("a", 1), ("b", 1)])) # one manifest, two rows
table.refresh()
table.upsert(batch([("a", 2)]), join_cols=["k"]) # replace one of them
table.refresh()
result = table.scan().to_arrow()
print(sorted(zip(result["k"].to_pylist(), result["v"].to_pylist())))
# 0.11.1 -> [('a', 2), ('b', 1)]
# 0.12.0 -> [('a', 1), ('a', 2), ('b', 1), ('b', 1)]
```
Happy to test a patch against these cases if that helps.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5242662919
Thanks for reporting the issue! I was able to reproduce it. There's a release candidate out for 0.12.0, lets cancel it and include a fix for this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "paulcaron16k (via GitHub)" <gi...@apache.org>.
paulcaron16k commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5242167407
Hi Amit! If you want to open a PR and discuss the options with the team (least change vs more-maintainable) that is fine with me. I appreciate the help.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5331741698
I was able to validate that this issue is fixed by https://github.com/apache/iceberg-python/pull/3320/changes#diff-23e8153e0fd497a9212215bd2067068f3b56fa071770c7ef326db3d3d03cee9bR833
Tested on these:
- 0.11.1 -> β
works as intended
- commit this issue references (`154288fb7`) -> β failed
- current `main` (c86eb8e5c) -> β
works as intended
Here's the script I used to repro:
<details>
```
uv run python - <<'PY'
import datetime as dt
import tempfile
import pyarrow as pa
from pyiceberg.catalog.sql import SqlCatalog
from pyiceberg.partitioning import PartitionField, PartitionSpec
from pyiceberg.schema import Schema
from pyiceberg.transforms import DayTransform
from pyiceberg.types import IntegerType, NestedField, StringType, TimestampType
warehouse = tempfile.mkdtemp()
catalog = SqlCatalog("c", uri=f"sqlite:///{warehouse}/c.db", warehouse=f"file://{warehouse}")
catalog.create_namespace("db")
table = catalog.create_table(
"db.t",
schema=Schema(
NestedField(1, "k", StringType(), required=False),
NestedField(2, "v", IntegerType(), required=False),
NestedField(3, "ts", TimestampType(), required=False),
),
partition_spec=PartitionSpec(PartitionField(3, 1000, DayTransform(), "p")),
properties={"format-version": "2"},
)
schema = pa.schema(
[pa.field("k", pa.string()), pa.field("v", pa.int32()), pa.field("ts", pa.timestamp("us"))]
)
when = dt.datetime(2026, 1, 6, 12)
def batch(pairs):
return pa.table(
{
"k": [k for k, _ in pairs],
"v": pa.array([v for _, v in pairs], type=pa.int32()),
"ts": [when] * len(pairs),
},
schema=schema,
)
table.append(batch([("a", 1), ("b", 1)])) # one manifest, two rows
table.refresh()
table.upsert(batch([("a", 2)]), join_cols=["k"]) # replace one of them
table.refresh()
result = table.scan().to_arrow()
rows = sorted(zip(result["k"].to_pylist(), result["v"].to_pylist()))
print(rows)
assert rows == [("a", 2), ("b", 1)], rows
PY
```
</details>
I'll take a look at the related PRs
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "amitpoorab (via GitHub)" <gi...@apache.org>.
amitpoorab commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5258470563
@paulcaron16k Thank you. you changes are correct.
I have added the PR #3782 based on your changes. Please review
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "gabeiglio (via GitHub)" <gi...@apache.org>.
gabeiglio commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5308048156
Agree, I found the root fix for it in [PR](https://github.com/apache/iceberg-python/pull/3780) which allows us to also reuse the same method for delete files and overwrites which is nice. I saw other folks PR and while correct it is more larger changes than it needs to
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5269570953
Hey folks, thanks for looking into this. I want to try to summarize the issue (as I understand it):
The regression comes from manifest pruning added to the write path in #3011. During an overwrite, PyIceberg uses partition summaries to skip manifests that cannot contain any data files being replaced. This avoids opening every manifest before performing the exact `DataFile` membership check.
The code mixes two predicate domains:
- Row predicates contain source values and must be projected through the partition transform.
- Predicates built from `DataFile.partition` contain already-transformed values and should be evaluated directly against the partition schema.
For example, with `day(ts)`:
```text
ts = 2026-01-06 12:00:00
DataFile.partition = 20459
```
The row-predicate path applies the partition transform before manifest evaluation. Reusing that path for a predicate built from `DataFile.partition` applies `day()` to the already-transformed value `20459`, producing approximately `partition_day == 0`. This does not match the manifestβs actual value, `partition_day == 20459`, so the manifest is incorrectly skipped.
Because the manifest is skipped, it is carried forward unchanged and the exact file-removal check never runs. The replacement files are still added, leaving both the old and new data in the table.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5269753777
I see a couple of PRs opened already, including a revert PR.
- https://github.com/apache/iceberg-python/pull/3776
- https://github.com/apache/iceberg-python/pull/3782
- https://github.com/apache/iceberg-python/pull/3780
Lets figure out a best path forward.
I have been iterating with codex on a solution to logically separate partition filters and row filters: https://github.com/apache/iceberg-python/compare/main...kevinjqliu:iceberg-python:kevinjqliu/codex-overwrite-pruning-regression-tests?expand=1
Originally I wanted to merge the revert PR to unblock the 0.12 release. However, it looks like reverting causes lots of unnecessary changes, and fixing forward seems relatively straight forward.
Let's see if we can fix forward first, we can fallback to revert
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "amitpoorab (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "paulcaron16k (via GitHub)" <gi...@apache.org>.
paulcaron16k commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5253689906
Traced this to the root cause. Two corrections to my narrowing above, and it turns out to be broader than `upsert`.
## Root cause
`_SnapshotProducer._build_delete_files_partition_predicate` (added in #3011, commit `8a47d2b`) builds its predicate via `Transaction._build_partition_predicate`, which emits
`EqualTo(Reference(<source column>), <partition value>)`. A `DataFile` records its partition values **already transformed**, so comparing them against the source column only holds for
identity transforms.
That helper's other caller, `dynamic_partition_overwrite`, rejects non-identity transforms before calling it:
```python
for field in self.table_metadata.spec().fields:
if not isinstance(field.transform, IdentityTransform):
raise ValueError("For now dynamic overwrite does not support a table with non-identity-transform field ...")
```
The pruning path added in #3011 has no such guard.
`inclusive_projection` then applies the transform a *second* time. Traced on a `day(ts)` table:
```
predicate : EqualTo(term=Reference(name='ts'), literal=LongLiteral(20459))
projection : EqualTo(term=Reference(name='p'), literal=LongLiteral(0))
evaluator -> False partitions=[PartitionFieldSummary[..., b'\xebO\x00\x00', ...]] # 0x4feb = 20459
```
20459 is the day ordinal; re-read as microseconds it becomes day 0. The evaluator rejects the very manifest holding the file being replaced, so `_existing_manifests` appends it whole beside
the rewritten file and `_deleted_entries` records nothing.
## Correction 1 β it is not specific to temporal transforms
My table above was an artifact of file layout, not of the transforms. `truncate` and `bucket` partition on `k`, the join column, so each key landed in its own data file; deleting one
dropped a whole file and never reached the partial-rewrite path. Forcing two rows to share a partition:
| spec | rows sharing one file | result on 0.12.0 |
|---|---|---|
| `identity(ts)` | `a`, `b` same ts | correct |
| `day(ts)` | `a`, `b` same day | duplicates |
| `truncate(k,1)` | `aa`, `ab` | correct β `truncate(truncate(x))==truncate(x)`, so the double transform is a no-op |
| `bucket(k,4)` | `k0`, `k1` (both bucket 2) | **raises** |
Only identity is genuinely correct. Truncate passes by idempotence, not by design.
## Correction 2 β bucket raises rather than duplicating
```
TypeError: Cannot convert LongLiteral into string
```
from `_OverwriteFiles._deleted_entries` β `inclusive_projection` β `bind`, where a bucket ordinal cannot bind to a string column.
## Correction 3 β `upsert` is not required
`Table.delete()` reaches the same path directly. On a `day(ts)` table holding `idx` 1 and 2 in one file:
```python
tbl.delete(EqualTo("idx", 1))
tbl.scan().to_arrow()["idx"].to_pylist()
# 0.11.1 -> [2]
# 0.12.0 -> [2, 1, 2]
```
The deleted row survives *and* an untouched row is duplicated, because the stale manifest and the rewritten file are both counted. `upsert` is just the common way to reach
`_OverwriteFiles`, not the only one.
## Fix direction
`manifest_evaluator` binds its filter against `Schema(*spec.partition_type(schema).fields)` β partition field names and transform result types. That is exactly what a partition record
already holds, so the filter can be built over the partition fields directly and the projection step drops out entirely. No transform is ever reapplied, and pruning is preserved for every
spec rather than only identity ones.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "paulcaron16k (via GitHub)" <gi...@apache.org>.
paulcaron16k commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5253749099
@amitpoorab
Hi Amit. I had Claude propose a fix with guidance for most-maintainable and preserving #3011 intended performance gains. Feel free to cherry pick if this seems like the right approach.
https://github.com/paulcaron16k/iceberg-python/tree/fix/3758-confine-overwrite-manifest-pruning
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "amitpoorab (via GitHub)" <gi...@apache.org>.
amitpoorab commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5313258190
Sounds good. I have approved the #3780
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] upsert duplicates rows on a partitioned table (regression vs 0.11.1) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #3758:
URL: https://github.com/apache/iceberg-python/issues/3758#issuecomment-5331409036
thanks folks! let me try to untangle this issue and the prs. I believe this is the last issue to fix before we can release 0.12 π
my agent tells me that this issue is already fixed (accidentally) by https://github.com/apache/iceberg-python/pull/3320/changes#diff-23e8153e0fd497a9212215bd2067068f3b56fa071770c7ef326db3d3d03cee9bR833
i'll take another look
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org