[SPARK-42093][SQL] Move JavaTypeInference to AgnosticEncoders #39615
Conversation
# Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/AgnosticEncoder.scala
|
yea let's fix scala 2.13 first |
# Conflicts: # sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/JavaTypeInferenceSuite.scala
|
@cloud-fan @dongjoon-hyun can you take another look please? |
|
thanks, merging to master/3.4 (the PR was merged before 3.4 cut due to scala 2.13 issues)! |
### What changes were proposed in this pull request? This PR makes `JavaTypeInference` produce an `AgnosticEncoder`. The expression generation for these encoders is moved to `ScalaReflection`. ### Why are the changes needed? For the Spark Connect Scala Client we also want to be able to use Java Bean based results. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? I have added a lot of tests to `JavaTypeInferenceSuite`. Closes #39615 from hvanhovell/SPARK-42093. Authored-by: Herman van Hovell <herman@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 0d93bb2) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
### What changes were proposed in this pull request? This PR makes `JavaTypeInference` produce an `AgnosticEncoder`. The expression generation for these encoders is moved to `ScalaReflection`. ### Why are the changes needed? For the Spark Connect Scala Client we also want to be able to use Java Bean based results. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? I have added a lot of tests to `JavaTypeInferenceSuite`. Closes apache#39615 from hvanhovell/SPARK-42093. Authored-by: Herman van Hovell <herman@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 0d93bb2) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
| Invoke(inputObject, "name", ObjectType(classOf[String]), returnNullable = false)) | ||
|
|
||
| case other => | ||
| val properties = getJavaBeanReadableAndWritableProperties(other) |
There was a problem hiding this comment.
Hmm, I'm not sure if this has been discussed, but this is a breaking change causing customer issue like https://issues.apache.org/jira/browse/SPARK-48073 when upgrading from Spark 3.2 to Spark 3.4.
There was a problem hiding this comment.
Is this an intentional change? @hvanhovell
There was a problem hiding this comment.
No matter what, this change has been there for a while. It seems difficult to change it back to old behavior. As discussed with @HeartSaVioR in the JIRA, I'd like to see if we can add a SQL conf to fall back to previous behavior of Encoder.bean().
There was a problem hiding this comment.
The behavior was apparently there before I got to it, this is PR that fixed it later: #42829
For the record there is no specification here, only tests.
There was a problem hiding this comment.
Thanks. After more internal discussion with the customer, we decided not to make a change to previous behavior with a config.
What changes were proposed in this pull request?
This PR makes
JavaTypeInferenceproduce anAgnosticEncoder. The expression generation for these encoders is moved toScalaReflection.Why are the changes needed?
For the Spark Connect Scala Client we also want to be able to use Java Bean based results.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
I have added a lot of tests to
JavaTypeInferenceSuite.