morningman opened a new pull request, #69711:
URL: https://github.com/apache/doris/pull/69711
### What problem does this PR solve?
Issue Number: None
Related PR: #69536, #68267
Problem Summary:
**In short.** FileScannerV2 now compares an Iceberg equality-delete key in
the wider of the delete file's and the data file's own key types, instead of in
the one table type the FE sends for the whole scan. For some table histories
that single type is wrong: the reader then deletes rows it must keep, keeps
rows it must delete, or fails the scan. This follows the review of #69536,
where these cases came up.
**Background.** An equality delete removes a data row whose key values equal
a tombstone's. After a type promotion the two sides can be stored in different
types: tombstones written before an INT -> LONG promotion hold INT values that
must still match LONG data. #68267 made FileScannerV2 compare in the current
schema's type, and for a key that the current schema no longer has, the FE
sends a type recovered from the retained schemas (#69536). The reader casts
both the tombstones and the data keys to that one type.
**The problem, and what it cost.** One type per scan cannot describe every
delete file:
| History | Table type from the FE | Delete file | Data file | Before |
After |
|---|---|---|---|---|---|
| `expireSnapshots().cleanExpiredMetadata(true)` removed the schema of a
schema-only INT -> LONG promotion of a dropped key | INT | LONG 4294967296 |
INT, one NULL key | the tombstone becomes NULL in INT and deletes the NULL-key
row | row kept |
| A read pinned before a later DATE -> TIMESTAMP_NS promotion of the key |
TIMESTAMP_NS | DATE 1000-01-01 | DATE 1000-01-01 | scan fails: `TIMESTAMP_NS
overflow when casting row ...` | row deleted |
| No retained schema gives the key a concrete type, so the FE sends Iceberg
`unknown` as STRING | STRING | TIMESTAMP_NS 2024-01-02 00:00 | DATE 2024-01-02
| `2024-01-02` != `2024-01-02 00:00:00.000000000`, the row stays visible | row
deleted |
| The delete was prepared under a TIMESTAMP_NS schema that the table later
abandoned for TIMESTAMP | TIMESTAMP | TIMESTAMP_NS 12:00:00.000000500 |
TIMESTAMP 12:00:00.000000 | the tombstone is rounded to microseconds and
deletes the row | row kept |
Three of the four return wrong rows without any error. The V1 reader is not
changed by this PR.
**How this PR fixes it.**
- The delete block keeps the delete file's own key types. When the predicate
for a data file is built, each key is compared in the wider of the delete
file's type and the data file's type (`equality_delete_comparison_type`). The
accepted widenings are the Iceberg promotions (INT -> LONG, FLOAT -> DOUBLE,
decimal precision, DATE -> TIMESTAMP and DATE -> TIMESTAMP_NS), a wider
fractional scale of the same date-time kind, wider integers, STRING ->
VARBINARY (an Iceberg BINARY key decodes as either one, depending on
`enable_mapping_varbinary`), and TIMESTAMP -> TIMESTAMP_NS. The last one is not
an Iceberg promotion; it is accepted for the abandoned-schema case above,
because scaling microseconds to nanoseconds is exact. Two types that convert in
neither direction fail the scan instead of being compared in a third type.
- The data keys are cast to that type. When it is the data file's wider
type, the tombstones are widened once per scanner, and the converted delete
block and its hash index are cached next to the original
(`_convert_equality_delete_filter`). A table without type changes is compared
as before, with no cast and no copy.
- A key that the data file lacks is compared through its initial default, in
the wider of the delete file's type and the table type. Without an initial
default the key is NULL in that file whatever its table type, so it is compared
in the delete file's type (an `unknown` key arrives as STRING).
- The comparison casts are strict whatever the session's cast mode, through
a new `strict` flag on `EvolutionCast`: a non-strict overflow would turn a key
into NULL and let a NULL tombstone delete it. DATE -> TIMESTAMP_NS was already
strict; other `EvolutionCast` users are unchanged.
- The table types from the FE still resolve table fields and the defaults of
missing keys; the delete-file cache key is unchanged.
**Results.**
- 9 new BE unit tests in `iceberg_reader_test.cpp`: the four histories
above; INT tombstones against LONG, INT and LONG data files of one scan (the
third split reuses the cached widened copy); a missing key's INT default
against a LONG tombstone; a missing key without a default under an `unknown`
table type; a microsecond key outside the nanosecond range, which must fail;
and incomparable key types. With the reader before this PR, 6 of them fail; the
other three pin behavior the old code shares. Removing the TIMESTAMP ->
TIMESTAMP_NS widening fails 2, and making the casts non-strict fails 1.
- End to end on a local cluster: a v2 table in a `file://` Hadoop catalog,
built with the Iceberg 1.12.0 Java API: `k INT` with rows (1, NULL), (2, 0),
(3, 1); `k` promoted to LONG without a snapshot; a delete of `k = 4294967296`
written under that schema; `k` dropped; the delete committed; then
`expireSnapshots().retainLast(2).cleanExpiredMetadata(true)`, which leaves
schema 0 (`k INT`) and schema 2 (no `k`). `SELECT id` through FileScannerV2
returned 2, 3 before this PR and returns 1, 2, 3 with it.
branch-4.1 and branch-4.2 have the same FileScannerV2 code.
### Release note
Fix FileScannerV2 comparing Iceberg equality-delete keys in a single
scan-wide type, which could delete or keep the wrong rows, or fail the scan,
after a key's type changed (for example a schema-only promotion removed by
`cleanExpiredMetadata`, or a key whose retained schemas only have it as
`unknown`).
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- `run-be-ut.sh --run
--filter='IcebergV2ReaderTest.*:*EqualityDelete*:*ColumnMapper*:*EvolutionCast*'`:
273 of 273 pass on macOS arm64 (ASAN), including the 9 new cases.
- [x] Manual test (add detailed scripts or steps below)
- The local end-to-end check described under Results, with an ASAN
BE built from this change.
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes. FileScannerV2 compares equality-delete keys per delete file
and data file; two key types that convert in neither direction now fail the
scan; the comparison casts are strict.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
