rdblue commented on issue #442: Optimize ExpressionVisitors.visit method to 
traverse minimum nodes number required for determining the result
URL: https://github.com/apache/incubator-iceberg/pull/442#issuecomment-527991469
 
 
   Thanks for working on this, @vvysotskyi. This looks like a good start, but 
it isn't safe to update the visitor like this. The visitors may be 
parameterized by Boolean or Expression or something else. If parameterized by 
boolean, like an evaluator, then this wouldn't work. And if parameterized by 
expression, then there is no guarantee that this is going to be correct for the 
operation that uses the visitor pattern.
   
   I suggest adding a new `visit` method called `visitEvaluator` that accepts 
`ExpressionVisitor<Boolean>` and applies the short-circuit logic using the 
boolean returned by the eval visitor.

----------------------------------------------------------------
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]

Reply via email to