[SPARK-32214][SQL] The type conversion function generated in makeFromJava for "other" type uses a wrong variable. #29029
Conversation
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
| case udt: UserDefinedType[_] => makeFromJava(udt.sqlType) | ||
|
|
||
| case other => (obj: Any) => nullSafeConvert(other)(PartialFunction.empty) | ||
| case other => (obj: Any) => nullSafeConvert(obj)(PartialFunction.empty) |
Member
There was a problem hiding this comment.
btw, for any input, this converter in this pattern returns null? I mean we cannot write it here like this?
case other => (_: Any) => null
Member
Author
There was a problem hiding this comment.
btw, for any input, this converter in this pattern returns null?
Yes so it's difficult to test based on the return value.
Or, how about testing that the converter still returns null, meaning there is no regression?
HyukjinKwon
reviewed
Jul 8, 2020
|
Test build #125261 has finished for PR 29029 at commit
|
Member
|
I changed the PR description by myself. Merged to master, branch-3.0 and branch-2.4. |
HyukjinKwon
pushed a commit
that referenced
this pull request
Jul 8, 2020
…Java for "other" type uses a wrong variable ### What changes were proposed in this pull request? This PR fixes an inconsistency in `EvaluatePython.makeFromJava`, which creates a type conversion function for some Java/Scala types. `other` is a type but it should actually pass `obj`: ```scala case other => (obj: Any) => nullSafeConvert(other)(PartialFunction.empty) ``` This does not change the output because it always returns `null` for unsupported datatypes. ### Why are the changes needed? To make the codes coherent, and consistent. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? No behaviour change. Closes #29029 from sarutak/fix-makeFromJava. Authored-by: Kousuke Saruta <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit 371b35d) Signed-off-by: HyukjinKwon <[email protected]>
HyukjinKwon
pushed a commit
that referenced
this pull request
Jul 8, 2020
…Java for "other" type uses a wrong variable ### What changes were proposed in this pull request? This PR fixes an inconsistency in `EvaluatePython.makeFromJava`, which creates a type conversion function for some Java/Scala types. `other` is a type but it should actually pass `obj`: ```scala case other => (obj: Any) => nullSafeConvert(other)(PartialFunction.empty) ``` This does not change the output because it always returns `null` for unsupported datatypes. ### Why are the changes needed? To make the codes coherent, and consistent. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? No behaviour change. Closes #29029 from sarutak/fix-makeFromJava. Authored-by: Kousuke Saruta <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit 371b35d) Signed-off-by: HyukjinKwon <[email protected]>
Member
Author
|
@HyukjinKwon Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
What changes were proposed in this pull request?
This PR fixes an inconsistency in
EvaluatePython.makeFromJava, which creates a type conversion function for some Java/Scala types.otheris a type but it should actually passobj:This does not change the output because it always returns
nullfor unsupported datatypes.Why are the changes needed?
To make the codes coherent, and consistent.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
No behaviour change.