You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org> on 2019/02/04 07:08:45 UTC

[Impala-ASF-CR] IMPALA-8034: Improve planner tests

Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12145 )

Change subject: IMPALA-8034: Improve planner tests
......................................................................


Patch Set 3:

(11 comments)

Great coverage. Tried to double check as much I can.  Just some minor comments.
Curious how you came up with all the test queries

http://gerrit.cloudera.org:8080/#/c/12145/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12145/3//COMMIT_MSG@14
PS3, Line 14: highlighlights
typo


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test
File testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test:

http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test@218
PS3, Line 218: # Bug: expected cardinality = 0
Ouch


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test@340
PS3, Line 340: 0.33
Is this a common default selectivity estimate for single slot non-equality predicate?


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test@343
PS3, Line 343: # Though |M.pk| > |D.fk|, we assume that filtering eliminated the unmatched keys
Do we even guess these as fk/pk joins? (looks like no since there are no fk/pk predicates)


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test@426
PS3, Line 426: Expected cardinality ~1
Shouldn't this be 0?


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test@533
PS3, Line 533: Bug: Expected cardinality ~1
why?


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-inner-join.test@560
PS3, Line 560: Expected cardinality ~1
same.


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-join.test
File testdata/workloads/functional-planner/queries/PlannerTest/card-join.test:

http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-join.test@1
PS3, Line 1: # Join cardinality tests
Did you split this up into other files? This doesn't seem to be called from PlannerTest


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-outer-join.test
File testdata/workloads/functional-planner/queries/PlannerTest/card-outer-join.test:

http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-outer-join.test@599
PS3, Line 599: # Join on a computed column
             : # Assumes Cartesian product * 0.1
             : # |join| = 11K * 7K * 0.1 = 7M
             : # Bug: Expected cardinality ~7M
             : select a.id, b.id
             : from functional.alltypes a, functional.alltypesagg b
             : where a.id = b.id + b.int_col
Does this belong in this file?


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test
File testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test:

http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test@169
PS3, Line 169: Inequality
I think you mean non-equal predicates here? Inequality means <>, no? (multiple places below)


http://gerrit.cloudera.org:8080/#/c/12145/3/testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test@247
PS3, Line 247: Ramakrisnan
nit: typo :-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I40e59e08d7ddf2b0391d42e50511aaf95d7275f4
Gerrit-Change-Number: 12145
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Feb 2019 07:08:45 +0000
Gerrit-HasComments: Yes