bvolpato opened a new pull request, #15966:
URL: https://github.com/apache/iceberg/pull/15966
## Summary
Adds a failing test that documents a correctness bug introduced by partition
pruning in the DV validation path added in #15653 (79d4fc82c).
`MergingSnapshotProducer.validateAddedDVs()` now projects the user-supplied
`conflictDetectionFilter` onto each concurrent delete manifest's partition spec
and drops manifests whose partition range doesn't match. The filter is
caller-supplied (for Spark, `SparkPositionDeltaWrite.conflictDetectionFilter`
derives it from `queryScan.filterExpressions()`, i.e. the query's WHERE clause)
and is not required to cover every partition the current commit writes DVs into.
When a commit writes DVs across multiple partitions but supplies a filter
that only names a subset of them, a concurrent delete manifest holding a DV for
one of the un-filtered partitions can be pruned out before the per-manifest
conflict scan ever runs. `dvsByReferencedFile.containsKey(...)` is never
checked against that manifest's entries, no conflict is raised, and the commit
goes through.
The resulting table state has two DVs that reference the same data file. Any
subsequent scan then fails in `DeleteFileIndex.Builder.add`:
```java
// core/src/main/java/org/apache/iceberg/DeleteFileIndex.java:528-536
private void add(Map<String, DeleteFile> dvByPath, DeleteFile dv) {
String path = dv.referencedDataFile();
DeleteFile existingDV = dvByPath.putIfAbsent(path, dv);
if (existingDV != null) {
throw new ValidationException(
\"Can't index multiple DVs for %s: %s and %s\",
path, ContentFileUtil.dvDesc(dv),
ContentFileUtil.dvDesc(existingDV));
}
}
```
Every read, DML, and maintenance task on the affected file blows up, so the
table is effectively bricked until the duplicate DVs are manually resolved.
## Reproducer
The added test \`testConcurrentDVsMultiPartitionNarrowFilter\` sets up:
1. Table partitioned by \`bucket[16](data)\`, seeded with two data files:
\`F_P0\` in bucket 0, \`F_P2\` in bucket 2.
2. Writer A builds a \`RowDelta\` that adds DVs for both \`F_P0\` and
\`F_P2\`, with \`conflictDetectionFilter = data = 'u'\` (projects to
\`bucket(data) = 0\`).
3. Writer B concurrently commits a DV for \`F_P2\`.
4. Writer A calls \`commit()\` and must throw \`Found concurrently added DV
for ...F_P2...\`.
On current main this commit returns normally, so \`assertThatThrownBy\`
fails with \`Expecting code to raise a throwable\`. Ran locally against
\`56092bc8c\` with JDK 21:
\`\`\`
TestRowDelta > testConcurrentDVsMultiPartitionNarrowFilter() > formatVersion
= 3, branch = main FAILED
TestRowDelta > testConcurrentDVsMultiPartitionNarrowFilter() > formatVersion
= 3, branch = testBranch FAILED
TestRowDelta > testConcurrentDVsMultiPartitionNarrowFilter() > formatVersion
= 4, branch = main FAILED
TestRowDelta > testConcurrentDVsMultiPartitionNarrowFilter() > formatVersion
= 4, branch = testBranch FAILED
6 tests completed, 4 failed, 2 skipped
\`\`\`
Also verified the corruption consequence locally with a follow-on assertion
(not included in this PR to keep the test direction clean): after the buggy
commit returns, \`table.newBatchScan().useRef(branch).planFiles()\` throws
\`ValidationException: Can't index multiple DVs for ...F_P2...\`, confirming
the state is unreadable.
## Test gap in #15653
The three DV validation tests added in #15653 all treat the conflict filter
and the current commit's DV set as partition-aligned:
- \`testConcurrentDVsInDifferentPartitionsWithFilter\` — Writer A's DV is
entirely in bucket 0; concurrent DV is in bucket 2. Correct outcome: A wins.
- \`testConcurrentDVsInSamePartitionWithFilter\` — Writer A's DV and
concurrent DV both in bucket 0. Correct outcome: A fails.
- \`testDVValidationPartitionPruningManifestCount\` — micro-test of the
pruning arithmetic.
None of them cross the current commit's \`dvsByReferencedFile\` partition
set against the pruning filter, which is where the bug lives.
## Scope
This PR contains **only** the failing reproducer. It is intentionally draft
and CI is expected to fail on \`:iceberg-core:test\` until a fix lands. A
separate PR will propose the fix — probable direction is to widen the pruning
filter so it covers both the user filter and the union of partitions referenced
by \`dvsByReferencedFile\`, or to drop the pruning entirely (the pre-#15653
behavior). Happy to take direction from maintainers on which approach they
prefer before the fix PR.
## Test plan
- [x] Reproducer test fails deterministically on current \`main\`
(\`56092bc8c\`) for format versions 3 and 4, on both \`main\` and non-main
branches.
- [ ] Fix to be proposed in a follow-up PR; same test must pass after the
fix.
--
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]
