voonhous commented on code in PR #19648:
URL: https://github.com/apache/hudi/pull/19648#discussion_r3813353926
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/PartitionPathFormatterBase.java:
##########
@@ -75,15 +75,11 @@ public final S combine(List<String> partitionPathFields,
Object... partitionPath
if (useHiveStylePartitioning) {
sb.appendJava(partitionPathFields.get(i))
- .appendJava("=")
- .append(partitionPathPartStr);
- } else if (slashSeparatedDatePartitioning) {
- String res = ((String) partitionPathPartStr).replace('-', '/');
- sb.append(((S) res));
- } else {
- sb.append(partitionPathPartStr);
+ .appendJava("=");
}
+ sb.append(partitionPathPartStr);
Review Comment:
The multi-field branch this rewrites is not reached by any write path:
`getPartitionPath(Row)` has no production caller whose output lands on disk,
and the `InternalRow` side threw on master. Its one production consumer is
query-side pruning: `SparkHoodieTableFileIndex.composeRelativePartitionPath`
(`:471-489`) builds a `StringPartitionPathFormatter` from table config and
`combine`s only the bound *prefix* of partition columns (`:404`).
For a `(datestr, city)` slash table written by the Avro path
(`2026-01-05/san-francisco`), `where datestr = '2026-01-05'` alone composes the
single-part `2026/01/05`, `exists()` fails at `:444`, and the query silently
returns zero rows. Same on master; this PR only fixes the all-columns-bound
case (master composed `2026/01/05/san/francisco`).
`ShowHoodieTablePartitionsCommand.scala:60-61` already `checkState`s "Only one
partition field is allowed for SlashEncodedPartitioning".
Ask: (1) reject `slash.separated.date.partitioning=true` with more than one
partition field at table creation and in writer config validation, next to the
existing hive-style check (or file a follow-up issue and link it here); (2)
reword the PR "Behavior change" section -- the multi-field change lands in
file-index prefix pruning, not in any write path, and no existing table layout
changes.
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/PartitionPathFormatterBase.java:
##########
@@ -62,11 +62,11 @@ public final S combine(List<String> partitionPathFields,
Object... partitionPath
// Avoid creating [[StringBuilder]] in case there's just one
partition-path part,
// and Hive-style of partitioning is not required
if (!useHiveStylePartitioning && partitionPathParts.length == 1) {
- if (slashSeparatedDatePartitioning) {
- return ((S) ((String) toString(partitionPathParts[0])).replace('-',
'/'));
- } else {
- return tryEncode(handleEmpty(toString(partitionPathParts[0])));
- }
+ S partitionPathPart =
tryEncode(handleEmpty(toString(partitionPathParts[0])));
+ // NOTE: Slash-separated date partitioning only kicks in for a table
partitioned by a single
Review Comment:
This NOTE is wrong for `CustomKeyGenerator`: it builds one single-field
sub-keygen per partition field (`CustomKeyGenerator.java:82-101`), so each
field takes this fast path and `date:simple,city:simple` yields
`2026/01/05/san/francisco` on Avro, Row and InternalRow alike (Avro does the
same via `KeyGenUtils#getPartitionPath`, which slashes unconditionally). This
PR is also what un-breaks that InternalRow path (same CCE), and nothing tests
it.
Ask: scope the NOTE to `SimpleKeyGenerator`/`ComplexKeyGenerator` and cite
`KeyGenUtils#getPartitionPath` (the method governing the single-field case)
alongside `getRecordPartitionPath`; add Row + InternalRow asserts to the
existing `TestCustomKeyGenerator.testSlashSeparatedDatePartitioning`, which
already builds the record.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestSlashSeparatedPartitionValue.scala:
##########
@@ -90,6 +90,61 @@ class TestSlashSeparatedPartitionValue extends
HoodieSparkSqlTestBase {
}
}
+ test("Test slash separated date partitions written through the row writer") {
+ withSQLConf("hoodie.sql.bulk.insert.enable" -> "true",
"hoodie.sql.insert.mode" -> "non-strict",
+ "hoodie.datasource.write.row.writer.enable" -> "true") {
+ withTempDir { tmp =>
+ val targetTable = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$targetTable"
+
+ spark.sql(
+ s"""
+ |create table $targetTable (
+ | `id` string,
+ | `name` string,
+ | `ts` bigint,
+ | `datestr` STRING
Review Comment:
Every slash test in the repo partitions on a STRING column pre-formatted
`yyyy-MM-dd`; none uses the DATE/TIMESTAMP type the feature is documented for.
DATE agrees on all three paths (`LocalDate.toString` via
`BuiltinKeyGenerator.convertToLogicalDataType` and
`HoodieAvroUtils.convertValueForAvroLogicalTypes`) but is unpinned. TIMESTAMP
diverges: the slash arm at `SqlKeyGenerator.scala:164-166` (from #17787) sits
ahead of the `TimestampType` normalization (cf. #12621) and shadows it, so the
Avro path (default `consistent.logical.timestamp=false`) writes raw micros
`1767607200000000` while the row writer now writes `2026/01/05 18:00:00.0` (it
threw before this PR).
Ask: add a DATE column case to this suite (insert + bulk_insert, same
directory, `datestr` reads back as `date'2026-01-05'`). For TIMESTAMP either
move the slash arm inside `case _ =>` of the dataType match or reject
TimestampType partition columns under slash -- a linked follow-up issue is fine.
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestSimpleKeyGenerator.java:
##########
@@ -213,4 +213,34 @@ void
testSlashSeparatedDatePartitioningWithAlreadyFormattedInput() {
Assertions.assertEquals("key1", key.getRecordKey());
Assertions.assertEquals("2026/01/01", key.getPartitionPath());
}
+
+ @Test
+ void testSlashSeparatedDatePartitioningOnRowWritingPaths() {
+ TypedProperties properties = getPropsWithSlashSeparatedDatePartitioning();
+ // NOTE: "ts_ms" is the string-typed field of the example schema,
"timestamp" is a long
+ properties.put(KeyGeneratorOptions.PARTITIONPATH_FIELD_NAME.key(),
"ts_ms");
+ SimpleKeyGenerator keyGenerator = new SimpleKeyGenerator(properties);
+
+ GenericRecord avroRecord = getRecord();
+ Assertions.assertEquals("2020/03/21",
keyGenerator.getPartitionPath(avroRecord));
+
+ Row row = KeyGeneratorTestUtilities.getRow(avroRecord);
+ Assertions.assertEquals("2020/03/21", keyGenerator.getPartitionPath(row));
+
+ InternalRow internalRow = KeyGeneratorTestUtilities.getInternalRow(row);
+ Assertions.assertEquals(UTF8String.fromString("2020/03/21"),
+ keyGenerator.getPartitionPath(internalRow, row.schema()));
+ }
+
+ @Test
+ void testSlashSeparatedDatePartitioningWithNullValue() {
+ TypedProperties properties = getPropsWithSlashSeparatedDatePartitioning();
+ properties.put(KeyGeneratorOptions.PARTITIONPATH_FIELD_NAME.key(),
"nested_col.prop1");
+ SimpleKeyGenerator keyGenerator = new SimpleKeyGenerator(properties);
+
+ GenericRecord avroRecord = getRecord(getNestedColRecord(null, 10L));
+
+ Row row = KeyGeneratorTestUtilities.getRow(avroRecord);
+ Assertions.assertEquals(HUDI_DEFAULT_PARTITION_PATH,
keyGenerator.getPartitionPath(row));
Review Comment:
This only asserts the Row path; the InternalRow + null case (the row-writer
NPE the PR describes) is asserted at no keygen level. The two extra lines below
go red on master with the same NPE.
```suggestion
Assertions.assertEquals(HUDI_DEFAULT_PARTITION_PATH,
keyGenerator.getPartitionPath(row));
InternalRow internalRow = KeyGeneratorTestUtilities.getInternalRow(row);
Assertions.assertEquals(UTF8String.fromString(HUDI_DEFAULT_PARTITION_PATH),
keyGenerator.getPartitionPath(internalRow, row.schema()));
```
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestSlashSeparatedPartitionValue.scala:
##########
@@ -90,6 +90,61 @@ class TestSlashSeparatedPartitionValue extends
HoodieSparkSqlTestBase {
}
}
+ test("Test slash separated date partitions written through the row writer") {
+ withSQLConf("hoodie.sql.bulk.insert.enable" -> "true",
"hoodie.sql.insert.mode" -> "non-strict",
+ "hoodie.datasource.write.row.writer.enable" -> "true") {
+ withTempDir { tmp =>
+ val targetTable = generateTableName
+ val tablePath = s"${tmp.getCanonicalPath}/$targetTable"
+
+ spark.sql(
+ s"""
+ |create table $targetTable (
+ | `id` string,
+ | `name` string,
+ | `ts` bigint,
+ | `datestr` STRING
+ |) using hudi
+ | tblproperties (
+ | 'primaryKey' = 'id',
+ | 'type' = 'COW',
+ | 'preCombineField'='ts',
+ |
'hoodie.datasource.write.slash.separated.date.partitioning'='true'
+ | )
+ | partitioned by (`datestr`)
+ | location '$tablePath'
+ """.stripMargin)
+
+ // NOTE: The row writer derives the partition path off of an
[[InternalRow]], which used to
+ // blow up with a [[ClassCastException]]; a null partition value
used to NPE
+ spark.sql(
+ s"""
+ | insert into $targetTable values
+ | (1, 'a1', 1000, "2026-01-05"),
+ | (2, 'a2', 2000, "2026-01-06"),
+ | (3, 'a3', 3000, null)
+ """.stripMargin)
+
+ checkAnswer(s"select id, name, ts, _hoodie_partition_path from
$targetTable order by id")(
+ Seq("1", "a1", 1000, "2026/01/05"),
+ Seq("2", "a2", 2000, "2026/01/06"),
+ Seq("3", "a3", 3000, "__HIVE_DEFAULT_PARTITION__")
Review Comment:
Siblings at `:62` and `:185` project `datestr`; this one does not, so the
read-back through `HoodieSparkUtils.doParsePartitionColumnValues` for
row-writer data and the null row is unasserted.
```suggestion
checkAnswer(s"select id, name, ts, _hoodie_partition_path, datestr
from $targetTable order by id")(
Seq("1", "a1", 1000, "2026/01/05", "2026-01-05"),
Seq("2", "a2", 2000, "2026/01/06", "2026-01-06"),
Seq("3", "a3", 3000, "__HIVE_DEFAULT_PARTITION__", null)
```
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestPartitionPathFormatter.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.keygen;
+
+import org.apache.spark.unsafe.types.UTF8String;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static
org.apache.hudi.common.util.PartitionPathEncodeUtils.DEFAULT_PARTITION_PATH;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Tests the partition-path formatters backing the key generators, making sure
that both the
+ * {@link String} (Avro/{@link org.apache.spark.sql.Row} write path) and the
{@link UTF8String}
+ * (row-writer/{@link org.apache.spark.sql.catalyst.InternalRow} write path)
flavors produce
+ * identical partition paths.
+ */
+class TestPartitionPathFormatter {
+
+ private static final List<String> SINGLE_FIELD =
Collections.singletonList("date_col");
+ private static final List<String> TWO_FIELDS = Arrays.asList("date_col",
"city");
+
+ private String combine(boolean useRowWriterPath,
+ boolean hiveStylePartitioning,
+ boolean encode,
+ boolean slashSeparatedDatePartitioning,
+ List<String> fields,
+ Object... parts) {
+ if (useRowWriterPath) {
+ return new UTF8StringPartitionPathFormatter(
+ UTF8StringPartitionPathFormatter.UTF8StringBuilder::new,
hiveStylePartitioning, encode,
+ slashSeparatedDatePartitioning).combine(fields, parts).toString();
+ }
+ return new StringPartitionPathFormatter(
+ StringPartitionPathFormatter.JavaStringBuilder::new,
hiveStylePartitioning, encode,
+ slashSeparatedDatePartitioning).combine(fields, parts);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testSlashSeparatedDatePartitioningSingleField(boolean useRowWriterPath)
{
Review Comment:
Delete: the only assert that fails on master is the UTF8 run of the first
line (CCE), which `...EncodesValues`, `...OnlyAppliesToSingleFieldPartitioning`
and
`TestSimpleKeyGenerator.testSlashSeparatedDatePartitioningOnRowWritingPaths`
already produce; the already-slashed input is a no-op `replace` duplicated by
`testSlashSeparatedDatePartitioningWithAlreadyFormattedInput` in
`TestSimpleKeyGenerator`, `TestComplexKeyGenerator`, `TestCustomKeyGenerator`
and by `TestSlashSeparatedPartitionValue.scala:175`.
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestPartitionPathFormatter.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.keygen;
+
+import org.apache.spark.unsafe.types.UTF8String;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static
org.apache.hudi.common.util.PartitionPathEncodeUtils.DEFAULT_PARTITION_PATH;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Tests the partition-path formatters backing the key generators, making sure
that both the
+ * {@link String} (Avro/{@link org.apache.spark.sql.Row} write path) and the
{@link UTF8String}
+ * (row-writer/{@link org.apache.spark.sql.catalyst.InternalRow} write path)
flavors produce
+ * identical partition paths.
+ */
+class TestPartitionPathFormatter {
+
+ private static final List<String> SINGLE_FIELD =
Collections.singletonList("date_col");
+ private static final List<String> TWO_FIELDS = Arrays.asList("date_col",
"city");
+
+ private String combine(boolean useRowWriterPath,
+ boolean hiveStylePartitioning,
+ boolean encode,
+ boolean slashSeparatedDatePartitioning,
+ List<String> fields,
+ Object... parts) {
+ if (useRowWriterPath) {
+ return new UTF8StringPartitionPathFormatter(
+ UTF8StringPartitionPathFormatter.UTF8StringBuilder::new,
hiveStylePartitioning, encode,
+ slashSeparatedDatePartitioning).combine(fields, parts).toString();
+ }
+ return new StringPartitionPathFormatter(
+ StringPartitionPathFormatter.JavaStringBuilder::new,
hiveStylePartitioning, encode,
+ slashSeparatedDatePartitioning).combine(fields, parts);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testSlashSeparatedDatePartitioningSingleField(boolean useRowWriterPath)
{
+ assertEquals("2026/01/05",
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD,
"2026-01-05"));
+ // Input that is already slash-separated is left untouched
+ assertEquals("2026/01/05",
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD,
"2026/01/05"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void
testSlashSeparatedDatePartitioningOnlyAppliesToSingleFieldPartitioning(boolean
useRowWriterPath) {
+ // NOTE: This mirrors [[KeyGenUtils#getRecordPartitionPath]] driving the
Avro write-path
+ assertEquals("2026-01-05/san-francisco",
+ combine(useRowWriterPath, false, false, true, TWO_FIELDS,
"2026-01-05", "san-francisco"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testSlashSeparatedDatePartitioningHandlesNullAndEmptyValues(boolean
useRowWriterPath) {
+ assertEquals(DEFAULT_PARTITION_PATH,
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD, new
Object[] {null}));
+ assertEquals(DEFAULT_PARTITION_PATH,
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD, ""));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testSlashSeparatedDatePartitioningEncodesValues(boolean
useRowWriterPath) {
+ // '?' has to be escaped, while the date separators are turned into
directory separators
+ assertEquals("2026/01/05", combine(useRowWriterPath, false, true, true,
SINGLE_FIELD, "2026-01-05"));
+ assertEquals("a%3Fb", combine(useRowWriterPath, false, true, true,
SINGLE_FIELD, "a?b"));
Review Comment:
Pre-PR the slash branch skipped `tryEncode`, so the one input whose output
actually changes under `urlencode=true` is an already slash-separated value
(`2026/01/05` -> `2026%2F01%2F05`, matching
`KeyGenUtils.getRecordPartitionPath:255-262` and fixing file-index prefix
pruning on such tables). Both asserts here are no-ops for that change; pin it:
```suggestion
assertEquals("a%3Fb", combine(useRowWriterPath, false, true, true,
SINGLE_FIELD, "a?b"));
// Encoding runs before the substitution (parity with KeyGenUtils), so
an already slash-separated
// value is escaped rather than turned into directories
assertEquals("2026%2F01%2F05", combine(useRowWriterPath, false, true,
true, SINGLE_FIELD, "2026/01/05"));
```
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestPartitionPathFormatter.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.keygen;
+
+import org.apache.spark.unsafe.types.UTF8String;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static
org.apache.hudi.common.util.PartitionPathEncodeUtils.DEFAULT_PARTITION_PATH;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Tests the partition-path formatters backing the key generators, making sure
that both the
+ * {@link String} (Avro/{@link org.apache.spark.sql.Row} write path) and the
{@link UTF8String}
+ * (row-writer/{@link org.apache.spark.sql.catalyst.InternalRow} write path)
flavors produce
+ * identical partition paths.
+ */
+class TestPartitionPathFormatter {
+
+ private static final List<String> SINGLE_FIELD =
Collections.singletonList("date_col");
+ private static final List<String> TWO_FIELDS = Arrays.asList("date_col",
"city");
+
+ private String combine(boolean useRowWriterPath,
+ boolean hiveStylePartitioning,
+ boolean encode,
+ boolean slashSeparatedDatePartitioning,
+ List<String> fields,
+ Object... parts) {
+ if (useRowWriterPath) {
+ return new UTF8StringPartitionPathFormatter(
+ UTF8StringPartitionPathFormatter.UTF8StringBuilder::new,
hiveStylePartitioning, encode,
+ slashSeparatedDatePartitioning).combine(fields, parts).toString();
+ }
+ return new StringPartitionPathFormatter(
+ StringPartitionPathFormatter.JavaStringBuilder::new,
hiveStylePartitioning, encode,
+ slashSeparatedDatePartitioning).combine(fields, parts);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testSlashSeparatedDatePartitioningSingleField(boolean useRowWriterPath)
{
+ assertEquals("2026/01/05",
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD,
"2026-01-05"));
+ // Input that is already slash-separated is left untouched
+ assertEquals("2026/01/05",
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD,
"2026/01/05"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void
testSlashSeparatedDatePartitioningOnlyAppliesToSingleFieldPartitioning(boolean
useRowWriterPath) {
+ // NOTE: This mirrors [[KeyGenUtils#getRecordPartitionPath]] driving the
Avro write-path
+ assertEquals("2026-01-05/san-francisco",
+ combine(useRowWriterPath, false, false, true, TWO_FIELDS,
"2026-01-05", "san-francisco"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testSlashSeparatedDatePartitioningHandlesNullAndEmptyValues(boolean
useRowWriterPath) {
+ assertEquals(DEFAULT_PARTITION_PATH,
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD, new
Object[] {null}));
+ assertEquals(DEFAULT_PARTITION_PATH,
+ combine(useRowWriterPath, false, false, true, SINGLE_FIELD, ""));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testSlashSeparatedDatePartitioningEncodesValues(boolean
useRowWriterPath) {
+ // '?' has to be escaped, while the date separators are turned into
directory separators
+ assertEquals("2026/01/05", combine(useRowWriterPath, false, true, true,
SINGLE_FIELD, "2026-01-05"));
+ assertEquals("a%3Fb", combine(useRowWriterPath, false, true, true,
SINGLE_FIELD, "a?b"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testHiveStylePartitioningTakesPrecedence(boolean useRowWriterPath) {
+ // NOTE: Hive-style partitioning and slash-separated date partitioning are
mutually exclusive --
+ // [[HoodieCatalogTable#extraTableConfig]] rejects a table
configuring both -- so this
+ // combination is unreachable and the formatter deliberately leaves
the value alone.
+ // This asserts the pre-existing behavior stays put, it is not a
statement about what the
+ // combination *should* produce
+ assertEquals("date_col=2026-01-05",
+ combine(useRowWriterPath, true, false, true, SINGLE_FIELD,
"2026-01-05"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testPlainPartitioningIsUnaffected(boolean useRowWriterPath) {
Review Comment:
Delete: cannot fail on master, and both asserts are already pinned at Row
and InternalRow level by `TestComplexKeyGenerator.testSingleValueKeyGenerator`
(`:148/:151`) and `testMultipleValueKeyGenerator` (`:183/:187`).
##########
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestPartitionPathFormatter.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hudi.keygen;
+
+import org.apache.spark.unsafe.types.UTF8String;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static
org.apache.hudi.common.util.PartitionPathEncodeUtils.DEFAULT_PARTITION_PATH;
Review Comment:
nit, feel free to ignore: sibling tests in this package use
`KeyGenUtils.HUDI_DEFAULT_PARTITION_PATH` (same constant,
`KeyGenUtils.java:55`).
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/common/TestSlashSeparatedPartitionValue.scala:
##########
@@ -90,6 +90,61 @@ class TestSlashSeparatedPartitionValue extends
HoodieSparkSqlTestBase {
}
}
+ test("Test slash separated date partitions written through the row writer") {
+ withSQLConf("hoodie.sql.bulk.insert.enable" -> "true",
"hoodie.sql.insert.mode" -> "non-strict",
+ "hoodie.datasource.write.row.writer.enable" -> "true") {
Review Comment:
`hoodie.sql.bulk.insert.enable` and `hoodie.sql.insert.mode` are
`@Deprecated` / `deprecatedAfter("0.14.0")` (`DataSourceOptions.scala:569-584`)
and `row.writer.enable=true` is the default; the single modern config below
still routes to `bulkInsertAsRow` (verified red on master with the same CCE,
green here).
```suggestion
withSQLConf("hoodie.spark.sql.insert.into.operation" -> "bulk_insert") {
```
nit, feel free to ignore: this is also the third copy of the DDL +
metaClient block in this file -- folding into `"Test slash separated date
partitions"` with `Seq("insert", "bulk_insert").foreach` plus the null row
would drop ~40 lines.
--
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]
