You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "szehon-ho (via GitHub)" <gi...@apache.org> on 2025/09/25 00:46:30 UTC

[PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

szehon-ho opened a new pull request, #52444:
URL: https://github.com/apache/spark/pull/52444

   This pr rebase effort of https://github.com/apache/spark/pull/48304/files, which is unexpectedly close even though the issue is still there.
   
   ### What changes were proposed in this pull request?
   Fix a bug in JavaTypeInference.encoderFor.  In the parameterizableType case, it overrides the existing map of parameterizable type -> real type.  So any nested parameterizable type will fail.
   
   
   ### Why are the changes needed?
   Fix nested parameterization cases.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   The original pr adds unit test to JavaTypeInferenceSuite and JavaDatasetSuite.  The class from the original pr are simplified a little to make it obvious which one is wrapping.
   
   
   ### 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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #52444: [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class
URL: https://github.com/apache/spark/pull/52444


-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #52444:
URL: https://github.com/apache/spark/pull/52444#discussion_r2379972485


##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -139,7 +139,9 @@ object JavaTypeInference {
       encoderFor(typeVariables(tv), seenTypeSet, typeVariables)
 
     case pt: ParameterizedType =>
-      encoderFor(pt.getRawType, seenTypeSet, JavaTypeUtils.getTypeArguments(pt).asScala.toMap)
+      val newTvs = JavaTypeUtils.getTypeArguments(pt).asScala.toMap

Review Comment:
   For reference, a walkthrough:
   
   ```
   StringFooWrapper extends FooWrapper<String>
   FooWrapper<U> contains Foo<U>
   Foo<T>
   ```
   
   On the first iteration for StringFooWrapper, typeVariable map contains (U => String)
   On the second iteration for FooWrapper, this gets overriden by (T => U)
   
   The correct behavior is to keep both (U => String) and (T => U) in the map.



-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #52444:
URL: https://github.com/apache/spark/pull/52444#issuecomment-3335493308

   @dongjoon-hyun thanks for the review!  FYI i also did a minor cleanup of the original patch's map aggregation logic (should be the same behavior).  Also previously test failure didnt look related, but re-triggering.  also cc @cloud-fan fyi 


-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #52444:
URL: https://github.com/apache/spark/pull/52444#discussion_r2379972485


##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -139,7 +139,9 @@ object JavaTypeInference {
       encoderFor(typeVariables(tv), seenTypeSet, typeVariables)
 
     case pt: ParameterizedType =>
-      encoderFor(pt.getRawType, seenTypeSet, JavaTypeUtils.getTypeArguments(pt).asScala.toMap)
+      val newTvs = JavaTypeUtils.getTypeArguments(pt).asScala.toMap

Review Comment:
   For reference, a walkthrough:
   
   ```
   StringFooWrapper extends FooWrapper<String>
   FooWrapper<U> contains Foo<U>
   Foo<T>
   ```
   
   On the first call for StringFooWrapper, typeVariable map contains (U => String)
   On the next recursive call for FooWrapper, this gets overriden by (T => U)
   
   The correct behavior is to keep both (U => String) and (T => U) in the final Encoder's typeVariable map.



-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #52444:
URL: https://github.com/apache/spark/pull/52444#issuecomment-3338881152

   The kafka test failure is unrelated, thanks, merging to master!


-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #52444:
URL: https://github.com/apache/spark/pull/52444#issuecomment-3332283614

   Could you re-trigger the failed 15 GitHub Action jobs? It seems to fail due to the Today's `Docker Hub` outage.


-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #52444:
URL: https://github.com/apache/spark/pull/52444#discussion_r2379972485


##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -139,7 +139,9 @@ object JavaTypeInference {
       encoderFor(typeVariables(tv), seenTypeSet, typeVariables)
 
     case pt: ParameterizedType =>
-      encoderFor(pt.getRawType, seenTypeSet, JavaTypeUtils.getTypeArguments(pt).asScala.toMap)
+      val newTvs = JavaTypeUtils.getTypeArguments(pt).asScala.toMap

Review Comment:
   For reference, a walkthrough:
   
   ```
   StringFooWrapper extends FooWrapper<String>
   FooWrapper<U> contains Foo<U>
   Foo<T>
   ```
   
   On the first iteration for StringFooWrapper, typeVariable map contains (U => String)
   On the second iteration for FooWrapper, this gets overriden by (T => U)
   
   The correct behavior is to keep both (U => String) and (T => U) in the final Encoder's typeVariable map.



-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #52444:
URL: https://github.com/apache/spark/pull/52444#issuecomment-3338917130

   SPARK-46679 is already resolved, @szehon-ho can you create a new ticket?


-- 
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


Re: [PR] [SPARK-46679][SQL] Fix for SparkUnsupportedOperationException Not found an encoder of the type T, when using Parameterized class [spark]

Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #52444:
URL: https://github.com/apache/spark/pull/52444#issuecomment-3339716010

   thank you @dongjoon-hyun @cloud-fan !


-- 
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