Alex Behm has posted comments on this change. Change subject: IMPALA-3450: LIMITs on plan nodes are reflected in cardinality estimates ......................................................................
Patch Set 4: (3 comments) Thanks for fixing this! I think it makes sense to add new test infra to cover cases like this. See comment in joins.test http://gerrit.cloudera.org:8080/#/c/3127/4/fe/src/main/java/com/cloudera/impala/planner/UnnestNode.java File fe/src/main/java/com/cloudera/impala/planner/UnnestNode.java: Line 70: cardinality_ = capAtLimit(cardinality_); Let's also fix SelectNode while we are at it. You can create a test with a construct like this: select * from (select * from functional.alltypes limit 100) v where id < 10 limit 1 http://gerrit.cloudera.org:8080/#/c/3127/4/testdata/workloads/functional-planner/queries/PlannerTest/joins.test File testdata/workloads/functional-planner/queries/PlannerTest/joins.test: Line 2202: # IMPALA-3450: limits on nodes are reflected in cardinality estimates. In this test, Given that we are adding a lot of lines for these tests, I think it makes more sense to add a targeted validation step directly in PlannerTestBase. We'd traverse the single-node plan tree, and assert that nodes with a limit have a corresponding cardinality. This infra is probably generally useful for other sanity checks. We can then hopefully reduce lines in the .test files. Also consider moving some of the tests into inline-view-limit.test for consistency. We have a lot of similar looking tests there, and it's easier to see whether we already have existing tests for some of these cases. http://gerrit.cloudera.org:8080/#/c/3127/4/testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test File testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test: Line 1813: # side, rather than the probe side, of a hash join. hash join -> cross join -- To view, visit http://gerrit.cloudera.org:8080/3127 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic06dcb93bbb2510c0d40151302bd817ef340b825 Gerrit-PatchSet: 4 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-HasComments: Yes
