cloud-fan edited a comment on issue #24910: [SPARK-28108][SQL] Simplify OrcFilters URL: https://github.com/apache/spark/pull/24910#issuecomment-503830569 > separate the logic of building a convertible filter tree and the actual SearchArgument builder, since the two procedures are different and their return types are different... The code is more readable. I don't quite agree with this. The benefit of putting them together is: they share a similar procedure and it's easy to maintain and read if these 2 are combined. For example, when we want to support a new leaf predicate, we will always support it in both "building a convertible filter tree" and the "actual SearchArgument builder". One idea: we can separate the `And`/`Or`/`Not` part, and keep the leaf predicate part combined. ``` def buildLeafSearchArguments... = { case EqualTo(...) => Some(...) ... case _ => None } def trimUnconvertibleFilters... = { def helper()... = { case And... case Or... ... case other => if (buildLeafSearchArguments(other).isDefined) Some(other) else None } ... } def buildSearchArgument... = { case And... ... case other => buildLeafSearchArguments(other) } ``` Then we can still remove `ActionType` stuff.
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
