[SPARK-44702][SQL] Fix ExpressionEncoder derivation for case class fi… - #58863
Open
Yuvraj2172 wants to merge 1 commit into
Open
Yuvraj2172 wants to merge 1 commit into
Yuvraj2172 wants to merge 1 commit into
Conversation
…elds tagged with a trait ### What changes were proposed in this pull request? `ScalaReflection.getConstructorParameters` crashed with a `MatchError` when a case class field's type was a compound type formed with `with` (e.g. `case class Bar(x: Foo with Tag)`), since such a type is represented as a `RefinedType` in Scala reflection, not a `TypeRef`. This extends the pattern match to handle that case, and also fixes a second issue in `constructParams`: a `RefinedType` has no constructor of its own, so member lookup needs to fall back to the type's erasure (which collapses it to its single concrete class parent) to find the real constructor. ### Why are the changes needed? This is a real, reproducible crash reported in the JIRA: ```scala trait Tag case class Foo(x: Int) case class Bar(x: Foo with Tag) ExpressionEncoder.apply[Bar]() ``` throws `scala.MatchError: Foo with Tag (of class ...RefinedType0)`. Trait tags like this are a common pattern for adding compile-time-only type safety without extra runtime fields, and there's no reason encoder derivation should fail on them. ### Does this PR introduce any user-facing change? Yes. Case classes with fields typed as `SomeType with SomeTrait` can now have an `ExpressionEncoder` derived for them successfully, instead of throwing a `MatchError`. ### How was this patch tested? Added a new test to `ScalaReflectionSuite` reproducing the JIRA's example (a case class field tagged with a marker trait) and asserting the derived schema is correct. All 38 tests in the suite pass, including the existing generic-type, value-class, and trait-without-companion edge cases, confirming no regressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…elds tagged with a trait
What changes were proposed in this pull request?
ScalaReflection.getConstructorParameterscrashed with aMatchErrorwhen a case class field's type was a compound type formed withwith(e.g.case class Bar(x: Foo with Tag)), since such a type is represented as aRefinedTypein Scala reflection, not aTypeRef. This extends the pattern match to handle that case, and also fixes a second issue inconstructParams: aRefinedTypehas no constructor of its own, so member lookup needs to fall back to the type's erasure (which collapses it to its single concrete class parent) to find the real constructor.Why are the changes needed?
This is a real, reproducible crash reported in the JIRA:
throws
scala.MatchError: Foo with Tag (of class ...RefinedType0). Trait tags like this are a common pattern for adding compile-time-only type safety without extra runtime fields, and there's no reason encoder derivation should fail on them.Does this PR introduce any user-facing change? Yes. Case classes with fields typed as
SomeType with SomeTraitcan now have anExpressionEncoderderived for them successfully, instead of throwing aMatchError.How was this patch tested?
Added a new test to
ScalaReflectionSuitereproducing the JIRA's example (a case class field tagged with a marker trait) and asserting the derived schema is correct. All 38 tests in the suite pass, including the existing generic-type, value-class, and trait-without-companion edge cases, confirming no regressions.