xabriel 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-462504839 > 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. Agreed. > Passing in a bound predicate means we don't need to pass options for expression binding in so many places. Agreed as well. Still, I tend to side with the approach of this PR. Here is my rationale: **What refactoring would yield more cohesion and less coupling?** In this PR's approach, the caller only needs to know how to pass thru a variable, to be used as the callee sees fit. In the alternate, the caller now is in the business of binding, even though it is just passing thru the `Expression` object. I also think that, in future, there may be other parameters that we will want to pass down to enhance `Evaluator`s. At that moment, we could upgrade `caseSensitive` flag to be an object, say: `EvaluatorParams`, that would contain `caseSensitive`, and whatever else we find we need. In that case, should the caller be in the business of knowing how to apply each and every `EvaluatorParam` just to maintain the interface intact, or should the caller just pass it thru as another variable? So, to me, with approach on this PR, even though verbose, we have less coupling by just passing thru the one flag, and more cohesion later on when, inevitably, we find other contextual flags we will need to pass down. ( On a side note, whether we go with approach in this PR or alternate, we would still need to pass `caseSensitive` to the other classes: `TableScan`, `FilteredManifest`, and the Spark `Reader`. Not in this PR, but I presume we'd need it in the Presto Reader as well. ) Let me know if this rationale makes sense @rdblue.
---------------------------------------------------------------- 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]
