Hongze, I agree with your point, and I have posted my point in PR-5698[1]. If there is an actual issue with case-class inheritance, I'm fine to have some more code and workaround to fix it.
[1]: https://github.com/apache/incubator-gluten/pull/5698 Hongze Zhang <hon...@apache.org> 于2024年5月31日周五 13:07写道： > > Thank you for raising the discussion up. > > > we find it introduces higher cost than > expected. It requires more code and workaround to resolve the issue of > removing case-class inheritance. > > I don't think it's true. If you think the previous inheritance approach of > VeloxColumnarWriteFilesExec has lower (code-learn / maintenance) cost than > the current solution, it's actually because the bad things were hidden. One > shouldn't consider the case-class inheritance pattern simple. It's actually > something very complicated and dangerous especially when it's used on a > regular Spark operator. > > If some code causes the following: > > ``` > val a = A() > val b = B() > > println(a == b) // "false" > println(b == a) // "true" > ``` > > Then the code shouldn't be described as "low cost". > > I don't like using workarounds either. So to me any solution for the issue > can be discussed but I don't think it's acceptable to bring case-class > inheritance back regarding the healthiness of outcode base. > > Best, > Hongze > > On Fri, May 31, 2024 at 11:35 AM XiDuo You <ulyssesyo...@gmail.com> wrote: > > > Hi folks, > > > > It is a discussion about PR-5698[1] and PR-5480[2]. I hope the > > community can reach an agreement. > > > > Recently, we are working on fixing the case-class inheritance issue, > > thanks to @zhztheplayer, it's great work. However, when touching > > `VeloxColumnarWriteFilesExec`, we find it introduces higher cost than > > expected. It requires more code and workaround to resolve the issue of > > removing case-class inheritance. I think when the cost of solving an > > issue exceeds the cost of the issue itself, we can consider tolerating > > the known issue. > > > > I propose to revert PR-5480[2] first to avoid regression. > > > > Thank you > > Xiduo > > > > [1]: https://github.com/apache/incubator-gluten/pull/5698 > > [2]: https://github.com/apache/incubator-gluten/pull/5480 > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@gluten.apache.org > > For additional commands, e-mail: dev-h...@gluten.apache.org > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@gluten.apache.org For additional commands, e-mail: dev-h...@gluten.apache.org
