You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "gbloisi-openaire (via GitHub)" <gi...@apache.org> on 2023/08/23 15:07:17 UTC
[GitHub] [spark] gbloisi-openaire opened a new pull request, #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
gbloisi-openaire opened a new pull request, #42634:
URL: https://github.com/apache/spark/pull/42634
### What changes were proposed in this pull request?
This pull request adds Encoders.bean support for beans having a superclass declared with generic type arguments. For example:
`class JavaBeanWithGenericsA<T> {
public T getPropertyA() {
return null;
}
public void setPropertyA(T a) {
}
}
class JavaBeanWithGenericBase extends JavaBeanWithGenericsA<String> {
}
Encoders.bean(JavaBeanWithGenericBase.class); // Exception
`
That feature had to be part of [PR 42327](https://github.com/apache/spark/commit/1f5d78b5952fcc6c7d36d3338a5594070e3a62dd) but was missing as I was focusing on nested beans only (@hvanhovell )
### Why are the changes needed?
JavaTypeInference.encoderFor did not solve TypeVariable objects for superclasses so when managing a case like in the example above an exception was thrown.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing tests have been extended, new specific tests have been added
### Was this patch authored or co-authored using generative AI tooling?
No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gbloisi-openaire commented on pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
Posted by "gbloisi-openaire (via GitHub)" <gi...@apache.org>.
gbloisi-openaire commented on PR #42634:
URL: https://github.com/apache/spark/pull/42634#issuecomment-1722306800
Since I was not able to re-run all tests successfully anymore (tried a few times) I had to merge with master to get a [green workflow run](https://github.com/gbloisi-openaire/spark/actions/runs/6207960488)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gbloisi-openaire commented on a diff in pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
Posted by "gbloisi-openaire (via GitHub)" <gi...@apache.org>.
gbloisi-openaire commented on code in PR #42634:
URL: https://github.com/apache/spark/pull/42634#discussion_r1315180662
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -156,4 +158,17 @@ object JavaTypeInference {
.filterNot(_.getName == "declaringClass")
.filter(_.getReadMethod != null)
}
+
+ @tailrec
+ def getClassHierarchyTypeArguments(cls: Class[_],
Review Comment:
At a second thought this recursive getClassHierarchyTypeArguments is not required as JavaTypeUtils.getTypeArguments(cls, classOf[Object]) already traverses the inheritance hierarchy till Object base class collecting type variables. So I just removed it.
To be clear what we are doing here is to collect type variable information about base class (which in turn could be derived from another base class, forming the hierarchy the requires to be traversed).
This is required for both provided class and nested beans that extend one or more generic base classes in their inheritance hierarchy (tests provided).
Interfaces are not required to be processed because they declare abstract methods only, so the definition of the method (and its actual return type) will come from the class implementing the interface.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gbloisi-openaire commented on pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
Posted by "gbloisi-openaire (via GitHub)" <gi...@apache.org>.
gbloisi-openaire commented on PR #42634:
URL: https://github.com/apache/spark/pull/42634#issuecomment-1718211140
> @gbloisi-openaire can you retrigger tests?
I was able to request re-run only on my forked branch [see here](https://github.com/gbloisi-openaire/spark/runs/16486058766): I'm not sure how I can force re-run of the tests to appear on this PR
I also prepared a branch 3.4 port of those changes. See [PR-42914](https://github.com/apache/spark/pull/42914)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on a diff in pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #42634:
URL: https://github.com/apache/spark/pull/42634#discussion_r1310650718
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -156,4 +158,17 @@ object JavaTypeInference {
.filterNot(_.getName == "declaringClass")
.filter(_.getReadMethod != null)
}
+
+ @tailrec
+ def getClassHierarchyTypeArguments(cls: Class[_],
Review Comment:
private? also can you document what we are doing here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
URL: https://github.com/apache/spark/pull/42634
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #42634:
URL: https://github.com/apache/spark/pull/42634#issuecomment-1724819818
Merged to master/3.5.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on pull request #42634: [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #42634:
URL: https://github.com/apache/spark/pull/42634#issuecomment-1715883809
@gbloisi-openaire can you retrigger tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org