This is an automated email from the ASF dual-hosted git repository. dongjoon pushed a commit to branch branch-3.5 in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.5 by this push: new 6a498087361 [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments 6a498087361 is described below commit 6a498087361ecbd653821fc283b9ea0fa703c820 Author: Giambattista Bloisi <gblo...@gmail.com> AuthorDate: Mon Sep 18 21:37:09 2023 -0700 [SPARK-44910][SQL] Encoders.bean does not support superclasses with generic type arguments ### 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 Closes #42634 from gbloisi-openaire/SPARK-44910. Lead-authored-by: Giambattista Bloisi <gblo...@gmail.com> Co-authored-by: gbloisi-openaire <141144100+gbloisi-opena...@users.noreply.github.com> Signed-off-by: Dongjoon Hyun <dh...@apple.com> (cherry picked from commit 7e14c8cc33f0ed0a9c53a888e8a3b17dd2a5d493) Signed-off-by: Dongjoon Hyun <dh...@apple.com> --- .../spark/sql/catalyst/JavaTypeInference.scala | 5 ++- ...thGenerics.java => JavaTypeInferenceBeans.java} | 51 +++++++++++++++++++--- .../sql/catalyst/JavaTypeInferenceSuite.scala | 41 +++++++++++++++-- 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala index 3d536b735db..191ccc52544 100644 --- a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala +++ b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala @@ -130,10 +130,13 @@ object JavaTypeInference { // TODO: we should only collect properties that have getter and setter. However, some tests // pass in scala case class as java bean class which doesn't have getter and setter. val properties = getJavaBeanReadableProperties(c) + // add type variables from inheritance hierarchy of the class + val classTV = JavaTypeUtils.getTypeArguments(c, classOf[Object]).asScala.toMap ++ + typeVariables // Note that the fields are ordered by name. val fields = properties.map { property => val readMethod = property.getReadMethod - val encoder = encoderFor(readMethod.getGenericReturnType, seenTypeSet + c, typeVariables) + val encoder = encoderFor(readMethod.getGenericReturnType, seenTypeSet + c, classTV) // The existence of `javax.annotation.Nonnull`, means this field is not nullable. val hasNonNull = readMethod.isAnnotationPresent(classOf[Nonnull]) EncoderField( diff --git a/sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/JavaBeanWithGenerics.java b/sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/JavaTypeInferenceBeans.java similarity index 54% rename from sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/JavaBeanWithGenerics.java rename to sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/JavaTypeInferenceBeans.java index b84a3122cf8..cc3540717ee 100644 --- a/sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/JavaBeanWithGenerics.java +++ b/sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/JavaTypeInferenceBeans.java @@ -17,25 +17,66 @@ package org.apache.spark.sql.catalyst; -class JavaBeanWithGenerics<T,A> { +public class JavaTypeInferenceBeans { + + static class JavaBeanWithGenericsA<T> { + public T getPropertyA() { + return null; + } + + public void setPropertyA(T a) { + + } + } + + static class JavaBeanWithGenericsAB<T> extends JavaBeanWithGenericsA<String> { + public T getPropertyB() { + return null; + } + + public void setPropertyB(T a) { + + } + } + + static class JavaBeanWithGenericsABC<T> extends JavaBeanWithGenericsAB<Long> { + public T getPropertyC() { + return null; + } + + public void setPropertyC(T a) { + + } + } + + static class JavaBeanWithGenerics<T, A> { private A attribute; private T value; public A getAttribute() { - return attribute; + return attribute; } public void setAttribute(A attribute) { - this.attribute = attribute; + this.attribute = attribute; } public T getValue() { - return value; + return value; } public void setValue(T value) { - this.value = value; + this.value = value; } + } + + static class JavaBeanWithGenericBase extends JavaBeanWithGenerics<String, String> { + + } + + static class JavaBeanWithGenericHierarchy extends JavaBeanWithGenericsABC<Integer> { + + } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/JavaTypeInferenceSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/JavaTypeInferenceSuite.scala index 64399976097..f7c1043d1cb 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/JavaTypeInferenceSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/JavaTypeInferenceSuite.scala @@ -24,6 +24,7 @@ import scala.beans.{BeanProperty, BooleanBeanProperty} import scala.reflect.{classTag, ClassTag} import org.apache.spark.SparkFunSuite +import org.apache.spark.sql.catalyst.JavaTypeInferenceBeans.{JavaBeanWithGenericBase, JavaBeanWithGenericHierarchy, JavaBeanWithGenericsABC} import org.apache.spark.sql.catalyst.encoders.{AgnosticEncoder, UDTCaseClass, UDTForCaseClass} import org.apache.spark.sql.catalyst.encoders.AgnosticEncoders._ import org.apache.spark.sql.types.{DecimalType, MapType, Metadata, StringType, StructField, StructType} @@ -66,7 +67,8 @@ class LeafBean { @BeanProperty var period: java.time.Period = _ @BeanProperty var enum: java.time.Month = _ @BeanProperty val readOnlyString = "read-only" - @BeanProperty var genericNestedBean: JavaBeanWithGenerics[String, String] = _ + @BeanProperty var genericNestedBean: JavaBeanWithGenericBase = _ + @BeanProperty var genericNestedBean2: JavaBeanWithGenericsABC[Integer] = _ var nonNullString: String = "value" @javax.annotation.Nonnull @@ -186,8 +188,18 @@ class JavaTypeInferenceSuite extends SparkFunSuite { encoderField("duration", DayTimeIntervalEncoder), encoderField("enum", JavaEnumEncoder(classTag[java.time.Month])), encoderField("genericNestedBean", JavaBeanEncoder( - ClassTag(classOf[JavaBeanWithGenerics[String, String]]), - Seq(encoderField("attribute", StringEncoder), encoderField("value", StringEncoder)))), + ClassTag(classOf[JavaBeanWithGenericBase]), + Seq( + encoderField("attribute", StringEncoder), + encoderField("value", StringEncoder) + ))), + encoderField("genericNestedBean2", JavaBeanEncoder( + ClassTag(classOf[JavaBeanWithGenericsABC[Integer]]), + Seq( + encoderField("propertyA", StringEncoder), + encoderField("propertyB", BoxedLongEncoder), + encoderField("propertyC", BoxedIntEncoder) + ))), encoderField("instant", STRICT_INSTANT_ENCODER), encoderField("localDate", STRICT_LOCAL_DATE_ENCODER), encoderField("localDateTime", LocalDateTimeEncoder), @@ -224,4 +236,27 @@ class JavaTypeInferenceSuite extends SparkFunSuite { )) assert(encoder === expected) } + + test("SPARK-44910: resolve bean with generic base class") { + val encoder = + JavaTypeInference.encoderFor(classOf[JavaBeanWithGenericBase]) + val expected = + JavaBeanEncoder(ClassTag(classOf[JavaBeanWithGenericBase]), Seq( + encoderField("attribute", StringEncoder), + encoderField("value", StringEncoder) + )) + assert(encoder === expected) + } + + test("SPARK-44910: resolve bean with hierarchy of generic classes") { + val encoder = + JavaTypeInference.encoderFor(classOf[JavaBeanWithGenericHierarchy]) + val expected = + JavaBeanEncoder(ClassTag(classOf[JavaBeanWithGenericHierarchy]), Seq( + encoderField("propertyA", StringEncoder), + encoderField("propertyB", BoxedLongEncoder), + encoderField("propertyC", BoxedIntEncoder) + )) + assert(encoder === expected) + } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org