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 <prog...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <prog...@cloudera.com> Gerrit-Comment-Date: Mon, 04 Feb 2019 07:08:45 +0000 Gerrit-HasComments: Yes