zhztheplayer commented on PR #5698: URL: https://github.com/apache/incubator-gluten/pull/5698#issuecomment-2105532508
> I searched Spark code repo, and really find [a case](https://github.com/apache/spark/blob/f699f556d8a09bb755e9c8558661a36fbdb42e73/sql/catalyst/src/main/scala/org/apache/spark/sql/sources/filters.scala#L361-L368)... is it valid for you @zhztheplayer ? The case is not really case-class inheritance we are discussing here. It's just creates an alias from `AlwaysTrue$` to `val INSTANCE = new AlwaysTrue()`. Case-class's default members, say `equals`, `hashCode`, `apply`, `unapply` don't get changed. Having said that I am personally not into the approach either. It's better to be `AlwaysTrue.INSTANCE` or something. > My point is that if there is an actually issue with case class Once we get some, it could be very hard to trouble-shoot. Say Spark's subquery / exchange reuse rely on case-class equality, and test cases could reply on case-class's `unapply` to check plan. I remember we used to suffer from one or several similar case-class inheritance issues from very early stage of Gluten (probably Gazelle, I can recall it), and the issues really cost us effort to debug and fix. If we don't keep cleaning similar code, I highly doubt we will soon face another issue again. -- 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]
