[spark] branch master updated: [SPARK-40385][SQL] Fix interpreted path for companion object constructor

[gu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gurwls223 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 73e3c36ec89 [SPARK-40385][SQL] Fix interpreted path for companion object constructor
73e3c36ec89 is described below

commit 73e3c36ec89242e4c586a5328e813310f82de728
Author: Emil Ejbyfeldt <ee...@liveintent.com>
AuthorDate: Thu Sep 22 11:46:35 2022 +0900

    [SPARK-40385][SQL] Fix interpreted path for companion object constructor
    
    ### What changes were proposed in this pull request?
    Fixes encoding of classes that uses companion object constructors in the interpreted path. Without this change the that is added in this change would fail with
    ```
    ...
      Cause: java.lang.RuntimeException: Error while decoding: java.lang.RuntimeException: Couldn't find a valid constructor on interface org.apache.spark.sql.catalyst.ScroogeLikeExample
    newInstance(interface org.apache.spark.sql.catalyst.ScroogeLikeExample)
      at org.apache.spark.sql.errors.QueryExecutionErrors$.expressionDecodingError(QueryExecutionErrors.scala:1199)
    ...
    ```
    
    As far as I can tell this bug has existed since the initial implementation in SPARK-8288 https://github.com/apache/spark/pull/23062
    
    The existing spec that tested this part of the code incorrectly provided an outerPointer which hid the bug from that test.
    
    ### Why are the changes needed?
    Fixes a bug, the new spec in the ExpressionsEncoderSuite shows that this is in fact a bug.
    
    ### Does this PR introduce _any_ user-facing change?
    Yes, it fixes a bug.
    
    ### How was this patch tested?
    New and existing specs in ExpressionEncoderSuite and ObjectExpressionsSuite.
    
    Closes #37837 from eejbyfeldt/spark-40385.
    
    Authored-by: Emil Ejbyfeldt <ee...@liveintent.com>
    Signed-off-by: Hyukjin Kwon <gu...@apache.org>
---
 .../main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala   | 4 ++--
 .../apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala  | 5 ++++-
 .../spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala      | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala
index 565774a5561..12093b9f4b2 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala
@@ -850,8 +850,8 @@ object ScalaReflection extends ScalaReflection {
         applyMethods.find { method =>
           val params = method.typeSignature.paramLists.head
           // Check that the needed params are the same length and of matching types
-          params.size == paramTypes.tail.size &&
-          params.zip(paramTypes.tail).forall { case(ps, pc) =>
+          params.size == paramTypes.size &&
+          params.zip(paramTypes).forall { case(ps, pc) =>
             ps.typeSignature.typeSymbol == mirror.classSymbol(pc)
           }
         }.map { applyMethodSymbol =>
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
index e2eafb7370d..9b481b13fee 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
@@ -26,7 +26,7 @@ import scala.reflect.runtime.universe.TypeTag
 
 import org.apache.spark.SparkArithmeticException
 import org.apache.spark.sql.{Encoder, Encoders}
-import org.apache.spark.sql.catalyst.{FooClassWithEnum, FooEnum, OptionalData, PrimitiveData}
+import org.apache.spark.sql.catalyst.{FooClassWithEnum, FooEnum, OptionalData, PrimitiveData, ScroogeLikeExample}
 import org.apache.spark.sql.catalyst.analysis.AnalysisTest
 import org.apache.spark.sql.catalyst.dsl.plans._
 import org.apache.spark.sql.catalyst.expressions.AttributeReference
@@ -477,6 +477,9 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
   encodeDecodeTest(Option("abc"), "option of string")
   encodeDecodeTest(Option.empty[String], "empty option of string")
 
+  encodeDecodeTest(ScroogeLikeExample(1),
+    "SPARK-40385 class with only a companion object constructor")
+
   productTest(("UDT", new ExamplePoint(0.1, 0.2)))
 
   test("AnyVal class with Any fields") {
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
index b3480693ea6..4ebd57e82e6 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
@@ -423,7 +423,7 @@ class ObjectExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
       inputTypes = Nil,
       propagateNull = false,
       dataType = ObjectType(classOf[ScroogeLikeExample]),
-      outerPointer = Some(() => outerObj))
+      outerPointer = None)
     checkObjectExprEvaluation(newInst3, ScroogeLikeExample(1))
   }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org