Skip to content

Commit f7c2cc4

Browse files
committed
[SPARK-56164][SQL] Fix SPJ merged key ordering
### What changes were proposed in this pull request? Fix a bug in `EnsureRequirements` where the ordering used to merge and dedup partition keys after reduction was based on the original partition key types rather than the reduced types. When compatible partition transform reducers are applied (e.g. reducing days keys to years keys), the resulting partition key values may have a different `DataType` than the originals. Please note that #54884 fixed the issue when left and right side reduced key types are not equal, but this PR fixes the issue when the common reduced types differ to the left side original types. ### Why are the changes needed? Without this fix, when reducers change the data type of partition keys (e.g. `DateType` → `LongType`), the ordering used to merge partition values is built for the wrong type, which can produce incorrect merge results or runtime failures during storage-partitioned joins with compatible transform reducers. ### Does this PR introduce any user-facing change? This PR fixes a theoretical bug, a real world example is unlikely to exist. ### How was this patch tested? Added a new test case. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Sonnet 4.6 Closes #54961 from peter-toth/SPARK-56164-fix-spj-merged-key-ordering. Authored-by: Peter Toth <peter.toth@gmail.com> Signed-off-by: Peter Toth <peter.toth@gmail.com>
1 parent aa10a79 commit f7c2cc4

3 files changed

Lines changed: 154 additions & 35 deletions

File tree

sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/EnsureRequirements.scala

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -517,17 +517,24 @@ case class EnsureRequirements(
517517
val (rightReducedDataTypes, rightReducedKeys) = rightReducers.fold(
518518
(rightPartitioning.expressionDataTypes, rightPartitioning.partitionKeys)
519519
)(rightPartitioning.reduceKeys)
520-
if (leftReducedDataTypes != rightReducedDataTypes) {
520+
val reducedDataTypes = if (leftReducedDataTypes == rightReducedDataTypes) {
521+
leftReducedDataTypes
522+
} else {
521523
throw QueryExecutionErrors.storagePartitionJoinIncompatibleReducedTypesError(
522524
leftReducers = leftReducers,
523525
leftReducedDataTypes = leftReducedDataTypes,
524526
rightReducers = rightReducers,
525527
rightReducedDataTypes = rightReducedDataTypes)
526528
}
527529

530+
val reducedKeyRowOrdering = RowOrdering.createNaturalAscendingOrdering(reducedDataTypes)
531+
val reducedKeyOrdering =
532+
reducedKeyRowOrdering.on((t: InternalRowComparableWrapper) => t.row)
533+
528534
// merge values on both sides
529-
var mergedPartitionKeys = mergeAndDedupPartitions(leftReducedKeys, rightReducedKeys,
530-
joinType, leftPartitioning.keyOrdering).map((_, 1))
535+
var mergedPartitionKeys =
536+
mergeAndDedupPartitions(leftReducedKeys, rightReducedKeys, joinType, reducedKeyOrdering)
537+
.map((_, 1))
531538

532539
logInfo(log"After merging, there are " +
533540
log"${MDC(LogKeys.NUM_PARTITIONS, mergedPartitionKeys.size)} partitions")

sql/core/src/test/scala/org/apache/spark/sql/connector/KeyGroupedPartitioningSuite.scala

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,23 @@ class KeyGroupedPartitioningSuite extends DistributionAndOrderingSuiteBase with
7575
Column.create("dept_id", IntegerType),
7676
Column.create("data", StringType))
7777

78-
def withFunction[T](fn: UnboundFunction)(f: => T): T = {
79-
val id = Identifier.of(Array.empty, fn.name())
80-
val oldFn = Option.when(catalog.listFunctions(Array.empty).contains(id)) {
81-
val fn = catalog.loadFunction(id)
82-
catalog.dropFunction(id)
83-
fn
84-
}
85-
catalog.createFunction(id, fn)
78+
def withFunction[T](fns: UnboundFunction*)(f: => T): T = {
79+
val fnIds = catalog.listFunctions(Array.empty)
80+
val oldFns = fns.map { fn =>
81+
val id = Identifier.of(Array.empty, fn.name())
82+
val oldFn = Option.when(fnIds.contains(id)) {
83+
val fn = catalog.loadFunction(id)
84+
catalog.dropFunction(id)
85+
fn
86+
}
87+
catalog.createFunction(id, fn)
88+
(id, oldFn)
89+
}
8690
try f finally {
87-
catalog.dropFunction(id)
88-
oldFn.foreach(catalog.createFunction(id, _))
91+
oldFns.foreach { case (id, oldFn) =>
92+
catalog.dropFunction(id)
93+
oldFn.foreach(catalog.createFunction(id, _))
94+
}
8995
}
9096
}
9197

@@ -3441,7 +3447,7 @@ class KeyGroupedPartitioningSuite extends DistributionAndOrderingSuiteBase with
34413447
}
34423448

34433449
test("SPARK-56046: Reducers with different result types") {
3444-
withFunction(UnboundDaysFunctionWithIncompatibleResultTypeReducer) {
3450+
withFunction(UnboundDaysFunctionWithToYearsReducerWithDateResult) {
34453451
val items_partitions = Array(days("arrive_time"))
34463452
createTable(items, itemsColumns, items_partitions)
34473453
sql(s"INSERT INTO testcat.ns.$items VALUES " +
@@ -3478,4 +3484,48 @@ class KeyGroupedPartitioningSuite extends DistributionAndOrderingSuiteBase with
34783484
}
34793485
}
34803486
}
3487+
3488+
test("SPARK-56164: Reducers with different result types to original keys") {
3489+
withFunction(
3490+
UnboundDaysFunctionWithToYearsReducerWithLongResult,
3491+
UnboundYearsFunctionWithToYearsReducerWithLongResult) {
3492+
val items_partitions = Array(days("arrive_time"))
3493+
createTable(items, itemsColumns, items_partitions)
3494+
sql(s"INSERT INTO testcat.ns.$items VALUES " +
3495+
s"(0, 'aa', 39.0, cast('2020-01-01' as timestamp)), " +
3496+
s"(1, 'aa', 40.0, cast('2020-01-01' as timestamp)), " +
3497+
s"(2, 'bb', 41.0, cast('2021-01-03' as timestamp)), " +
3498+
s"(3, 'bb', 42.0, cast('2021-01-04' as timestamp))")
3499+
3500+
val purchases_partitions = Array(years("time"))
3501+
createTable(purchases, purchasesColumns, purchases_partitions)
3502+
sql(s"INSERT INTO testcat.ns.$purchases VALUES " +
3503+
s"(1, 42.0, cast('2020-01-01' as timestamp)), " +
3504+
s"(5, 44.0, cast('2020-01-15' as timestamp)), " +
3505+
s"(7, 46.5, cast('2021-02-08' as timestamp))")
3506+
3507+
withSQLConf(
3508+
SQLConf.V2_BUCKETING_PUSH_PART_VALUES_ENABLED.key -> "true",
3509+
SQLConf.V2_BUCKETING_ALLOW_COMPATIBLE_TRANSFORMS.key -> "true") {
3510+
Seq(
3511+
s"testcat.ns.$items i JOIN testcat.ns.$purchases p ON p.time = i.arrive_time",
3512+
s"testcat.ns.$purchases p JOIN testcat.ns.$items i ON i.arrive_time = p.time"
3513+
).foreach { joinString =>
3514+
val df = sql(
3515+
s"""
3516+
|${selectWithMergeJoinHint("i", "p")} id, item_id
3517+
|FROM $joinString
3518+
|ORDER BY id, item_id
3519+
|""".stripMargin)
3520+
3521+
val shuffles = collectShuffles(df.queryExecution.executedPlan)
3522+
assert(shuffles.isEmpty, "should not add shuffle for both sides of the join")
3523+
val groupPartitions = collectGroupPartitions(df.queryExecution.executedPlan)
3524+
assert(groupPartitions.forall(_.outputPartitioning.numPartitions == 2))
3525+
3526+
checkAnswer(df, Seq(Row(0, 1), Row(1, 1)))
3527+
}
3528+
}
3529+
}
3530+
}
34813531
}

sql/core/src/test/scala/org/apache/spark/sql/connector/catalog/functions/transformFunctions.scala

Lines changed: 83 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,34 @@ import org.apache.spark.sql.catalyst.util.DateTimeUtils
2424
import org.apache.spark.sql.types._
2525
import org.apache.spark.unsafe.types.UTF8String
2626

27-
object UnboundYearsFunction extends UnboundFunction {
27+
abstract class UnboundYearsFunctionBase extends UnboundFunction {
28+
protected def isValidType(dt: DataType): Boolean = dt match {
29+
case DateType | TimestampType => true
30+
case _ => false
31+
}
32+
33+
override def description(): String = name()
34+
override def name(): String = "years"
35+
}
36+
37+
object UnboundYearsFunction extends UnboundYearsFunctionBase {
2838
override def bind(inputType: StructType): BoundFunction = {
2939
if (inputType.size == 1 && isValidType(inputType.head.dataType)) YearsFunction
3040
else throw new UnsupportedOperationException(
3141
"'years' only take date or timestamp as input type")
3242
}
43+
}
3344

34-
private def isValidType(dt: DataType): Boolean = dt match {
35-
case DateType | TimestampType => true
36-
case _ => false
45+
object UnboundYearsFunctionWithToYearsReducerWithLongResult extends UnboundYearsFunctionBase {
46+
override def bind(inputType: StructType): BoundFunction = {
47+
if (inputType.size == 1 && isValidType(inputType.head.dataType)) {
48+
YearsFunctionWithToYearsReducerWithLongResult
49+
} else throw new UnsupportedOperationException(
50+
"'years' only take date or timestamp as input type")
3751
}
38-
39-
override def description(): String = name()
40-
override def name(): String = "years"
4152
}
4253

43-
object YearsFunction extends ScalarFunction[Int] with ReducibleFunction[Int, Int] {
54+
abstract class YearsFunctionBase[O] extends ScalarFunction[Int] with ReducibleFunction[Int, O] {
4455
override def inputTypes(): Array[DataType] = Array(TimestampType)
4556
override def resultType(): DataType = IntegerType
4657
override def name(): String = "years"
@@ -49,14 +60,37 @@ object YearsFunction extends ScalarFunction[Int] with ReducibleFunction[Int, Int
4960
val UTC: ZoneId = ZoneId.of("UTC")
5061
val EPOCH_LOCAL_DATE: LocalDate = Instant.EPOCH.atZone(UTC).toLocalDate
5162

52-
def invoke(ts: Long): Int = {
63+
protected def doInvoke(ts: Long): Long = {
5364
val localDate = DateTimeUtils.microsToInstant(ts).atZone(UTC).toLocalDate
54-
ChronoUnit.YEARS.between(EPOCH_LOCAL_DATE, localDate).toInt
65+
ChronoUnit.YEARS.between(EPOCH_LOCAL_DATE, localDate)
5566
}
67+
}
5668

69+
// This `years` function reduces `IntegerType` partition keys to `IntegerType` partition keys when
70+
// partitions are reduced to partitions of a `days` function, which produces `DateType` keys.
71+
object YearsFunction extends YearsFunctionBase[Int] {
72+
def invoke(ts: Long): Int = doInvoke(ts).toInt
5773
override def reducer(otherFunction: ReducibleFunction[_, _]): Reducer[Int, Int] = null
5874
}
5975

76+
// This `years` function reduces `IntegerType` partition keys to `LongType` partition keys when
77+
// partitions are reduced to partitions of a `days` function, which produces `DateType` keys.
78+
object YearsFunctionWithToYearsReducerWithLongResult extends YearsFunctionBase[Long] {
79+
def invoke(ts: Long): Int = doInvoke(ts).toInt
80+
override def reducer(otherFunction: ReducibleFunction[_, _]): Reducer[Int, Long] = {
81+
if (otherFunction == DaysFunctionWithToYearsReducerWithLongResult) {
82+
YearsToYearsReducerWithLongResult()
83+
} else {
84+
null
85+
}
86+
}
87+
}
88+
89+
case class YearsToYearsReducerWithLongResult() extends Reducer[Int, Long] {
90+
override def resultType(): DataType = LongType
91+
override def reduce(days: Int): Long = days.toLong
92+
}
93+
6094
abstract class UnboundDaysFunctionBase extends UnboundFunction {
6195
protected def isValidType(dt: DataType): Boolean = dt match {
6296
case DateType | TimestampType => true
@@ -75,16 +109,25 @@ object UnboundDaysFunction extends UnboundDaysFunctionBase {
75109
}
76110
}
77111

78-
object UnboundDaysFunctionWithIncompatibleResultTypeReducer extends UnboundDaysFunctionBase {
112+
object UnboundDaysFunctionWithToYearsReducerWithDateResult extends UnboundDaysFunctionBase {
113+
override def bind(inputType: StructType): BoundFunction = {
114+
if (inputType.size == 1 && isValidType(inputType.head.dataType)) {
115+
DaysFunctionWithToYearsReducerWithDateResult
116+
} else throw new UnsupportedOperationException(
117+
"'days' only take date or timestamp as input type")
118+
}
119+
}
120+
121+
object UnboundDaysFunctionWithToYearsReducerWithLongResult extends UnboundDaysFunctionBase {
79122
override def bind(inputType: StructType): BoundFunction = {
80123
if (inputType.size == 1 && isValidType(inputType.head.dataType)) {
81-
DaysFunctionWithIncompatibleResultTypeReducer
124+
DaysFunctionWithToYearsReducerWithLongResult
82125
} else throw new UnsupportedOperationException(
83126
"'days' only take date or timestamp as input type")
84127
}
85128
}
86129

87-
abstract class DaysFunctionBase extends ScalarFunction[Int] with ReducibleFunction[Int, Int] {
130+
abstract class DaysFunctionBase[O] extends ScalarFunction[Int] with ReducibleFunction[Int, O] {
88131
override def inputTypes(): Array[DataType] = Array(TimestampType)
89132
override def resultType(): DataType = DateType
90133
override def name(): String = "days"
@@ -93,7 +136,7 @@ abstract class DaysFunctionBase extends ScalarFunction[Int] with ReducibleFuncti
93136

94137
// This `days` function reduces `DateType` partition keys to `IntegerType` partition keys when
95138
// partitions are reduced to partitions of a `years` function, which produces `IntegerType` keys.
96-
object DaysFunction extends DaysFunctionBase {
139+
object DaysFunction extends DaysFunctionBase[Int] {
97140
override def reducer(otherFunc: ReducibleFunction[_, _]): Reducer[Int, Int] = {
98141
if (otherFunc == YearsFunction) {
99142
DaysToYearsReducer()
@@ -105,32 +148,51 @@ object DaysFunction extends DaysFunctionBase {
105148

106149
// This `days` function reduces `DateType` partition keys to `DateType` partition keys when
107150
// partitions are reduced to partitions of a `years` function, which produces `IntegerType` keys.
108-
object DaysFunctionWithIncompatibleResultTypeReducer extends DaysFunctionBase {
151+
object DaysFunctionWithToYearsReducerWithDateResult extends DaysFunctionBase[Int] {
109152
override def reducer(otherFunc: ReducibleFunction[_, _]): Reducer[Int, Int] = {
110153
if (otherFunc == YearsFunction) {
111-
DaysToYearsReducerWithIncompatibleResultType()
154+
DaysToYearsReducerWithDateResult()
112155
} else {
113156
null
114157
}
115158
}
116159
}
117160

118-
abstract class DaysToYearsReducerBase extends Reducer[Int, Int] {
161+
// This `days` function reduces `DateType` partition keys to `LongType` partition keys when
162+
// partitions are reduced to partitions of a `years` function, which produces `IntegerType` keys.
163+
object DaysFunctionWithToYearsReducerWithLongResult extends DaysFunctionBase[Long] {
164+
override def reducer(otherFunc: ReducibleFunction[_, _]): Reducer[Int, Long] = {
165+
if (otherFunc == YearsFunctionWithToYearsReducerWithLongResult) {
166+
DaysToYearsReducerWithLongResult()
167+
} else {
168+
null
169+
}
170+
}
171+
}
172+
173+
abstract class DaysToYearsReducerBase[O] extends Reducer[Int, O] {
119174
val UTC: ZoneId = ZoneId.of("UTC")
120175
val EPOCH_LOCAL_DATE: LocalDate = Instant.EPOCH.atZone(UTC).toLocalDate
121176

122-
override def reduce(days: Int): Int = {
177+
protected def doReduce(days: Int): Long = {
123178
val localDate = EPOCH_LOCAL_DATE.plusDays(days)
124-
ChronoUnit.YEARS.between(EPOCH_LOCAL_DATE, localDate).toInt
179+
ChronoUnit.YEARS.between(EPOCH_LOCAL_DATE, localDate)
125180
}
126181
}
127182

128-
case class DaysToYearsReducer() extends DaysToYearsReducerBase {
183+
case class DaysToYearsReducer() extends DaysToYearsReducerBase[Int] {
129184
override def resultType(): DataType = IntegerType
185+
override def reduce(days: Int): Int = doReduce(days).toInt
130186
}
131187

132-
case class DaysToYearsReducerWithIncompatibleResultType() extends DaysToYearsReducerBase {
188+
case class DaysToYearsReducerWithDateResult() extends DaysToYearsReducerBase[Int] {
133189
override def resultType(): DataType = DateType
190+
override def reduce(days: Int): Int = doReduce(days).toInt
191+
}
192+
193+
case class DaysToYearsReducerWithLongResult() extends DaysToYearsReducerBase[Long] {
194+
override def resultType(): DataType = LongType
195+
override def reduce(days: Int): Long = doReduce(days)
134196
}
135197

136198
object UnboundBucketFunction extends UnboundFunction {

0 commit comments

Comments
 (0)