morningman opened a new pull request, #68919:
URL: https://github.com/apache/doris/pull/68919
### What problem does this PR solve?
Issue Number: None
Related PR: #66685, #66321, #66626, #66907
Problem Summary:
**In short.** This ports the tests of four branch-4.1 PRs that master
already implements but does not pin: partition- and bucket-aware concurrent
writes for external tables (#66685), Paimon Variant V2 writes (#66321), the
required Variant array fallback value (#66626) and the BE exec version
alignment (#66907). It changes no production code.
**Background.** On master the behavior of these PRs arrived through #68320,
the generic connector write framework (connector-defined write distribution,
opaque commit reports, generic Variant V2 writes), and #67395, Paimon table
writes. Checking each 4.x change against master, with branch-4.2 as the final
4.x form:
| 4.x change | master |
|---|---|
| Hive partitioned writes hashed by partition columns, skewed writer
assignment | equivalent: `HiveWritePlanProvider.requiresPartitionHashWrite` →
`HIVE_TABLE_SINK_HASH_PARTITIONED` → BE scale writer |
| Paimon fixed-bucket routing, single writer for tables that need one |
equivalent: `PaimonWritePlanProvider.getWriteDistribution`; a GATHER sink runs
as one instance, one task, one writer |
| Paimon Variant V2 writes and reads, including #66626 | equivalent:
`ConnectorComputeVariantType`, `CheckCast`, the BE Variant V2 serde, the JNI
reader |
| BE exec versions 11 and 13 | same meaning on both sides |
**The problem, and what it cost.** These behaviors are unpinned on master:
- `test_paimon_write_pk` checks only that the writer plan contains a sink
and a TVF, and its comment still describes a gather fallback master does not
have. A change that gathers the write to one writer passes it.
- Paimon write distribution is tested only for the default options. Nothing
covers a HASH_DYNAMIC table with an assigner parallelism, the route a
fixed-bucket table actually gets (route columns, Paimon's bucket function,
IDENTITY assignment), or the fallbacks to a single writer for a non-default
bucket function, an unroutable partition key or a defaulted route field.
- Route columns of a positional-write connector must be resolved by the
bound target schema, not by the INSERT column list; only the name-mapped case
is tested.
- The analysis-time contract of a connector Variant column has one positive
case: nothing shows that MAP, STRUCT, TIME, DECIMAL beyond 38 or VARBINARY
sources are rejected in both cast modes, including inside arrays.
- How a Doris INSERT OVERWRITE sets Paimon's `dynamic-partition-overwrite`,
nested Variant mapping on read, and the fallback-pair read order with a blank
primary branch or a chain table are untested.
**How this PR fixes it.**
- `test_paimon_write_pk`: the plan check becomes `qt_pk_writer_scaling_plan`
as on branch-4.2, regenerated on master: `PhysicalConnectorTableSink` →
`PhysicalDistribute[DistributionSpecExternalTableSinkHashPartitioned]`.
- `PaimonWritePlanProviderTest`: HASH_DYNAMIC with
`dynamic-bucket.assigner-parallelism=4` stays a single writer; a routable
fixed-bucket table gets `paimon_fixed_bucket` with route columns `[part, id]`,
`num_buckets=4`, partition index 0, bucket index 1 and IDENTITY;
`bucket-function=mod`, a DATE partition key, and a defaulted bucket or
partition field fall back to GATHER.
- `PhysicalConnectorTableSinkTest`: route columns of a full-schema-order
connector come from their bound-schema positions.
- `CheckCastTest`: the accepted and rejected sources of
`ConnectorComputeVariantType` in strict and non-strict mode, the array shape
changes, and the leaf check inside containers.
- `PaimonWriteBindingTest` (new): a full overwrite uses static semantics
unless the table asks for dynamic ones; a static-partition overwrite is always
static.
- `PaimonTypeMappingReadTest`: a Variant nested in ARRAY, ROW or MAP maps to
the V2 execution carrier.
- `PaimonReaderOptionsTest` and `PaimonJniScannerTest`: the fallback-pair
read order for a blank primary branch and for a chain table, on the FE and the
BE side.
Branch-4.x tests that do not apply are not ported: the Variant V2 session
switch (master only has V2), the Hive plan assertions on a distribution the
test itself requested, and Paimon route fallbacks the planner cannot produce on
master.
Found while checking and left to separate changes:
- Iceberg partitioned writes are not hash-distributed by partition on
master. An `INSERT` into an Iceberg table that reaches 200 partitions fails
with `Too many open partitions 128` on a local cluster; branch-4.x spreads the
partitions over its writers.
- When a Variant is coerced to a common type with a number (UNION, IF,
CASE), master writes object and array Variants as NULL, as
`test_paimon_write_variant_errors` documents; branch-4.x rejects the statement
with its Variant write validator, which will come with the Iceberg Variant
write port.
- `TReportExecStatusParams.34` and the external partition types in
`Partitions.thrift` mean different things on master and on the released 4.1.4;
#68837 aligns them with branch-4.1.
**Results.** Each new unit test fails when the logic it guards is changed:
dropping the defaulted-field check in `PaimonWritePlanProvider`, accepting a
MAP source in `CheckCast`, removing the chain-table precedence in
`PaimonReaderOptions`, or resolving route columns by the INSERT column list.
Original author of the ported tests: @suxiaogang223
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `test_paimon_write_pk` regenerated and passing on a local cluster
with the Paimon docker environment; the six Paimon Variant write suites,
`test_paimon_write_bucket_modes` and `test_paimon_write_pk` pass there (8
suites).
- [x] Unit Test
- `PaimonWritePlanProviderTest`, `PaimonWriteBindingTest`,
`PaimonTypeMappingReadTest`, `PaimonReaderOptionsTest`, `PaimonJniScannerTest`,
`CheckCastTest`, `PhysicalConnectorTableSinkTest` pass.
- [ ] 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]
