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 <[email protected]> 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: [email protected] > For additional commands, e-mail: [email protected] > >
