Skip to content

Commit 4c39501

Browse files
committed
[SPARK-57184][SQL] Recurse into CalendarInterval children when appending a null struct
### What changes were proposed in this pull request? Include `CalendarIntervalType` in the recursion guard of `WritableColumnVector.appendStruct(boolean isNull)`, so that appending a NULL parent struct cascades `appendStruct(true)` into a `CalendarInterval` child column and advances all three of its grandchild columns (months / days / microseconds). ```java for (WritableColumnVector c: childColumns) { if (c.type instanceof StructType || c.type instanceof VariantType || c.type instanceof CalendarIntervalType) { c.appendStruct(true); } else { c.appendNull(); } } ``` This was split out of the nanosecond-timestamp `ColumnVector` PR (SPARK-57100, #56198) per review, since it is an independent fix. ### Why are the changes needed? A `CalendarInterval` column is struct-shaped: it is backed by three grandchild primitive columns (`months` as int, `days` as int, `microseconds` as long). The recursion guard in `appendStruct` only handled `StructType` and `VariantType`, so an interval child column took the `else` branch (`c.appendNull()`), which advances only the interval column's own cursor and leaves its three grandchild cursors un-advanced. As a result, for a struct column with a `CalendarInterval` field, appending a NULL parent row left the interval's grandchild cursors behind by one. A subsequent non-null row then wrote its `months`/`days`/`microseconds` into the wrong (earlier) grandchild slots, and reading that row back returned a skewed value - silent data corruption for the nested struct-of-interval case. ### Does this PR introduce _any_ user-facing change? Yes. Reading back a struct-of-interval column that contains a NULL parent row followed by a non-null row now returns the correct interval value instead of a skewed one. Previously it returned corrupted data. ### How was this patch tested? Added a unit test to `ColumnarBatchSuite` that uses `RowToColumnConverter` to convert a null parent struct followed by non-null struct-of-interval rows, and verifies the interval values are read back correctly. The test fails without the fix and passes with it. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.8 Closes #56235 from MaxGekk/fix-appendstruct-interval. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Max Gekk <max.gekk@gmail.com> (cherry picked from commit 18f2bac) Signed-off-by: Max Gekk <max.gekk@gmail.com>
1 parent eb4b41e commit 4c39501

2 files changed

Lines changed: 32 additions & 0 deletions

File tree

sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/WritableColumnVector.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,7 @@ public final int appendStruct(boolean isNull) {
763763
elementsAppended++;
764764
for (WritableColumnVector c: childColumns) {
765765
if (c.type instanceof StructType || c.type instanceof VariantType
766+
|| c.type instanceof CalendarIntervalType
766767
|| c.type instanceof TimestampNTZNanosType
767768
|| c.type instanceof TimestampLTZNanosType) {
768769
c.appendStruct(true);

sql/core/src/test/scala/org/apache/spark/sql/execution/vectorized/ColumnarBatchSuite.scala

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1977,6 +1977,37 @@ class ColumnarBatchSuite extends SparkFunSuite {
19771977
}
19781978
}
19791979

1980+
test("SPARK-57184: appendStruct(true) recurses into CalendarInterval child columns") {
1981+
// A struct column whose only field is a CalendarInterval. When the parent struct is
1982+
// appended as null, the recursion must also advance the interval child's grandchild
1983+
// columns (months / days / microseconds). Otherwise a subsequent non-null row's interval
1984+
// would be read from skewed grandchild slots.
1985+
val schema = new StructType()
1986+
.add("s", new StructType().add("cal", CalendarIntervalType))
1987+
val converter = new RowToColumnConverter(schema)
1988+
val columns = OnHeapColumnVector.allocateColumns(3, schema)
1989+
try {
1990+
// row 0: null parent struct.
1991+
converter.convert(new GenericInternalRow(Array[Any](null)), columns.toArray)
1992+
// row 1: non-null struct holding interval (1, 2, 3).
1993+
converter.convert(
1994+
new GenericInternalRow(Array[Any](
1995+
new GenericInternalRow(Array[Any](new CalendarInterval(1, 2, 3))))),
1996+
columns.toArray)
1997+
// row 2: non-null struct holding interval (4, 5, 6).
1998+
converter.convert(
1999+
new GenericInternalRow(Array[Any](
2000+
new GenericInternalRow(Array[Any](new CalendarInterval(4, 5, 6))))),
2001+
columns.toArray)
2002+
2003+
assert(columns(0).isNullAt(0))
2004+
assert(columns(0).getStruct(1).getInterval(0) === new CalendarInterval(1, 2, 3))
2005+
assert(columns(0).getStruct(2).getInterval(0) === new CalendarInterval(4, 5, 6))
2006+
} finally {
2007+
columns.foreach(_.close())
2008+
}
2009+
}
2010+
19802011
testVector("Decimal API", 4, DecimalType.IntDecimal) {
19812012
column =>
19822013

0 commit comments

Comments
 (0)