[GitHub] [calcite] vlsi commented on a change in pull request #2238: [CALCITE-4364] `a IN (1, 2) AND a = 1` should be simplified to `a = 1`

[GitBox <gi...@apache.org>]

vlsi commented on a change in pull request #2238:
URL: https://github.com/apache/calcite/pull/2238#discussion_r516743019



##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -1330,7 +1330,7 @@ RexNode simplifyAnd(RexCall e, RexUnknownAs unknownAs) {
 
     final SargCollector sargCollector = new SargCollector(rexBuilder, true);
     operands.forEach(t -> sargCollector.accept(t, terms));
-    if (sargCollector.map.values().stream().anyMatch(b -> b.complexity() > 1)) {
+    if (sargCollector.needToFix(terms.size())) {

Review comment:
       What do you mean `unnecessary plan change`?
   Do you mean plan changes when compared with 1.25 or 1.26?
   
   I guess 1.26 is not really viable (since there are significant issues with Sarg), so I would skip 1.26 from consideration with regard to plan changes.




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