morningman opened a new pull request, #69547:
URL: https://github.com/apache/doris/pull/69547
### What problem does this PR solve?
Issue Number: None
Related PR: #66538
Problem Summary:
**In short.** This ports a fix from branch-4.1 #66538 (Iceberg V3 default
values) to the V1 Parquet reader on master. When an Iceberg equality delete is
keyed on a struct field such as `payload.existing`, and the query also projects
`payload` (any `SELECT *`), or the delete has several keys in the same struct,
master reads only one of the columns that map to the struct, and returns an
empty struct or no rows.
**Background.** The V1 Parquet reader reads a nested equality key through
its top-level struct. A hidden carrier column named
`__equality_delete_column__<id>_<leaf>` maps to the physical root (`payload`),
the whole struct is read into it, and the key's leaf is extracted after the
read. `ParquetReader::_init_read_columns` builds a map from each file column to
one table column.
**The problem, and what it cost.** When two table columns name the same file
column, the later one replaces the earlier one in that map, and only one of
them gets a column reader. Two cases do this: a projected `payload` next to the
carrier of `payload.existing`, and the carriers of two keys under one root,
such as `payload.a` and `payload.b`. Reproduced with BE unit tests on a file
holding (1,{10}), (2,{20}), (3,{30}) and an equality delete on
`payload.existing = 20`:
| Query projects | Master today | After this PR |
|---|---|---|
| `id, payload` | `payload` is empty, and the block has fewer `payload` rows
than `id` rows (a DCHECK in `vparquet_group_reader.cpp` under ASAN) | (1,{10}),
(3,{30}) |
| `payload, id` | no rows | (1,{10}), (3,{30}) |
| `id`, with keys `payload.existing` and `payload.k` | the carrier of
`payload.existing` is never read | rows 1 and 3 |
ORC reads of a projected root struct are not affected: the ORC cases below
pass without this change.
**How this PR fixes it.** It follows branch-4.1 / branch-4.2 (the Parquet
carrier setup in `be/src/format/table/iceberg_reader.cpp` and
`ParquetReader::set_duplicate_file_column_aliases`):
- When the query projects the key's root struct, the file has the key's full
path and the projected struct holds the key with the key's type, the key is
taken from the projected column. Nothing extra is read.
- Otherwise the first key under a physical root reads the root, and every
later key under that root is extracted from that carrier's column.
- A carrier that reads nothing gets a placeholder of the batch row count, so
the reader filters it with the other columns, and the leaf replaces it after
the read. All leaves are extracted before any carrier column is replaced,
because one carrier can be the source of other keys.
- `ParquetReader` reads a file column once for each table column that names
it, and prefetches it once. This covers a key that the projected struct lacks,
for example a key dropped from the table, whose carrier reads the same root as
the projected `payload`.
Two differences from branch-4.x, both on the safe side: a key whose path is
incomplete in the file (an older file written before the key existed) keeps
reading the physical root instead of relying on the projected struct for the
missing child, and a key whose type differs from the projected type (a key
written before a type promotion) also keeps reading the physical root.
Branch-4.x casts that case; master does not cast equality keys yet, which is a
separate fix.
**Results.** All 50 tests of `IcebergReaderTest` pass. Without this change,
each of the five Parquet cases below aborts on the DCHECK `block->rows() ==
col.column->size()` in `vparquet_group_reader.cpp`, because one of the columns
that share the root is never read:
- Four cases from #66538 for a projected root struct, Parquet and ORC, with
`payload` listed before and after `id`. Without this change the Parquet cases
leave `payload` unread in both orders; the ORC cases already pass.
- `v1_parquet_equality_keys_share_unprojected_root`: two keys under an
unprojected root share one carrier. Without this change the carrier of
`payload.existing` is never read.
- `v1_parquet_equality_keys_come_from_projected_root`: two keys are taken
from the projected root. Without this change `payload` is never read.
- `v1_parquet_equality_key_outside_projected_struct_rereads_root`: the
projected struct lacks the key, so the carrier reads the same file column
again. Without this change `payload` is never read.
Original author: @hubgeter
### Release note
Fix empty or missing rows when the V1 Parquet reader reads an Iceberg table
with an equality delete keyed on a struct field, and the query also projects
that struct or the delete has several keys in the struct.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
Such reads return the correct rows.
- 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]
