liuyao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17344 )

Change subject: IMPALA-7560: Set selectivity of Not-equal
......................................................................


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/17344/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17344/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-7560:
> You might want to use IMPALA-7560 as this was the original Jira and it cont
Done


http://gerrit.cloudera.org:8080/#/c/17344/4//COMMIT_MSG@11
PS4, Line 11: col != 5", but not "2 * col !=
> nit: wouldn't be better to use != in the example?
Done


http://gerrit.cloudera.org:8080/#/c/17344/3/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/17344/3/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@257
PS3, Line 257:  || op_ == Ope
> SlotRef retrieves the NDV from the SlotDescriptor's ColumnStats in SlotRef.
Done


http://gerrit.cloudera.org:8080/#/c/17344/4/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java:

http://gerrit.cloudera.org:8080/#/c/17344/4/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java@a372
PS4, Line 372:
> That seems wrong looking at the data. I think it should be 1.
Done


http://gerrit.cloudera.org:8080/#/c/17344/4/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java@344
PS4, Line 344:
> NULL values are not included in the NDV, also null count is stored separate
Done


http://gerrit.cloudera.org:8080/#/c/17344/4/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java@355
PS4, Line 355: 0);
> I think the expected value is 0 here, as null_str is all nulls.
Done


http://gerrit.cloudera.org:8080/#/c/17344/4/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java@359
PS4, Line 359: some_nulls is not distinct from 'foo'",
> I think it'd be worth to add a test case with
Done



--
To view, visit http://gerrit.cloudera.org:8080/17344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd6f5945840ea2a8194d72aa440ddfa6915cbb3a
Gerrit-Change-Number: 17344
Gerrit-PatchSet: 6
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: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: liuyao <[email protected]>
Gerrit-Comment-Date: Fri, 18 Jun 2021 07:30:15 +0000
Gerrit-HasComments: Yes

Reply via email to