jackMintao opened a new issue, #17915:
URL: https://github.com/apache/iceberg/issues/17915
### Apache Iceberg version
1.10.0
### Query engine
None
### Please describe the bug 🐞
### Apache Iceberg version
1.10.0, 1.11.0 (latest as of filing; still present on `main`)
### Latest installation method and version
Maven / Gradle build from source (`main`), also verified on released 1.10.x
and 1.11.0
### Bug Description
`compute_partition_stats` in incremental mode merges the previous stats file
(base) with a
delta computed from newly added manifests. The merge map (`PartitionMap`)
compares partition
tuples using **each spec's own partition type**, while the two sides use
different layouts:
| Side | Key layout |
|---|---|
| Base rows from the stats file | Unified partition type: union of **all
historical specs'** fields, ordered by field ID |
| Delta rows from manifests | The manifest's spec partition type: only that
spec's fields |
When the current spec's field IDs are not a **prefix** of the unified field
list — which
happens after dropping a non-last partition field, or replacing one (e.g.
changing a bucket
count, which drops the old field and adds a new one with a higher field ID)
— the per-spec
comparator misaligns positions and two failure modes appear:
1. **Partition row collapse.** All base rows sharing the same leading
partition value compare
"equal" (the misaligned slot is null on every row), so loading the base
file keeps only the
last row per leading value. E.g. a `dt + bucket(8, user_id)` table ends
up with **1 row per
`dt` instead of 8**; the other buckets silently vanish. The collapsed
file becomes the next
round's base, so the error propagates.
2. **Negative counters.** Deleted-file delta rows can no longer match their
base row, so the
decrement is written as a standalone row, e.g. `data_file_count = -1`,
`data_record_count = -1` (we observed `-100615` in production).
Subsequent rounds compound it.
Both corrupt a file that downstream consumers (partition expiry / retention)
trust: partitions
missing from stats never expire, and negative counters break "partition is
empty" checks.
The defect was introduced by #12629 (incremental compute), first released in
**1.10.0**.
### Apache Iceberg version
1.10.0, 1.11.0 (latest as of filing; still present on `main`)
### Latest installation method and version
Maven / Gradle build from source (`main`), also verified on released 1.10.x
and 1.11.0
### Bug Description
`compute_partition_stats` in incremental mode merges the previous stats file
(base) with a
delta computed from newly added manifests. The merge map (`PartitionMap`)
compares partition
tuples using **each spec's own partition type**, while the two sides use
different layouts:
| Side | Key layout |
|---|---|
| Base rows from the stats file | Unified partition type: union of **all
historical specs'** fields, ordered by field ID |
| Delta rows from manifests | The manifest's spec partition type: only that
spec's fields |
When the current spec's field IDs are not a **prefix** of the unified field
list — which
happens after dropping a non-last partition field, or replacing one (e.g.
changing a bucket
count, which drops the old field and adds a new one with a higher field ID)
— the per-spec
comparator misaligns positions and two failure modes appear:
1. **Partition row collapse.** All base rows sharing the same leading
partition value compare
"equal" (the misaligned slot is null on every row), so loading the base
file keeps only the
last row per leading value. E.g. a `dt + bucket(8, user_id)` table ends
up with **1 row per
`dt` instead of 8**; the other buckets silently vanish. The collapsed
file becomes the next
round's base, so the error propagates.
2. **Negative counters.** Deleted-file delta rows can no longer match their
base row, so the
decrement is written as a standalone row, e.g. `data_file_count = -1`,
`data_record_count = -1` (we observed `-100615` in production).
Subsequent rounds compound it.
Both corrupt a file that downstream consumers (partition expiry / retention)
trust: partitions
missing from stats never expire, and negative counters break "partition is
empty" checks.
The defect was introduced by #12629 (incremental compute), first released in
**1.10.0**.
### Reproduction (Spark SQL)
```sql
CREATE TABLE t (dt string, user_id int, data string)
USING iceberg PARTITIONED BY (dt, bucket(4, user_id));
-- round 1: full compute (no base file) -> correct
INSERT INTO t SELECT '2024-01-01', id, 'a' FROM range(1, 40);
CALL catalog.system.compute_partition_stats('default.t');
-- spec evolution: bucket(4) -> bucket(8)
ALTER TABLE t DROP PARTITION FIELD bucket(4, user_id);
ALTER TABLE t ADD PARTITION FIELD bucket(8, user_id);
-- round 2: incremental; dt2 rows are new, this round is still correct
INSERT INTO t SELECT '2024-01-02', id, 'b' FROM range(41, 80);
CALL catalog.system.compute_partition_stats('default.t');
-- round 3: incremental; loading the round-2 base collapses dt2's 8 bucket
rows into 1
INSERT INTO t SELECT '2024-01-03', id, 'c' FROM range(81, 120);
CALL catalog.system.compute_partition_stats('default.t');
SELECT partition, spec_id, file_count, record_count
FROM default.t.partitions WHERE partition.dt = '2024-01-02';
-- expected: 8 rows (one per bucket); actual on unfixed code: 1 row
```
A negative-counter variant (drop the first partition field, insert, compute,
delete the row,
compute) yields a [null, b] row with file_count = -1, record_count = -1.
Expected Behavior
Incremental compute must merge delta rows onto their base rows regardless of
spec evolution:
bucket rows should survive one-per-bucket, and counters should never go
negative. If a merge
result does contain negative counters, the incremental compute should fail
(falling back to a
full compute) rather than write a corrupt stats file.
### Root cause
computeAndMergeStatsIncremental builds PartitionMap.create(table.specs());
each spec's
StructLikeMap is created from spec.partitionType() and compares tuples
positionally with
that spec's arity. Base keys are deserialized in the unified layout while
delta keys are
spec-typed (file.partition() via partitionDataToRecord). For a spec whose
fields sit at
unified positions with a gap (e.g. positions 0 and 2 after position 1 was
dropped), the
comparator reads positions 0..1 — position 1 is the dropped field's slot,
null on every row of
this spec — collapsing rows and unmatched deltas as described above.
### Proposed fix
I have a working fix and would like to contribute it as a PR:
- Key the merge map per spec ID with
StructLikeMap.create(unifiedPartitionType) instead of
PartitionMap, so every key is compared in the unified layout the stats file
actually uses.
- Coerce delta keys with PartitionUtil.coercePartition before merging, so
delete/overwrite
deltas always land on their base row.
- Fail incremental compute with InvalidStatsFileException when merged
counters go negative,
so callers fall back to full compute instead of writing a corrupt file.
Verified with new regression tests covering spec evolution + delete,
multi-round propagation,
stale/wrong-base fallbacks, and parquet/Spark end-to-end paths; 6 of them
fail on unfixed code
and all pass with the fix, with existing partition-stats suites green.
Affected tables
A table is affected iff some spec with rows in the stats file has a field-ID
list that is not a
prefix of the unified field-ID list: dropping a middle/front partition
field, or changing a
bucket count / transform. Never-evolved tables, pure field additions, and
dropping the last
field are safe. v1 tables (void placeholders for dropped fields) are
naturally safe.
### Willingness to contribute
- [ ] I can contribute a fix for this bug independently
- [x] I would be willing to contribute a fix for this bug with guidance from
the Iceberg community
- [ ] I cannot contribute a fix for this bug at this time
--
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]
