Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16462 )
Change subject: IMPALA-10185: se bool stats for selectivity calculations ...................................................................... Patch Set 2: (3 comments) I thought this was ready to go in so I was going to do the cleanup myself, but it looks like it doesn't implement it for some equivalent constructs, so we should make it complete before moving ahead. http://gerrit.cloudera.org:8080/#/c/16462/2/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java: http://gerrit.cloudera.org:8080/#/c/16462/2/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@253 PS2, Line 253: return; > Doing the early return seems a bit confusing when it's in a method that doe Done http://gerrit.cloudera.org:8080/#/c/16462/2/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@255 PS2, Line 255: IS_TRUE_LITERAL > There are also ISNOTTRUE and ISNOTFALSE. I wonder if they are normalized in Thanks for asking this. I'm realizing that this solution isn't actually complete, because we can have boolean SlotRefs used as predicates, for which we don't calculate selectivity (but could use this method). Same for the istrue/isfalse/isnottrue/isnotfalse functions implemented with FunctionCallExpr. So we should fix this up before considering this complete. http://gerrit.cloudera.org:8080/#/c/16462/2/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@261 PS2, Line 261: } else if (numNulls >= 0) { > Maybe add a condition to test for ISNULL predicate? IS NULL predicates get turned into an IsNullPredicate object, so the selectively calculation for that case is in that class. -- To view, visit http://gerrit.cloudera.org:8080/16462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95c1c7c915bf6bca13fe006c0531c33988187d12 Gerrit-Change-Number: 16462 Gerrit-PatchSet: 2 Gerrit-Owner: Shant Hovsepian <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Shant Hovsepian <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 07 Jan 2021 20:36:12 +0000 Gerrit-HasComments: Yes
