rdblue commented on issue #89: Make read-path Evaluators honor case sensitivity flag. Expose flag in Spark Reader. URL: https://github.com/apache/incubator-iceberg/pull/89#issuecomment-461936572 Thanks for working on this, @xabriel! I haven't replied yet because I'm trying to debate between approaches to this problem. There are two ways to go: 1. Add a case sensitivity flag to all the cases that end up using a binder, like this PR. 2. Requires that predicates passed to these evaluators are already bound. I'm not sure which is the right way to go. There are quiet a few paths changed by this, and most of them are duplicating work that could be done before passing an expression in. Passing a bound expression would be safe because the bound expression visitor [throws an exception](https://github.com/apache/incubator-iceberg/blob/master/api/src/main/java/com/netflix/iceberg/expressions/ExpressionVisitors.java#L126) when it hits an unbound predicate. Passing in a bound predicate means we don't need to pass options for expression binding in so many places. The drawback is that these classes are less friendly to callers. Instead of preparing predicates as needed, they would throw runtime exceptions when the predicates don't meet some expectation. I think tests would cover all the cases, but it would be more difficult for people to work with. What do you think?
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on 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]
