This is an automated email from the ASF dual-hosted git repository. dongjoon pushed a commit to branch branch-3.4 in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.4 by this push: new e5d82875463 [SPARK-44910][SQL][3.4] Encoders.bean does not support superclasses with generic type arguments e5d82875463 is described below commit e5d8287546306f31d3888662669970e46895c155 Author: Giambattista Bloisi <gblo...@gmail.com> AuthorDate: Mon Sep 18 21:38:42 2023 -0700 [SPARK-44910][SQL][3.4] 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 hvanhovell this is branch-3.4 port of [PR-42634](https://github.com/apache/spark/pull/42634) Closes #42914 from gbloisi-openaire/SPARK-44910-branch-3.4. Authored-by: Giambattista Bloisi <gblo...@gmail.com> Signed-off-by: Dongjoon Hyun <dh...@apple.com> --- .../spark/sql/catalyst/JavaTypeInference.scala | 6 ++- ...thGenerics.java => JavaTypeInferenceBeans.java} | 52 +++++++++++++++++++--- .../sql/catalyst/JavaTypeInferenceSuite.scala | 41 +++++++++++++++-- 3 files changed, 88 insertions(+), 11 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala index 75aca3ccbdd..a0341c0d9c8 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala @@ -131,10 +131,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( @@ -158,4 +161,3 @@ object JavaTypeInference { .filter(_.getReadMethod != null) } } - 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 old mode 100755 new mode 100644 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..8438b75c762 --- 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,65 @@ 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..c9c3bf08999 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