ahshahid commented on PR #48252: URL: https://github.com/apache/spark/pull/48252#issuecomment-2375259210
@hvanhovell I have filed a new bug [SPARK-49789](https://issues.apache.org/jira/browse/SPARK-49789) and linked this PR to it. Previously this PR was created against bug [SPARK-46679](https://issues.apache.org/jira/browse/SPARK-46679) It was brought to my notice by @squito that they are 2 different issues. [SPARK-46679](https://issues.apache.org/jira/browse/SPARK-46679) is a bug due to missing code in handling of parameterized bean, while this bug is for the case of generic bean which is not parameterized. For Paramerized bean, there is further scope of introspection. I am also adding the review comments put in the previous PR ( which I will be closing) [hvanhovell](https://github.com/hvanhovell) [2 days ago](https://github.com/apache/spark/pull/48158#discussion_r1771680158) For readability purposes I would create a branch at the beginning where you handle case tv: TypeVariable[_] if forGenericBound => This way the rest of the code is less impacted. done [hvanhovell](https://github.com/hvanhovell) [2 days ago](https://github.com/apache/spark/pull/48158#discussion_r1771684833) This will be an issue for Connect. While the API supports Kryo, Connect can't support Kryo in its current form. Either we have detect whether we are in connect mode, or we have to just fall back to java serialization. done [hvanhovell](https://github.com/hvanhovell) [2 days ago](https://github.com/apache/spark/pull/48158#discussion_r1771700787) This sort of begs for a queue and a loop. I am reasonable sure that is more readable... done [hvanhovell](https://github.com/hvanhovell) [2 days ago](https://github.com/apache/spark/pull/48158#discussion_r1771703915) Can we use the actual superclasses here instead of going through the error message? I'd also prefer if you unify this with the other java serialization code. Unifying it will other java serialziation code, is something which I intend to do in my refactoring.. Author @[ahshahid](https://github.com/ahshahid) ahshahid [2 days ago](https://github.com/apache/spark/pull/48158#discussion_r1771987465) I had thought about using the Serializable encoders as part of the match/case statement, but it appears tricky as the idea is that the Serializable encoders are to be used as last resort. And the way current logic works ( if I am not wrong), it collects all the interfaces/classes via the CommonUtils method, and it is possible , I think, that Serializable interface may show early as the introspection data is converted to a map in the Bean class val parentClassesTypeMap = JavaTypeUtils.getTypeArguments(c, classOf[Object]).asScala.toMap I am hesitant at this point to further complicate the code, unless you all think that its worthwhile to do it in this PR. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
