Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17637 )
Change subject: IMPALA-10766: Better selectivity for =,not distinct ...................................................................... Patch Set 5: (3 comments) Looks very good. Thanks a lot for the follow-up work! http://gerrit.cloudera.org:8080/#/c/17637/5/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/17637/5/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@250 PS5, Line 250: rChildIsNull nit. Do we need to worry about left child is null here, or not since the left child is always a column reference? http://gerrit.cloudera.org:8080/#/c/17637/5/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@280 PS5, Line 280: // For = and !=, all null values are false : // For is distinct from null, all null values are false : // For is not distinct from non-null, all null values are false nit. may combine them into a single sentence: For =, !=, "is distinct from null" and "is not distinct from non-null", all null values must be excluded. http://gerrit.cloudera.org:8080/#/c/17637/5/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@284 PS5, Line 284: Operator.NOT_DISTINCT && rChildIsNull) Another case here is "is distinct from not-null". -- To view, visit http://gerrit.cloudera.org:8080/17637 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8ec62f2355a7036125cc0d261b790644b9f4b60 Gerrit-Change-Number: 17637 Gerrit-PatchSet: 5 Gerrit-Owner: liuyao <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: liuyao <[email protected]> Gerrit-Comment-Date: Fri, 09 Jul 2021 17:55:28 +0000 Gerrit-HasComments: Yes
