morningman opened a new pull request, #69229:
URL: https://github.com/apache/doris/pull/69229
### What problem does this PR solve?
Issue Number: None
Related PR: #68481, #68124
Problem Summary:
**In short.** Ports the rest of branch-4.1 #68481 to master. When the source
column of a historical Iceberg partition field has been dropped, DELETE and
UPDATE on master commit a delete file under partition NULL, after which Doris
can no longer read the table. They now fail with a clear error before anything
is written, ordinary reads keep working, and TIME partition values can be
parsed back at commit.
**Background.** #68481 fixed Iceberg row-level DML after a table evolves to
an unpartitioned spec. Its main part, keeping each file's own spec ID and
partition tuple in the scan range once any spec is partitioned, reached master
with #68124; the regression added here confirms that REPLACE to an
unpartitioned table and dropping the last partition field work on master. Row
IDs carry each file's `partition_data_json`, and BE writes the position-delete
file of a DELETE, UPDATE or MERGE under that partition.
**The problem, and what it cost.**
- Dropping the source column of a historical partition field leaves the
field with no type, so the file's partition tuple cannot be serialized.
`IcebergPartitionUtils.getPartitionValues` swallowed the
`UnsupportedOperationException` and wrote a null entry. A DELETE on such a
table then committed a delete file under partition NULL for a data file in
partition 7. Spark still reads the table, but every later Doris query of it
fails:
`Mismatched partition tuples (PartitionData{partition_key=null},
PartitionData{partition_key=7}) for delete file s3://.../delete_pos_....parquet
and data file s3a://.../partition_key=7/00000-....parquet`
- `parsePartitionValueFromString` had no TIME branch, so the value
`serializePartitionValue` renders for a TIME partition could not be parsed back
when the delete file is committed (`convertToPartitionData`).
**How this PR fixes it.**
- `getPartitionValues` throws instead of returning a null entry.
- When a data file's partition tuple cannot be serialized, its range carries
no `partition_data_json` and is marked. Ordinary reads do not use the payload,
so they are unaffected.
- A scan that produces row IDs over such a range is rejected: `Cannot
produce Iceberg row IDs with unsupported partition types in spec N. Rewrite
historical data files before DELETE or UPDATE.` The check runs on every
`planScan` call, including one served by the statement scan reuse (its key
leaves out the projection, as for the metadata-column reader check), and on
each range of a streaming split source.
- `getColumnHandles` returns a handle for the hidden row-id column, as it
does for `_file` and `_pos`, so a DML scan request carries it. Before, the
column had no handle and never reached the connector. The field-id schema
dictionary leaves it out, since it is not a table column.
- `parsePartitionValueFromString` parses TIME (microseconds since midnight).
Branch-4.x renders `partition_data_json` only for scans that produce row
IDs. Master keeps rendering it for every scan, because
`specId|partitionDataJson` is the partition identity behind EXPLAIN
`partition=N/M` and `sql_block_rule` `partition_num`.
**Results.** Local cluster with the Iceberg docker environment (tables
written by Spark):
| Case | Before | After |
|---|---|---|
| Spark REPLACE a partitioned table with an unpartitioned one, then two
Doris DELETEs | Works (#68124) | Same |
| Drop the last partition field, then DELETE and UPDATE across both specs |
Works | Same |
| Drop the partition field, then its source column; Doris DELETE (Parquet
and ORC) | Commits a delete file under partition NULL; Doris reads of the table
then fail with `Mismatched partition tuples` | Rejected with the error above;
no delete file, rows unchanged in Doris and Spark |
| Same table, Doris UPDATE | Same as DELETE | Rejected |
| DELETE on historical binary partitions; DELETE and UPDATE on pre-1970
timestamp partitions | Works | Same |
Original author: @Gabriel39 (#68481)
### Release note
Iceberg DELETE, UPDATE and MERGE now fail with a clear error, instead of
writing delete files that make the table unreadable from Doris, when a data
file belongs to a partition field whose source column was dropped.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] 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? -->
- `IcebergScanPlanProviderTest`: an ordinary read of such a file and the
rejection of row-ID scans through `planScan`, statement reuse and
`streamSplits`; the row-id handle stays out of the field-id dictionary.
`IcebergPartitionUtilsTest`: `getPartitionDataJson` throws for a field without
a type, and TIME parses back what serialize renders.
`IcebergConnectorMetadataTest`: the row-id handle. Mutations each turn the
intended tests red: restoring the null fallback (4 tests), skipping the check
for reused ranges, skipping it in the streaming source, dropping the TIME
branch, and keeping the row-id name in the dictionary. All 1592
fe-connector-iceberg tests pass.
- Regression: `test_iceberg_delete_unpartitioned_evolution` (ported from
branch-4.x) passes. Of the 33 Iceberg suites that run DELETE / UPDATE / MERGE,
31 pass. The other two fail the same way without this change:
`test_iceberg_write_binary_partitions` needs a BE newer than my local one (exec
version 19), and `test_iceberg_deletion_vector` hits `Cannot obtain metadata`
in the Trino container.
- Behavior changed:
- [ ] No.
- [x] Yes. Row-level DML over data files of a partition field whose
source column was dropped is rejected instead of writing a NULL delete
partition.
- 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]
