This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 7026c77fffc [test](fe) Cover Iceberg Variant write gating and
effective-type scan gating (#68806)
7026c77fffc is described below
commit 7026c77fffcf6973bef6cf4c9dc2cca3eace8420
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Fri Oct 9 20:56:33 2026 +0800
[test](fe) Cover Iceberg Variant write gating and effective-type scan
gating (#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.
Co-authored-by: Gabriel <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
---
.../iceberg/IcebergWritePlanProviderTest.java | 46 ++++++++++++++++++++++
.../PluginDrivenScanNodeCompatibilityTest.java | 25 ++++++++++++
2 files changed, 71 insertions(+)
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergWritePlanProviderTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergWritePlanProviderTest.java
index b06c8b63792..870c3658b52 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergWritePlanProviderTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergWritePlanProviderTest.java
@@ -130,6 +130,29 @@ public class IcebergWritePlanProviderTest {
() ->
IcebergWritePlanProvider.validateWriteSchema(partialInsert));
}
+ @Test
+ public void rejectsRootAndNestedComputeVariantCarrier() {
+ // The engine hands an Iceberg VARIANT column to the write path as the
VARIANT_COMPUTE_V2 carrier
+ // (ConnectorColumnConverter), not as plain VARIANT, so the read-only
gate must recognize the carrier
+ // both as a root column and nested in a complex type. MUTATION:
dropping the VARIANT_COMPUTE_V2 arm
+ // of containsVariant -> red.
+ ConnectorType carrier = ConnectorType.of("VARIANT_COMPUTE_V2");
+ ConnectorColumn rootVariant = new ConnectorColumn("v", carrier, null,
true, null);
+ ConnectorColumn nestedVariant = new ConnectorColumn("payload",
+ ConnectorType.structOf(Collections.singletonList("nested"),
Collections.singletonList(carrier)),
+ null, true, null);
+ for (ConnectorColumn column : Arrays.asList(rootVariant,
nestedVariant)) {
+ DorisConnectorException exception =
Assertions.assertThrows(DorisConnectorException.class,
+ () ->
IcebergWritePlanProvider.validateWriteSchema(Collections.singletonList(column),
true),
+ column.getName());
+ Assertions.assertTrue(exception.getMessage().contains("VARIANT")
+ && exception.getMessage().contains("read-only"),
exception.getMessage());
+ // A delete-only MERGE writes no data file, so the same schema
stays usable for it.
+ Assertions.assertDoesNotThrow(() ->
IcebergWritePlanProvider.validateWriteSchema(
+ Collections.singletonList(column), false),
column.getName());
+ }
+ }
+
private static InMemoryCatalog freshCatalog() {
InMemoryCatalog catalog = new InMemoryCatalog();
catalog.initialize("test", Collections.emptyMap());
@@ -1828,6 +1851,29 @@ public class IcebergWritePlanProviderTest {
+ " legal UPDATEs whose predicate matches a row
through several source rows");
}
+ @Test
+ public void planWriteMergeSinkShipsWhetherTheWriteProducesDataFiles() {
+ // BE opens the data-file writer only when writes_data_files is true.
A delete-only SQL MERGE must
+ // ship false so it neither builds a writer it never uses nor trips
the writer-side schema checks
+ // (an Iceberg VARIANT target is deletable but not writable).
MUTATION: dropping setWritesDataFiles,
+ // or shipping a constant -> one of the two plans below carries the
wrong value -> red.
+ Table table = partitionedSortedTable(freshCatalog());
+ TIcebergMergeSink deleteOnlyMerge = planMergeSink(table,
contextWithStorage(),
+ new WriteHandle(new IcebergTableHandle("db1", "t1"))
+ .writeOperation(WriteOperation.MERGE)
+ .writesDataFiles(false)
+ .requireMergeCardinalityCheck(true));
+ Assertions.assertTrue(deleteOnlyMerge.isSetWritesDataFiles(),
+ "the field must always be set so BE never has to guess from an
unset field");
+ Assertions.assertFalse(deleteOnlyMerge.isWritesDataFiles());
+
+ TIcebergMergeSink update = planMergeSink(table, contextWithStorage(),
+ new WriteHandle(new IcebergTableHandle("db1", "t1"))
+ .writeOperation(WriteOperation.UPDATE));
+ Assertions.assertTrue(update.isSetWritesDataFiles());
+ Assertions.assertTrue(update.isWritesDataFiles());
+ }
+
@Test
public void planWriteBuildsMergeSinkWithTableDerivedFields() {
Table table = partitionedSortedTable(freshCatalog());
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/scan/PluginDrivenScanNodeCompatibilityTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/scan/PluginDrivenScanNodeCompatibilityTest.java
index 7b3c4759fbe..7b03c7221b7 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/scan/PluginDrivenScanNodeCompatibilityTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/scan/PluginDrivenScanNodeCompatibilityTest.java
@@ -18,12 +18,17 @@
package org.apache.doris.datasource.scan;
import org.apache.doris.analysis.SlotDescriptor;
+import org.apache.doris.analysis.SlotId;
import org.apache.doris.analysis.TableSample;
import org.apache.doris.analysis.TupleDescriptor;
+import org.apache.doris.analysis.TupleId;
import org.apache.doris.catalog.ArrayType;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.PartitionItem;
+import org.apache.doris.catalog.StructField;
+import org.apache.doris.catalog.StructType;
import org.apache.doris.catalog.TableIf;
+import org.apache.doris.catalog.Type;
import org.apache.doris.common.Config;
import org.apache.doris.common.UserException;
import org.apache.doris.common.jmockit.Deencapsulation;
@@ -157,6 +162,26 @@ public class PluginDrivenScanNodeCompatibilityTest {
.types.get(1).scalar_type.variant_is_v2);
}
+ @Test
+ public void projectsComputeVariantFollowsTheEffectiveSlotType() {
+ // Nested-column pruning narrows the slot type but keeps the original
Column, so a scan that pruned
+ // the Variant child away decodes no Variant and must not be fenced
off old backends.
+ // MUTATION: deciding from the slot's Column instead of its effective
type -> red.
+ StructType full = new StructType(
+ new StructField("label", Type.STRING),
+ new StructField("payload", new ConnectorComputeVariantType()));
+ StructType pruned = new StructType(new StructField("label",
Type.STRING));
+ TupleDescriptor tuple = new TupleDescriptor(new TupleId(0));
+ SlotDescriptor slot = new SlotDescriptor(new SlotId(1), tuple.getId());
+ slot.setColumn(new Column("info", full));
+ tuple.addSlot(slot);
+
+ slot.setType(pruned);
+
Assertions.assertFalse(PluginDrivenScanNode.projectsComputeVariant(tuple));
+ slot.setType(full);
+
Assertions.assertTrue(PluginDrivenScanNode.projectsComputeVariant(tuple));
+ }
+
@Test
public void oldBackendAllowsOnlyFullyPrecomputedVariantCountPlans() throws
UserException {
ConnectorScanRange countRange = new ConnectorScanRange() {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
