LuciferYang opened a new issue, #856:
URL: https://github.com/apache/iceberg-cpp/issues/856
**Summary**
`internal::CanContainEqDeletesForFile` (`src/iceberg/delete_file_index.cc`)
throws `std::bad_optional_access` and crashes the process when an equality
delete file carries lower/upper bounds for some of its equality fields but not
others. The throw is an uncaught exception escaping the `Result`-based
(non-throwing) call chain, so it terminates the caller rather than surfacing as
an error.
Two reachable entry points, both consuming files written by arbitrary
engines:
- Scan planning: `ManifestGroup` → `DeleteFileIndex::ForEntry`
(`manifest_group.cc:214`).
- Commit validation:
`MergingSnapshotUpdate::ValidateNoNewDeletesForDataFiles` →
`DeleteFileIndex::ForDataFile` (`merging_snapshot_update.cc:1196`/`:1256`),
reached from `RowDelta`/`OverwriteFiles`/`RewriteFiles`.
Equality delete files with bounds for only some fields are produced
legitimately by per-column metrics modes (`counts`/`none`/`truncate`) and by
cross-engine writers; `ContentFileUtil::DropUnselectedStats` drops non-equality
column stats without back-filling missing bounds, so the asymmetry is preserved
on read. The crash is reachable in normal reader use, not just a synthetic
input.
**Root Cause**
`EqualityDeleteFile::LowerBound`/`UpperBound` return
`Result<std::optional<std::reference_wrapper<const Literal>>>`, i.e.
`std::expected<std::optional<...>, Error>`. A field that has no bound yields an
*engaged* `expected` wrapping a *disengaged* `optional`. The guard checks the
wrong layer:
```cpp
auto delete_lower = delete_file.LowerBound(field_id); //
expected<optional<...>>
auto delete_upper = delete_file.UpperBound(field_id);
if (!delete_lower.has_value() || !delete_upper.has_value()) { //
expected::has_value(): only the error state
continue; // Missing bounds, assume may match
}
...
RangesOverlap(..., delete_lower->value().get(),
delete_upper->value().get()); // optional::value() on nullopt -> throws
```
`has_value()` here calls `std::expected::has_value()`, which only reports
whether an error is present; it does not detect the empty inner `optional`. So
a missing bound passes the guard and `->value()` throws. The data-side guard a
few lines above (`data_file.lower_bounds.find(...) == cend() ||
...->second.empty()`) checks field-level presence correctly, so the two sides
are asymmetric. Java's `DeleteFileIndex.canContainEqDeletesForFile` (checked
against apache/iceberg `main`) guards all four bounds uniformly (`dataLower ==
null || dataUpper == null || deleteLower == null || deleteUpper == null` →
`continue`, "assume the delete file may match"), and only prunes when all four
are present and the ranges do not overlap.
**Reproduction**
An equality delete over fields `{1, 2}` whose `lower_bounds`/`upper_bounds`
contain only field 1, evaluated against a data file that has bounds for both
fields (field 1's range overlapping so evaluation proceeds to field 2), throws
`std::bad_optional_access` out of `EqualityDeletes::Filter`.
**Proposed Fix**
Unwrap the `expected` first with `ICEBERG_ASSIGN_OR_RAISE` so `has_value()`
tests the inner `optional`, matching the data-side guard and Java:
```cpp
ICEBERG_ASSIGN_OR_RAISE(auto delete_lower, delete_file.LowerBound(field_id));
ICEBERG_ASSIGN_OR_RAISE(auto delete_upper, delete_file.UpperBound(field_id));
if (!delete_lower.has_value() || !delete_upper.has_value()) {
continue;
}
...
RangesOverlap(..., delete_lower->get(), delete_upper->get());
```
This also propagates a genuine bound-deserialization error instead of
silently swallowing it as "may match", consistent with the data-side handling
in the same function.
I have a fix and a regression test ready and will open a PR.
--
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]
