LuciferYang opened a new pull request, #857:
URL: https://github.com/apache/iceberg-cpp/pull/857
## What
`internal::CanContainEqDeletesForFile` crashes with
`std::bad_optional_access` when an equality delete file has lower/upper bounds
for some of its equality fields but not others.
`EqualityDeleteFile::LowerBound`/`UpperBound` return
`Result<std::optional<...>>` (i.e. `std::expected<std::optional<...>, Error>`),
and a field with no bound yields an engaged `expected` wrapping a disengaged
`optional`. The guard checked `std::expected::has_value()`, which only reports
the error state and does not detect the empty inner `optional`, so a missing
bound slipped past and `->value()` threw at the range check.
The throw is an uncaught exception escaping the `Result`-based call chain,
so it terminates the caller instead of surfacing as an error. It is reachable
during scan planning (`ManifestGroup` → `DeleteFileIndex::ForEntry`) and commit
validation (`MergingSnapshotUpdate::ValidateNoNewDeletesForDataFiles` →
`ForDataFile`). 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, so this is reachable in normal reader use.
Fixes #856.
## How
Unwrap the `expected` with `ICEBERG_ASSIGN_OR_RAISE` before the guard, so
`has_value()` tests the inner `optional`. This matches the data-side guard
already used a few lines above in the same function, and matches Java's
`DeleteFileIndex.canContainEqDeletesForFile`, which guards all four bounds
uniformly and assumes may-match on any missing bound (it never prunes on
missing statistics).
As a side effect this propagates a genuine bound-deserialization error
upward instead of silently treating it as may-match. That is consistent with
the data-side `Literal::Deserialize` calls in the same function, which already
propagate via `ICEBERG_ASSIGN_OR_RAISE`.
## Testing
Added `DeleteFileIndexTest.TestEqualityDeletePartialFieldBounds`: an
equality delete over fields `{1, 2}` with bounds for field 1 only, evaluated
against a data file that has bounds for both (field 1's range overlapping so
evaluation reaches field 2). Verified fail-without (throws
`std::bad_optional_access`) / pass-with (returns the delete file as may-match).
`null_value_counts` is pinned to 0 for both fields so the null short-circuits
cannot mask the range-check path if the schema fields later change nullability.
Full `manifest_test` passes.
--
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]
