Commit 1817e67
[SPARK-53524][CONNECT][SQL] Fix temporal value conversion in LiteralValueProtoConverter
### What changes were proposed in this pull request?
This PR fixes temporal value conversion issues in the `LiteralValueProtoConverter` for Spark Connect. The main changes include:
1. **Fixed temporal value conversion in `getConverter` method**: Updated the conversion logic for temporal data types (DATE, TIMESTAMP, TIMESTAMP_NTZ, DAY_TIME_INTERVAL, YEAR_MONTH_INTERVAL, TIME) to use proper utility methods from `SparkDateTimeUtils` and `SparkIntervalUtils` instead of directly returning raw protobuf values.
2. **Added comprehensive test coverage**: Extended the `PlanGenerationTestSuite` with a new test case that includes a tuple containing all temporal types to ensure proper conversion and serialization.
3. **Updated test expectations**: Modified the expected explain output and query test files to reflect the corrected temporal value handling.
### Why are the changes needed?
The struct type in typedlit doesn't work well with temporal values due to bugs in type conversions. For example, the code below fails:
```scala
import org.apache.spark.sql.functions.typedlit
spark.sql("select 1").select(typedlit((1, java.time.LocalDate.of(2020, 10, 10)))).collect()
"""
org.apache.spark.SparkIllegalArgumentException: The value (18545) of the type (java.lang.Integer) cannot be converted to the DATE type.
org.apache.spark.sql.catalyst.CatalystTypeConverters$DateConverter$.toCatalystImpl(CatalystTypeConverters.scala:356)
org.apache.spark.sql.catalyst.CatalystTypeConverters$DateConverter$.toCatalystImpl(CatalystTypeConverters.scala:347)
org.apache.spark.sql.catalyst.CatalystTypeConverters$CatalystTypeConverter.toCatalyst(CatalystTypeConverters.scala:110)
org.apache.spark.sql.catalyst.CatalystTypeConverters$StructConverter.toCatalystImpl(CatalystTypeConverters.scala:271)
org.apache.spark.sql.catalyst.CatalystTypeConverters$StructConverter.toCatalystImpl(CatalystTypeConverters.scala:251)
org.apache.spark.sql.catalyst.CatalystTypeConverters$CatalystTypeConverter.toCatalyst(CatalystTypeConverters.scala:110)
org.apache.spark.sql.catalyst.CatalystTypeConverters$.$anonfun$createToCatalystConverter$2(CatalystTypeConverters.scala:532)
org.apache.spark.sql.connect.planner.LiteralExpressionProtoConverter$.toCatalystExpression(LiteralExpressionProtoConverter.scala:116)
"""
```
### Does this PR introduce _any_ user-facing change?
**Yes.** This PR fixes temporal value conversion in LiteralValueProtoConverter, allowing the struct type in typedlit to work with temporal values.
### How was this patch tested?
`build/sbt "connect-client-jvm/testOnly org.apache.spark.sql.PlanGenerationTestSuite"`
`build/sbt "connect/testOnly org.apache.spark.sql.connect.ProtoToParsedPlanTestSuite"`
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor 1.5.11
Closes #52270 from heyihong/SPARK-53524.
Authored-by: Yihong He <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>1 parent d017779 commit 1817e67
File tree
5 files changed
+123
-10
lines changed- sql/connect
- client/jvm/src/test/scala/org/apache/spark/sql
- common/src
- main/scala/org/apache/spark/sql/connect/common
- test/resources/query-tests
- explain-results
- queries
5 files changed
+123
-10
lines changedLines changed: 11 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3404 | 3404 | | |
3405 | 3405 | | |
3406 | 3406 | | |
| 3407 | + | |
| 3408 | + | |
| 3409 | + | |
| 3410 | + | |
| 3411 | + | |
| 3412 | + | |
| 3413 | + | |
| 3414 | + | |
| 3415 | + | |
| 3416 | + | |
| 3417 | + | |
3407 | 3418 | | |
3408 | 3419 | | |
3409 | 3420 | | |
| |||
Lines changed: 16 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
476 | 476 | | |
477 | 477 | | |
478 | 478 | | |
479 | | - | |
| 479 | + | |
480 | 480 | | |
481 | 481 | | |
482 | 482 | | |
| |||
487 | 487 | | |
488 | 488 | | |
489 | 489 | | |
490 | | - | |
491 | | - | |
492 | | - | |
493 | | - | |
494 | | - | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
495 | 502 | | |
496 | 503 | | |
497 | 504 | | |
| |||
646 | 653 | | |
647 | 654 | | |
648 | 655 | | |
649 | | - | |
| 656 | + | |
650 | 657 | | |
651 | 658 | | |
652 | 659 | | |
| |||
691 | 698 | | |
692 | 699 | | |
693 | 700 | | |
694 | | - | |
| 701 | + | |
695 | 702 | | |
696 | 703 | | |
697 | 704 | | |
| |||
724 | 731 | | |
725 | 732 | | |
726 | 733 | | |
727 | | - | |
| 734 | + | |
728 | 735 | | |
729 | 736 | | |
730 | 737 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
Lines changed: 95 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
674 | 674 | | |
675 | 675 | | |
676 | 676 | | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
677 | 772 | | |
678 | 773 | | |
679 | 774 | | |
| |||
Binary file not shown.
0 commit comments