morningman opened a new pull request, #70093:
URL: https://github.com/apache/doris/pull/70093
### What problem does this PR solve?
Issue Number: None
Related PR: #69723, #69821
Problem Summary:
#### In short
Since #69723 removed the V1 file scanner, FileScannerV2 also reads the plans
of an FE that predates the Iceberg scan semantics versions (Doris 4.1.3 and
earlier), which a BE upgraded first executes during a rolling upgrade. For such
a plan, the V2 Iceberg reader ignored an equality delete keyed on a nested
field of a data file without field ids, or made it delete every row of the
file, whenever the query did not read the field's parent struct. Neither case
raised an error. This PR resolves such a key by its full path in the delete
file. It also adds the V2 tests for old-FE plans whose equality keys are
missing from the data file, which #69821 covered on the V1 reader before V1 was
removed.
#### Background
An FE of 4.1.3 or earlier plans an Iceberg scan without
`iceberg_scan_semantics_version` and without initial defaults. Its schema
metadata (`current_schema_id = -1`, a single schema `-1`) lists only the
projected columns, or every column when Top-N lazy materialization needs them,
and marks every field optional. An equality-delete key is a hidden dependency
of the scan, so the old FE usually does not list it.
The V2 reader resolves an equality key through that schema metadata. For a
key that an old FE does not list, it falls back to the name the delete file
gives the key (`IcebergTableReader::_find_equality_delete_data_field`). In a
data file with field ids the key is found by its field id at any depth. In a
data file without them, as in an Iceberg table created by `migrate` or
`add_files`, it is looked up by name.
#### The problem, and what it cost
The fallback kept only the key's leaf name and looked it up among the data
file's top-level columns. A nested key was therefore never found and was read
as a missing key: NULL in every row of the file.
Take an ID-less data file `id, payload STRUCT<k INT>` holding `(1, {5})`,
`(2, {7})` and `(3, NULL)`, an equality delete on `payload.k`, and an old-FE
scan that reads only `id`:
| Tombstones on `payload.k` | Iceberg result | Before this PR | This PR |
|---|---|---|---|
| `7` | 1, 3 | 1, 2, 3 (row 2 comes back) | 1, 3 |
| `7`, `NULL` | 1 | none (every row is deleted) | 1 |
Parquet and ORC behave the same. A data file with field ids, or a scan whose
schema lists `payload`, was already read correctly. Before #69723 the V1 reader
failed this plan (`Missing Iceberg schema metadata for equality-delete field
id`), and a 4.1.3 BE refuses equality deletes on data files without field ids,
so during an upgrade this was an error before it became wrong rows. It takes an
old FE in front of an upgraded BE, data files without field ids, an equality
delete keyed on a nested struct field, and a query that does not read that
struct. branch-4.2 reads 4.1.3 plans with the same V2 code.
#### How this PR fixes it
`EqualityDeleteFilter` now keeps, for each key, the delete file's names from
the key's top-level column down to the key (`field_name_paths`, for example
`{"payload", "k"}`) instead of the leaf name alone. The old-FE fallback builds
a table path from these names, so the walk by name finds `payload` and then
`k`, with the same matcher as before. A present parent without the leaf is
still a missing key and reads as NULL. The retry with the delete file's leaf
name, and the fallback field that builds a missing key's NULL, use the last
name of the path, as before.
One limit remains: a key renamed after a data file without field ids was
written is not found under the delete file's name, because the old FE sends no
name mapping for a column it does not list. Such a key reads as NULL, as a
top-level key already does.
#### Results
New cases in `be/test/format_v2/table/iceberg_reader_test.cpp`, all on scan
parameters built the way a 4.1.3 FE builds them
(`make_old_fe_iceberg_scan_params`):
| Test | Cases | Before this PR | This PR |
|---|---|---|---|
| `IcebergOldFePlanFindsUnlistedNestedEqualityKeyByDeleteFileNames`: the
example above, with and without field ids, with and without `payload` in the
schema | 16 | 4 fail: no field ids and `payload` not listed, Parquet and ORC,
both tombstone sets | all pass |
| `IcebergOldFePlanMatchesMissingEqualityKeyAsNull`: an INT or STRING key
the file lacks, alone or with `id`, against NULL and `0` / `''` tombstones | 24
| all pass | all pass |
| `IcebergOldFePlanMatchesMissingNestedEqualityKeyAsNull`: `payload.k` the
file lacks, under NULL and present parents | 8 | all pass | all pass |
The last two pin what V2 already did for a key the file lacks: it is NULL in
every row and matches only a NULL tombstone, never the `0` or empty string
stored under a NULL slot.
### Release note
Fix equality deletes on a nested field of Iceberg data files without field
ids being ignored, or deleting every row of the file, when a BE runs a query
planned by an FE of 4.1.3 or earlier during a rolling upgrade.
### Check List (For Author)
- Test: Unit Test
- BE UT: the three tests above. Without the two source changes only the
4 cases listed fail; with them all pass. `IcebergV2ReaderTest`,
`IcebergTableReaderTest`, `EqualityDeletePredicateTest`,
`IcebergPositionDeleteSysTableV2*`, `TableReaderTest`, `ColumnMapper*`,
`LocalColumnIndexTest` and `CloneTableExprTreeTest` pass locally (411 tests,
macOS, ASAN). `build-support/check-build-hygiene.sh` passes, and clang-tidy
reports nothing on the changed lines.
- No regression test: it needs an FE of 4.1.3 or earlier in front of
this BE, which the regression framework cannot set up.
- Behavior changed: Yes. For such a plan, a nested equality key that a data
file without field ids stores is read from the file, so its tombstones delete
the rows Iceberg deletes.
- Does this need documentation: No
--
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]
