morningman opened a new pull request, #69394:
URL: https://github.com/apache/doris/pull/69394
### What problem does this PR solve?
Issue Number: None
Related PR: #66538 #65851
Problem Summary:
**In short.** This ports the Iceberg V3 default-value tests of branch-4.1
#66538 to master: 3 BE unit tests, 37 FE unit tests and one regression check,
all for behavior master already has. It also reserves field id 13 of
`TIcebergFileDesc`, which branch-4.1 and branch-4.2 use for
`equality_delete_schema`. No product code changes.
**Background.** Iceberg V3 initial / write defaults were implemented twice:
by #65851 on master and by #66538 on branch-4.1, which branch-4.2 inherits. The
4.1 tests target fe-core classes (`IcebergScanNode`,
`IcebergWriteSchemaContext`, `IcebergTransaction`, …) and V1 reader internals
that master no longer has. On master the Iceberg FE lives in the
`fe-connector-iceberg` plugin and the V1 reader in `iceberg_reader_mixin.h`. So
most of the 4.1 tests never reached master, even where master implements the
same behavior.
**The problem, and what it cost.** A scenario-by-scenario comparison of the
two test suites found that master implements most of the 4.1-tested behavior
with no test of its own. The untested behavior:
| Area | Untested on master before this PR |
|---|---|
| Equality-delete schema carrier | a dropped key keeps its latest name,
optionality and initial default; a dropped nested key comes back inside its
parent; a dropped nanosecond key stays `TIMESTAMP_NS`; only referenced
historical fields enter the carrier |
| Rolling-upgrade fences | missing-required-field and nested-initial-default
fences follow the projection; collection wrappers do not trigger them;
schema-only targets and cherry-picked ancestry are covered; name-mapping alias
collisions |
| Write commit fences | schema skew, a recreated table and current-spec
replacement are rejected before a transaction opens; a required column, a spec
drift to unpartitioned or a table replacement that lands while the commit
stages its snapshot is rejected at the final metadata swap |
| Pinned writer metadata | INSERT commit, merge sink (including the
unpartitioned delete spec id) and file conversion use the statement-pinned
spec, sort order and format, not the live table |
| Write defaults | typed SQL literal per primitive type; maps and NULL
elements in complex defaults; `DEFAULT` resolves by field id, not by a reused
name; UPDATE and matched MERGE do not inject write defaults |
| Wire compatibility | `TIcebergFileDesc` ends at field 12 on master, while
4.1 / 4.2 send `13: optional ExternalTableSchema.TSchema
equality_delete_schema`; nothing stops master from reusing 13 for an unrelated
field |
Without these tests a regression in any of these paths would go unnoticed,
and a reused field 13 would misdecode 4.x plans.
**How this PR fixes it.** It rewrites the 4.1 scenarios against master's
APIs. Plugin tests use the existing Mockito-free fakes (`InMemoryCatalog`,
`Proxy` over `TableOperations`). The commit-fence tests hook `newSnapshotId` on
the staging operations, so the concurrent change lands after the commit
preflight and before the metadata swap. The BE tests are copied unchanged from
4.1 (the helper signatures match on master). `PlanNodes.thrift` gets a comment
reserving field 13. `test_iceberg_write_default` gains a `qt_` check that
`information_schema.columns.COLUMN_DEFAULT` stays NULL for a column whose
default is a connector-only write default. The behavior differences found in
the same comparison are fixed in separate PRs (for example #69369).
**Results.** All new tests pass on master. To show they check something, the
fence and pinned-metadata tests were re-run with the guarded condition removed
from the product code; each failed.
| Suite | Result |
|---|---|
| `fe-connector-iceberg` (all tests) | 1656 run, 0 failures, 0 errors, 5
skipped; checkstyle clean |
| fe-core `ConnectorWriteSchemaUtilsTest`,
`ExternalRowLevelUpdatePlanBuilderTest`, `ExternalRowLevelMergePlanBuilderTest`
| 13 run, 0 failures |
| BE
`IcebergV2ReaderTest.IcebergProjectedStructDoesNotReuseReaddedEqualityKeyByName`,
`ParquetReadLinesTest.iceberg_row_id_fetch_materializes_readded_missing_column`,
`VOrcTransformerTest.SkipsInvalidBinaryChildrenHiddenByNullableCollections` |
pass |
| Regression `external_table_p0/iceberg/test_iceberg_write_default` | pass |
Original author: @hubgeter
### Release note
None
### 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? -->
The BE tests ran locally on a 10-20 master base; the BE files master
changed since then (profile counters, zone-map pruning, Paimon timestamps) are
outside the tested paths.
- 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]
