vinishjail97 commented on code in PR #828:
URL: https://github.com/apache/incubator-xtable/pull/828#discussion_r3454808480
##########
xtable-core/src/main/java/org/apache/xtable/delta/DeltaPartitionExtractor.java:
##########
@@ -326,8 +326,14 @@ private String getSerializedPartitionValue(
if (partitionFieldNames.size() == 1) {
return values.getOrDefault(partitionFieldNames.get(0), null);
}
+ // Composite partition (e.g. year and month generated columns derived from
a single source
+ // field). If any component value is missing the partition value cannot be
reconstructed, so
+ // return null instead of joining into a string containing the literal
"null".
+ if (partitionFieldNames.stream().anyMatch(name -> values.get(name) ==
null)) {
Review Comment:
Checked the delta-spark source: null partition values are stored as `null`
in the AddFile `partitionValues` map, not `__HIVE_DEFAULT_PARTITION__`. The
sentinel only appears in the physical directory path
(`PartitionUtils.getTargetDirectory`); on write that path is parsed back to a
`null` literal via `literalToNormalizedString` (returns null when
`literal.value == null`), and the kernel returns `Literal.ofNull` for nulls. So
the `== null` check is correct here and no sentinel handling is needed. (The
map only holds the literal string for a String-typed column whose actual value
is `__HIVE_DEFAULT_PARTITION__`, which is correctly kept as-is — irrelevant to
these INT generated columns.)
##########
xtable-core/src/test/java/org/apache/xtable/kernel/TestDeltaKernelPartitionExtractor.java:
##########
@@ -513,6 +513,68 @@ public void
testYearMonthDayHourGeneratedPartitionValueExtraction() {
assertEquals(expectedPartitionValues, partitionValues);
}
+ @Test
+ public void testGeneratedPartitionValueExtractionWithNullSource() {
+ // A composite (generated column) partition is derived from a single
source column (e.g. year,
+ // month and day generated from one timestamp column). When the source
value is null all of the
+ // derived component values are null in the Delta partition values. The
partition value must
+ // resolve to null rather than a string containing the literal "null"
(e.g. "null-null-null")
+ // which would then fail date parsing.
+ Map<String, String> partitionValuesMap =
+ new HashMap<String, String>() {
+ {
+ put("partition_column1", "partition_value1");
+ put("year_partition_column", null);
+ put("month_partition_column", null);
+ put("day_partition_column", null);
+ }
+ };
+ java.util.Map<String, String> scalaMap = partitionValuesMap;
Review Comment:
Done in 0cbdc0d2 — dropped the misleading `scalaMap` alias and pass
`partitionValuesMap` directly.
--
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]
