morningman opened a new pull request, #69503:
URL: https://github.com/apache/doris/pull/69503
### What problem does this PR solve?
Issue Number: None
Related PR: #66538, #68786
Problem Summary:
**In short.** With the file scanner V1, an Iceberg equality delete whose key
is a BINARY or FIXED column fails the read, and in one case aborts the BE. This
PR matches string and VARBINARY delete keys by their bytes, as branch-4.1 and
branch-4.2 do (#66538).
**Background.** Since #68786 the BE reads Iceberg BINARY and FIXED columns
as VARBINARY. V1 (`enable_file_scanner_v2 = false`) applies equality deletes in
`be/src/format/table/equality_delete.cpp`. For one key column,
`SimpleEqualityDelete` builds a `HybridSet` with `create_set` on the key type
and probes it with the data column. For several key columns,
`MultiEqualityDelete` hashes every row with `update_hashes_with_value` and
confirms a hash hit with `compare_at`. Neither handles VARBINARY: `create_set`
has no VARBINARY set, and `ColumnVarbinary` does not implement
`update_hashes_with_value`. The delete file and the data file can also carry
the same key as STRING on one side and VARBINARY on the other.
`MultiEqualityDelete` rejects that as a type change, and `SimpleEqualityDelete`
probes the set with a column of the wrong class. branch-4.1 and branch-4.2
(#66538) store, hash and compare these keys as bytes.
**The problem, and what it cost.** From the new unit tests, run against
master's `equality_delete.cpp`:
| Equality delete key | Master today (V1) | After this PR |
|---|---|---|
| one VARBINARY key | `VARBINARY predicates are not supported` | the rows
with matching bytes are deleted |
| one key, VARBINARY in the delete file and STRING in the data file |
`VARBINARY predicates are not supported` | deleted by bytes |
| one key, STRING in the delete file and VARBINARY in the data file | the BE
aborts: `Bad cast from type:doris::ColumnVarbinary to doris::ColumnStr<unsigned
int>` | deleted by bytes |
| VARBINARY key with an INT key | `Method update_hashes_with_value is not
supported for ColumnVarbinary` | deleted by bytes |
| composite key, STRING in the delete file and VARBINARY in the data file |
`Not support type change` | deleted by bytes |
| composite key whose data column is a const NULL STRING | `Method
update_hashes_with_value is not supported for Const(Nullable(String))` | the
row matching the NULL delete key is deleted |
The const column comes from
`IcebergReaderMixin::_create_missing_equality_delete_value`: when the FE does
not send Iceberg scan semantics v2 and the data file lacks the key column, the
column is filled from `create_column_const(1, Field())`.
**How this PR fixes it.**
- Keys of a string type or VARBINARY are byte keys. A single byte key goes
into a STRING `HybridSet` as bytes, and data rows probe it with their bytes. A
byte delete key against a non-byte data column is still rejected as a type
change.
- For composite keys, a byte key is hashed with xxhash over its bytes
(`HashUtil::xxHash64WithSeed`, and `xxHash64NullWithSeed` for NULL) on both the
delete side and the data side, and a hash hit compares the bytes. STRING and
VARBINARY keys count as the same type.
- Unlike branch-4.2, the byte path also reads const key columns, so a const
NULL key matches a NULL delete key instead of being read as an empty string.
- Non-byte keys keep the current code path.
**Results.**
- BE unit tests: the new `be/test/format/table/equality_delete_test.cpp` has
the 5 tests of branch-4.2 and one for a const NULL key. With master's
`equality_delete.cpp` all 6 fail (one aborts the test process); with this PR
all 6 pass. The other BE tests around V1 Iceberg deletes pass too:
`IcebergReaderTest`, `ParquetReadLinesTest`,
`IcebergPositionDeleteSysTableReaderTest`,
`IcebergPositionDeleteSysTableV2ReaderTest` and `VIcebergDeleteSinkTest`.
- No regression test: Spark SQL cannot write equality delete files, and no
docker fixture has an equality delete with a BINARY or FIXED key.
`test_iceberg_uuid_binary_compatibility` covers UUID keys, on the V2 scanner
only.
### Release note
Fix: With `enable_file_scanner_v2 = false`, reading an Iceberg table whose
equality deletes use a BINARY or FIXED key no longer fails or crashes the BE.
### 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:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- 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]
