This is an automated email from the ASF dual-hosted git repository.
924060929 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 f823bfde17c [test](fe) Pin the partition values and null flags of
external file ranges (#69001)
f823bfde17c is described below
commit f823bfde17c25b4e89bc82ed5fdef1af3ab13e6e
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Wed Oct 14 09:29:38 2026 +0800
[test](fe) Pin the partition values and null flags of external file ranges
(#69001)
### What problem does this PR solve?
Issue Number: None
Related PR: #66012, #62821, #68124, #68786
Problem Summary:
**In short.** This PR ports to master the tests of branch-4.1 #66012
whose behavior master already has, so that behavior is pinned. It
changes no production code. #66012 made external scans and file loads
send each file's partition values with explicit NULL flags; on master
the same behavior arrived through #62821 and the connector plugins, but
nothing tests three parts of it.
**Background.** A file scan range tells BE the partition values of its
file in `columns_from_path_keys`, `columns_from_path` and
`columns_from_path_is_null`, and BE fills the partition columns from
them instead of reading them from the file. Two producers matter here:
- `FilePartitionUtils.parseColumnsFromPathWithNullInfo` parses
`key=value` directories for Hive-style tables, TVFs and file loads. The
Hive default partition (`__HIVE_DEFAULT_PARTITION__`) becomes SQL NULL
through the null flag, and a literal `\N` stays data.
- The Iceberg connector sends a file's identity partition values from
the file's own partition spec and partition data, not from its path.
After partition evolution, files of different specs carry different
keys.
**The problem, and what it cost.** These are the parts of #66012's tests
that hold on master and that no master test covers. Each of them decides
what BE writes into a partition column:
| Behavior | What a regression would do |
|---|---|
| A case-insensitive parse lowercases a copy of the keys, not the
caller's list | The caller's column list changes under it |
| `__HIVE_DEFAULT_PARTITION__` → `""` with the null flag; a literal `\N`
→ `\N` without it; an ACID layout skips its delta directory | A NULL
partition reads as an empty string, or a `\N` value reads as NULL |
| An Iceberg range sends its identity values in spec order, a NULL value
through the null flag | Values land in the wrong columns, or a NULL
partition reads as an empty string |
| After the spec evolves from (Region, Dt) to (Dt, Category), a file of
each spec sends its own spec's keys | Files of one spec get the other
spec's columns, which BE then fills as NULL |
**How this PR fixes it.** It adds three tests:
- `BrokerUtilTest#parseColumnsFromPathPreservesNullMetadataAndInputKeys`
(#66012's test of the same name, calling `FilePartitionUtils`).
-
`IcebergScanPlanProviderTest#planScanSendsIdentityPartitionsInSpecOrderWithNullFlags`
(from #66012's
`IcebergScanNodeTest#testSetPartitionValuesBuildsStableAlignedMetadata`).
-
`IcebergScanPlanProviderTest#planScanSendsEachFilesOwnIdentityPartitionsAfterSpecEvolution`
(from `#testSetPartitionValuesUsesPerSpecMetadataWithFileScannerV2`; it
also checks the old-spec file). The Iceberg tests plan a scan of an
in-memory table, as the neighboring #68124 test does.
#66012 changes six more tests that are not ported, because master
differs on purpose:
| branch-4.x test | Why it does not apply to master |
|---|---|
| `BrokerUtilTest#normalizeColumnsFromPathPreservesNullInfo` | The
connectors map the Hive default partition to NULL themselves
(`HiveScanRangePartitionValuesTest`), so fe-core's normalization only
maps a Java null |
|
`IcebergUtilsTest#testGetCommonIdentityPartitionColumnsUsesSafeIntersection`,
`IcebergScanNodeTest#testSetPartitionValuesKeepsCommonMetadataWithLegacyFileScanner`
| branch-4.x keeps only the identity columns every spec shares; master
keeps the union, so a file written before the spec evolved to
unpartitioned keeps its values (#68124) |
| `IcebergUtilsTest#testMappedTypesAreExcludedFromPartitionMetadata`,
`IcebergScanNodeTest#testSetPartitionValuesSkipsValuesUnsupportedByMappedTypes`
| Since #68786, master sends UUID and TIMESTAMPTZ identity values too |
| `BrokerUtilTest#parseColumnsFromPathForLoadKeepsLegacyNullSemantics` |
A real difference: on branch-4.x a file load stores a literal `\N` path
value as NULL, master stores the two characters `\N`. It needs a code
change and comes in a separate PR |
**Results.** The three tests pass on master without any code change, and
each catches a regression of what it pins:
| Change to master | Fails |
|---|---|
| `FilePartitionUtils` stops flagging the Hive default partition as NULL
| `parseColumnsFromPathPreservesNullMetadataAndInputKeys` |
| `FilePartitionUtils` lowercases the caller's key list in place |
`parseColumnsFromPathPreservesNullMetadataAndInputKeys` |
| `IcebergScanRange` sends a NULL identity value without the null flag |
both new Iceberg tests |
| The Iceberg planner sends every identity key of every spec for each
file | `planScanSendsEachFilesOwnIdentityPartitionsAfterSpecEvolution`
(and the existing
`planScanKeepsUnpartitionedSpecIdentityAfterEvolvingToPartitioned`) |
Original author: @suxiaogang223
---------
Co-authored-by: Socrates <[email protected]>
---
.../iceberg/IcebergScanPlanProviderTest.java | 67 ++++++++++++++++++++++
.../apache/doris/common/util/BrokerUtilTest.java | 26 +++++++++
2 files changed, 93 insertions(+)
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
index e83a3049fde..8654cc569de 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergScanPlanProviderTest.java
@@ -57,6 +57,7 @@ import org.apache.iceberg.FileScanTask;
import org.apache.iceberg.ManifestFile;
import org.apache.iceberg.MetadataColumns;
import org.apache.iceberg.Metrics;
+import org.apache.iceberg.PartitionData;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.Snapshot;
@@ -1016,6 +1017,72 @@ public class IcebergScanPlanProviderTest {
Assertions.assertEquals(OptionalLong.empty(),
provider.scannedPartitionCount(ranges));
}
+ @Test
+ public void planScanSendsIdentityPartitionsInSpecOrderWithNullFlags() {
+ // From branch-4.x #66012: a range carries its identity partition
values keyed and ordered by the partition
+ // spec, and a NULL value travels as an explicit null flag, not as a
string.
+ Schema schema = new Schema(
+ Types.NestedField.optional(1, "Region",
Types.StringType.get()),
+ Types.NestedField.optional(2, "Dt", Types.StringType.get()));
+ PartitionSpec spec =
PartitionSpec.builderFor(schema).identity("Region").identity("Dt").build();
+ Table table = createTable("aligned", schema, spec);
+ table.newAppend().appendFile(partitionedFile(spec,
"s3://b/db/aligned/a.parquet", "cn", null)).commit();
+
+ IcebergScanPlanProvider provider = new IcebergScanPlanProvider(
+ IcebergCatalogProperties.of(Collections.emptyMap()),
opsReturning(table));
+ List<ConnectorScanRange> ranges = provider.planScan(new
FakeScanSession("UTC", Collections.emptyMap()),
+ ConnectorScanRequest.builder(new IcebergTableHandle("db1",
"aligned"), Collections.emptyList())
+ .build());
+
+ Assertions.assertEquals(1, ranges.size());
+ TFileRangeDesc desc = populate(ranges.get(0));
+ Assertions.assertEquals(Arrays.asList("Region", "Dt"),
desc.getColumnsFromPathKeys());
+ Assertions.assertEquals(Arrays.asList("cn", ""),
desc.getColumnsFromPath());
+ Assertions.assertEquals(Arrays.asList(false, true),
desc.getColumnsFromPathIsNull());
+ }
+
+ @Test
+ public void
planScanSendsEachFilesOwnIdentityPartitionsAfterSpecEvolution() {
+ // From branch-4.x #66012: after the spec evolves from (Region, Dt) to
(Dt, Category), a file of either spec
+ // carries the identity values of its own spec, in that spec's order.
+ Schema schema = new Schema(
+ Types.NestedField.optional(1, "Region",
Types.StringType.get()),
+ Types.NestedField.optional(2, "Dt", Types.StringType.get()),
+ Types.NestedField.optional(3, "Category",
Types.StringType.get()));
+ PartitionSpec oldSpec =
PartitionSpec.builderFor(schema).identity("Region").identity("Dt").build();
+ Table table = createTable("evolved", schema, oldSpec,
Collections.singletonMap("format-version", "2"));
+ table.newAppend().appendFile(partitionedFile(oldSpec,
"s3://b/db/evolved/old.parquet", "us", "2024-01-01"))
+ .commit();
+ table.updateSpec().removeField("Region").addField("Category").commit();
+ table.newAppend().appendFile(partitionedFile(table.spec(),
"s3://b/db/evolved/new.parquet", null, "books"))
+ .commit();
+
+ IcebergScanPlanProvider provider = new IcebergScanPlanProvider(
+ IcebergCatalogProperties.of(Collections.emptyMap()),
opsReturning(table));
+ List<ConnectorScanRange> ranges = provider.planScan(new
FakeScanSession("UTC", Collections.emptyMap()),
+ ConnectorScanRequest.builder(new IcebergTableHandle("db1",
"evolved"), Collections.emptyList())
+ .build());
+
+ TFileRangeDesc oldDesc = populate(byPath(ranges, "old.parquet"));
+ Assertions.assertEquals(Arrays.asList("Region", "Dt"),
oldDesc.getColumnsFromPathKeys());
+ Assertions.assertEquals(Arrays.asList("us", "2024-01-01"),
oldDesc.getColumnsFromPath());
+ Assertions.assertEquals(Arrays.asList(false, false),
oldDesc.getColumnsFromPathIsNull());
+ TFileRangeDesc newDesc = populate(byPath(ranges, "new.parquet"));
+ Assertions.assertEquals(Arrays.asList("Dt", "Category"),
newDesc.getColumnsFromPathKeys());
+ Assertions.assertEquals(Arrays.asList("", "books"),
newDesc.getColumnsFromPath());
+ Assertions.assertEquals(Arrays.asList(true, false),
newDesc.getColumnsFromPathIsNull());
+ }
+
+ /** A data file whose partition values, in the spec's field order, may be
null. */
+ private static DataFile partitionedFile(PartitionSpec spec, String path,
String... values) {
+ PartitionData data = new PartitionData(spec.partitionType());
+ for (int i = 0; i < values.length; i++) {
+ data.set(i, values[i]);
+ }
+ return
DataFiles.builder(spec).withPath(path).withFormat(FileFormat.PARQUET).withPartition(data)
+ .withFileSizeInBytes(1024).withRecordCount(1).build();
+ }
+
@Test
public void
planScanCountsScannedPartitionsWhileCurrentSpecStaysPartitioned() {
// The other side of the display gate: with a partitioned CURRENT
spec, files of an older spec keep
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/common/util/BrokerUtilTest.java
b/fe/fe-core/src/test/java/org/apache/doris/common/util/BrokerUtilTest.java
index 25fa2206182..5e1d286fc84 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/common/util/BrokerUtilTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/common/util/BrokerUtilTest.java
@@ -18,6 +18,7 @@
package org.apache.doris.common.util;
import org.apache.doris.common.UserException;
+import org.apache.doris.connector.spi.scan.ConnectorPartitionValues;
import org.apache.doris.datasource.scan.FilePartitionUtils;
import com.google.common.collect.Lists;
@@ -125,4 +126,29 @@ public class BrokerUtilTest {
}
+ @Test
+ public void parseColumnsFromPathPreservesNullMetadataAndInputKeys() throws
Exception {
+ // A case-insensitive parse lowercases its own copy of the keys, not
the caller's list, and turns the Hive
+ // default partition into SQL NULL.
+ List<String> partitionKeys = Lists.newArrayList("Region", "Dt");
+ FilePartitionUtils.ParsedColumnsFromPath parsed =
FilePartitionUtils.parseColumnsFromPathWithNullInfo(
+ "hdfs://host/table/REGION=cn/dt=" +
ConnectorPartitionValues.NULL_PARTITION_NAME + "/data.parquet",
+ partitionKeys, false, false);
+ Assertions.assertEquals(Lists.newArrayList("cn", ""),
parsed.getValues());
+ Assertions.assertEquals(Lists.newArrayList(false, true),
parsed.getIsNull());
+ Assertions.assertEquals(Lists.newArrayList("Region", "Dt"),
partitionKeys);
+
+ // A literal \N is data on the query path.
+ parsed = FilePartitionUtils.parseColumnsFromPathWithNullInfo(
+ "hdfs://host/table/p=\\N/data.orc",
Collections.singletonList("p"), true, false);
+ Assertions.assertEquals(Collections.singletonList("\\N"),
parsed.getValues());
+ Assertions.assertEquals(Collections.singletonList(false),
parsed.getIsNull());
+
+ // An ACID layout has one more directory level between the partition
and the file.
+ parsed = FilePartitionUtils.parseColumnsFromPathWithNullInfo(
+ "hdfs://host/table/p=value/delta_1_1/bucket_00000",
Collections.singletonList("p"), true, true);
+ Assertions.assertEquals(Collections.singletonList("value"),
parsed.getValues());
+ Assertions.assertEquals(Collections.singletonList(false),
parsed.getIsNull());
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
