Commit 595a802
committed
[SPARK-56046][SQL] Typed SPJ partition key
### What changes were proposed in this pull request?
This PR adds a new method to SPJ partition key `Reducer`s to return the type of a reduced partition key.
### Why are the changes needed?
After the [SPJ refactor](#54330) some Iceberg SPJ tests, that join a `hours` transform partitioned table with a `days` transform partitioned table, started to fail. This is because after the refactor the keys of a `KeyedPartitioning` partitioning are `InternalRowComparableWrapper`s, which include the type of the key, and when the partition keys are reduced the type of the reduced keys are inherited from their original type.
- #54330
This means that when `hours` transformed hour keys are reduced to days, the keys actually remain having `IntegerType` type, while the `days` transformed keys have `DateType` type in Iceberg. This type difference causes that the left and right side `InternalRowComparableWrapper`s are not considered equal despite their `InternalRow` raw key data are equal.
Before the refactor the type of (possibly reduced) partition keys were not stored in the partitioning. When the left and right side raw keys were compared in `EnsureRequirement` a common comparator was initialized with the type of the left side keys.
So in the Iceberg SPJ tests the `IntegerType` keys were forced to be interpreted as `DateType`, or the `DateType` keys were forced to be interpreted as `IntegerType`, depending on the join order of the tables.
The reason why this was not causing any issues is that the `PhysicalDataType` of both `DateType` and `IntegerType` logical types is `PhysicalIntegerType`.
This PR introduces a new `resultType()` method of `Reducer` to return the correct type of the reduced keys and properly compares the left and right side reduced key types and thorws an error when they are not the same.
### Does this PR introduce _any_ user-facing change?
Yes, the reduced key types are now properly compared and incompatibilities are reported to users.
### How was this patch tested?
Added new UTs.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #54884 from peter-toth/SPARK-56046-typed-spj-reducers.
Authored-by: Peter Toth <peter.toth@gmail.com>
Signed-off-by: Peter Toth <peter.toth@gmail.com>Reducers1 parent 3b1d078 commit 595a802
10 files changed
Lines changed: 253 additions & 45 deletions
File tree
- common/utils/src/main/resources/error
- sql
- catalyst/src
- main
- java/org/apache/spark/sql/connector/catalog/functions
- scala/org/apache/spark/sql
- catalyst/plans/physical
- errors
- test/scala/org/apache/spark/sql/connector/catalog
- core/src
- main/scala/org/apache/spark/sql/execution
- datasources/v2
- exchange
- test/scala/org/apache/spark/sql/connector
- catalog/functions
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6541 | 6541 | | |
6542 | 6542 | | |
6543 | 6543 | | |
| 6544 | + | |
| 6545 | + | |
| 6546 | + | |
| 6547 | + | |
| 6548 | + | |
| 6549 | + | |
| 6550 | + | |
| 6551 | + | |
6544 | 6552 | | |
6545 | 6553 | | |
6546 | 6554 | | |
| |||
Lines changed: 11 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
20 | 21 | | |
21 | 22 | | |
22 | 23 | | |
| |||
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
| 35 | + | |
34 | 36 | | |
35 | | - | |
36 | | - | |
| 37 | + | |
| 38 | + | |
37 | 39 | | |
38 | 40 | | |
39 | 41 | | |
| |||
47 | 49 | | |
48 | 50 | | |
49 | 51 | | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
50 | 59 | | |
Lines changed: 13 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
465 | 465 | | |
466 | 466 | | |
467 | 467 | | |
468 | | - | |
| 468 | + | |
469 | 469 | | |
470 | | - | |
471 | | - | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
472 | 473 | | |
473 | 474 | | |
474 | 475 | | |
| |||
586 | 587 | | |
587 | 588 | | |
588 | 589 | | |
589 | | - | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
590 | 595 | | |
591 | | - | |
592 | | - | |
| 596 | + | |
| 597 | + | |
593 | 598 | | |
594 | 599 | | |
595 | 600 | | |
596 | 601 | | |
597 | 602 | | |
598 | 603 | | |
599 | 604 | | |
| 605 | + | |
| 606 | + | |
600 | 607 | | |
601 | 608 | | |
602 | 609 | | |
| |||
Lines changed: 25 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| 50 | + | |
50 | 51 | | |
51 | 52 | | |
52 | 53 | | |
| |||
3128 | 3129 | | |
3129 | 3130 | | |
3130 | 3131 | | |
| 3132 | + | |
| 3133 | + | |
| 3134 | + | |
| 3135 | + | |
| 3136 | + | |
| 3137 | + | |
| 3138 | + | |
| 3139 | + | |
| 3140 | + | |
| 3141 | + | |
| 3142 | + | |
| 3143 | + | |
| 3144 | + | |
| 3145 | + | |
| 3146 | + | |
| 3147 | + | |
| 3148 | + | |
| 3149 | + | |
| 3150 | + | |
| 3151 | + | |
| 3152 | + | |
| 3153 | + | |
| 3154 | + | |
| 3155 | + | |
3131 | 3156 | | |
3132 | 3157 | | |
3133 | 3158 | | |
| |||
Lines changed: 3 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
203 | 203 | | |
204 | 204 | | |
205 | 205 | | |
206 | | - | |
| 206 | + | |
207 | 207 | | |
208 | 208 | | |
209 | | - | |
| 209 | + | |
210 | 210 | | |
211 | 211 | | |
212 | 212 | | |
| |||
225 | 225 | | |
226 | 226 | | |
227 | 227 | | |
228 | | - | |
| 228 | + | |
229 | 229 | | |
230 | 230 | | |
231 | 231 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
141 | 141 | | |
142 | 142 | | |
143 | 143 | | |
144 | | - | |
| 144 | + | |
145 | 145 | | |
146 | 146 | | |
147 | 147 | | |
148 | 148 | | |
149 | 149 | | |
150 | 150 | | |
151 | 151 | | |
152 | | - | |
| 152 | + | |
153 | 153 | | |
154 | 154 | | |
155 | 155 | | |
| |||
Lines changed: 26 additions & 16 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| 30 | + | |
30 | 31 | | |
31 | 32 | | |
32 | 33 | | |
| |||
509 | 510 | | |
510 | 511 | | |
511 | 512 | | |
512 | | - | |
513 | | - | |
514 | 513 | | |
515 | | - | |
516 | | - | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
517 | 527 | | |
518 | 528 | | |
519 | | - | |
520 | | - | |
521 | | - | |
| 529 | + | |
| 530 | + | |
522 | 531 | | |
523 | 532 | | |
524 | 533 | | |
| |||
752 | 761 | | |
753 | 762 | | |
754 | 763 | | |
755 | | - | |
| 764 | + | |
756 | 765 | | |
757 | 766 | | |
758 | 767 | | |
759 | 768 | | |
760 | | - | |
| 769 | + | |
761 | 770 | | |
762 | 771 | | |
763 | | - | |
| 772 | + | |
764 | 773 | | |
765 | 774 | | |
766 | 775 | | |
767 | 776 | | |
768 | 777 | | |
769 | 778 | | |
770 | | - | |
771 | | - | |
772 | | - | |
773 | | - | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
774 | 784 | | |
775 | 785 | | |
776 | | - | |
| 786 | + | |
777 | 787 | | |
778 | 788 | | |
779 | 789 | | |
780 | 790 | | |
781 | 791 | | |
782 | 792 | | |
783 | 793 | | |
784 | | - | |
| 794 | + | |
785 | 795 | | |
786 | 796 | | |
787 | 797 | | |
| |||
Lines changed: 95 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
78 | 92 | | |
79 | 93 | | |
80 | 94 | | |
| |||
88 | 102 | | |
89 | 103 | | |
90 | 104 | | |
91 | | - | |
| 105 | + | |
92 | 106 | | |
93 | 107 | | |
94 | 108 | | |
| |||
3385 | 3399 | | |
3386 | 3400 | | |
3387 | 3401 | | |
| 3402 | + | |
| 3403 | + | |
| 3404 | + | |
| 3405 | + | |
| 3406 | + | |
| 3407 | + | |
| 3408 | + | |
| 3409 | + | |
| 3410 | + | |
| 3411 | + | |
| 3412 | + | |
| 3413 | + | |
| 3414 | + | |
| 3415 | + | |
| 3416 | + | |
| 3417 | + | |
| 3418 | + | |
| 3419 | + | |
| 3420 | + | |
| 3421 | + | |
| 3422 | + | |
| 3423 | + | |
| 3424 | + | |
| 3425 | + | |
| 3426 | + | |
| 3427 | + | |
| 3428 | + | |
| 3429 | + | |
| 3430 | + | |
| 3431 | + | |
| 3432 | + | |
| 3433 | + | |
| 3434 | + | |
| 3435 | + | |
| 3436 | + | |
| 3437 | + | |
| 3438 | + | |
| 3439 | + | |
| 3440 | + | |
| 3441 | + | |
| 3442 | + | |
| 3443 | + | |
| 3444 | + | |
| 3445 | + | |
| 3446 | + | |
| 3447 | + | |
| 3448 | + | |
| 3449 | + | |
| 3450 | + | |
| 3451 | + | |
| 3452 | + | |
| 3453 | + | |
| 3454 | + | |
| 3455 | + | |
| 3456 | + | |
| 3457 | + | |
| 3458 | + | |
| 3459 | + | |
| 3460 | + | |
| 3461 | + | |
| 3462 | + | |
| 3463 | + | |
| 3464 | + | |
| 3465 | + | |
| 3466 | + | |
| 3467 | + | |
| 3468 | + | |
| 3469 | + | |
| 3470 | + | |
| 3471 | + | |
| 3472 | + | |
| 3473 | + | |
| 3474 | + | |
| 3475 | + | |
| 3476 | + | |
| 3477 | + | |
| 3478 | + | |
| 3479 | + | |
| 3480 | + | |
3388 | 3481 | | |
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1174 | 1174 | | |
1175 | 1175 | | |
1176 | 1176 | | |
1177 | | - | |
| 1177 | + | |
1178 | 1178 | | |
1179 | 1179 | | |
1180 | 1180 | | |
| |||
0 commit comments