likun666661 commented on PR #50372: URL: https://github.com/apache/arrow/pull/50372#issuecomment-4999295736
Thank you for the detailed and constructive feedback. I agree with the concerns you raised, especially that a conservative CSE pass needs both a clearly tested safety contract and evidence that it improves the workloads it targets without regressing ordinary expressions. I'm happy to continue working on this. My initial direction was inspired by this article about Apache Cloudberry's Gandiva optimization work: [Apache Cloudberry vectorized execution practice (3): Gandiva optimization](https://juejin.cn/post/7525700630524821513). It discusses repeated expression construction, structural deduplication with expression DAGs, deterministic semantic hashes, and reports meaningful reductions in expression nodes and construction time. That said, the article describes an optimization at the expression-construction layer, while this PR currently applies folding inside Gandiva before validation and code generation. I agree that we should validate this implementation independently rather than assume that the same results carry over. My proposed next steps are: 1. Make unoptimized-IR availability depend on the IR actually captured by `Engine`, including the mutable-`Configuration` regression case. 2. Add focused unit tests for `FoldCommonSubexpressions`, especially every "do not fold" condition and the structurally unequal cases you listed. 3. Revisit the purity/determinism contract for custom functions. The default-flags volatile-function case is a real gap; absence of the existing flags should not by itself be treated as proof that a function is safe to reuse. 4. Add large-expression benchmarks for no-duplicate, repeated-safe, and repeated-unsafe trees at several sizes, measuring `Projector`/`Filter` construction and IR size separately from evaluation. 5. Share the before/after numbers and adjust or narrow the implementation based on the results. If your team can share representative CASE-shaped expressions from real workloads, I would also be glad to incorporate them into the benchmark cases. Thanks again for taking the time to walk through this so carefully. The feedback is very helpful, and I'd like to keep pushing the work forward. -- 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]
