morningman opened a new pull request, #68806:
URL: https://github.com/apache/doris/pull/68806
### What problem does this PR solve?
Issue Number: None
Related PR: #66302, #66413
Problem Summary:
**In short.** This ports three FE unit-test assertions of branch-4.1 #66302
(reading Iceberg Variant from Parquet) to master. master already has the
behavior (#66413 forward-ported it into the connector framework), but these
guards are currently unprotected by any FE unit test. Only tests are added; no
production code changes.
**Background.** #66302 made Iceberg VARIANT columns readable as the
compute-only Variant V2 type and added three guards around them: data-file
writes to a table with a Variant column are rejected ("read-only"), a
delete-only MERGE tells BE it writes no data file so it can still run, and the
old-backend Variant fence looks at the type a scan actually projects after
nested-column pruning. In master these live in `IcebergWritePlanProvider`,
`PluginDrivenTableSink` and `PluginDrivenScanNode`.
| 4.1 test (#66302) | master today |
|---|---|
| `IcebergMergeSinkTest#testBindDataSinkMarksDeleteOnlyMerge`: thrift
`writes_data_files` is set and false | only the engine-to-handle half is
covered
(`PluginDrivenTableSinkTest#bindDataSinkThreadsDeleteOnlyMergeToHandle`); no
test reads `TIcebergMergeSink.writes_data_files` |
| `IcebergUtilsTest#testIcebergWriteRejectsRootAndNestedVariant`: root and
nested Variant rejected, message says "read-only" |
`rejectsVariantDataWritesButAllowsDeleteOnlyMerge` only checks a STRUCT-nested
plain `VARIANT`; no root column, no `VARIANT_COMPUTE_V2`, no message check |
|
`IcebergScanNodeTest#testVariantUpgradeGateUsesEffectiveProjectedSlotType`:
pruned type without Variant → false, full type → true | only the positive case
(`translatedScanTuplePreservesNestedComputeVariantCarrier`) |
**The problem, and what it cost.** The engine hands an Iceberg VARIANT
column to the write path as the `VARIANT_COMPUTE_V2` carrier, not as plain
`VARIANT`. Today the `VARIANT_COMPUTE_V2` arm of
`IcebergWritePlanProvider#containsVariant` can be deleted without any FE unit
test failing; only a docker regression suite would notice, and Doris would then
try to write Variant data it cannot encode. Likewise, nothing pins that a
delete-only MERGE ships `writes_data_files=false` (otherwise BE opens a data
writer the statement never uses and hits writer-side schema checks), or that a
scan whose pruning removed the Variant child is not needlessly fenced off old
backends.
**How this PR fixes it.** It adds the three assertions in master's terms:
| New test | Asserts |
|---|---|
|
`IcebergWritePlanProviderTest#planWriteMergeSinkShipsWhetherTheWriteProducesDataFiles`
| a delete-only MERGE ships `writes_data_files=false`, an UPDATE ships true,
and the field is always set |
| `IcebergWritePlanProviderTest#rejectsRootAndNestedComputeVariantCarrier` |
a root and a STRUCT-nested `VARIANT_COMPUTE_V2` column are rejected with the
read-only message for data writes and accepted for delete-only writes |
|
`PluginDrivenScanNodeCompatibilityTest#projectsComputeVariantFollowsTheEffectiveSlotType`
| for one slot, the gate answers false with the pruned type and true with the
full type |
**Results.** The new tests pass on current master and fail if the
corresponding guard regresses.
Original author: @Gabriel39
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- `run-fe-ut.sh --run` on `IcebergWritePlanProviderTest`,
`PluginDrivenScanNodeCompatibilityTest`, `IcebergTypeMappingReadTest`,
`ConnectorColumnConverterTest`, `PruneNestedColumnTest`,
`VariantPruningLogicTest`, `PluginDrivenTableSinkTest`: 232 tests, 0 failures
- `mvn checkstyle:check` on fe-core and fe-connector-iceberg: 0
violations
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- 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]
