Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/3778#issuecomment-68169507
Actually I'd highly suggest you breaking this PR into at least two self
contained PRs, which can be much easier to review and merge. Rule sets 1 and 4
can be merged into one PR, rule sets 2 and 3 into another. Maybe we can remove
rules 2 and 3 from this PR after your refactoring and get rule sets 1 and 4
merged first (I realized #3784 doesn't cover all rules in set 4, because the
second rule in set 4 doesn't help optimizing cartesian products).
The reason why I'm hesitant to include rule sets 2 and 3 is that, for now I
don't see a sound yet concise implementation without introducing extra
dependencies. Although I proposed the Spark `Interval` solution, I'd rather not
introduce Spire. On the other hand, rule sets 1 and 4 have been proven to be
both useful and easy to implement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
