vlsi commented on issue #1044: [CALCITE-2838] Simplification: Remove redundant 
IS TRUE/IS NOT FALSE …
URL: https://github.com/apache/calcite/pull/1044#issuecomment-463701536
 
 
   >If you mean; rewrite `x IS FALSE` to `not x` in case of `UnknownAs.False` ; 
I think that will work, I'll include it
   
   Well, in the code you do hardcode IS TRUE/IS NOT FALSE, and you had nothing 
regarding IS NOT TRUE/IS FALSE. I think the rest operations should be covered 
either in the code or in comments that explain why handing those makes no sense.
   
   For instance, if replacement of  IS_FALSE+UnknownAs.FALSE to NOT(x) is worth 
doing, a comment is required to clarify why it works.
   If the replacement does not play well (e.g. it increases expression size 
with no benefit), then a comment is required to clarify things.
   
   By the way, why don't you consider IS_TRUE+UnknownAs.TRUE and other 
Unknown.As.TRUE cases?
   

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to