Alex Behm has posted comments on this change. Change subject: IMPALA-3940: Fix getting column stats through views. ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/3865/2/fe/src/test/java/com/cloudera/impala/common/FrontendTestBase.java File fe/src/test/java/com/cloudera/impala/common/FrontendTestBase.java: Line 66: public class FrontendTestBase { > nice, thank you! Done Line 180: CreateViewStmt createVieweStmt = (CreateViewStmt) AnalyzesOk(createViewSql); > remove e in Viewe Done http://gerrit.cloudera.org:8080/#/c/3865/2/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test File testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test: Line 2515: | | hash predicates: ss_store_sk = s_store_sk > do you know what the perf impact of these plan changes is? Don't know, but the plan looks saner since the join on date_dim (the scan has predicates) was moved before the join on store. Even if the old plan was faster I feel like that would be just by chance and we should accept the current saner looking plan. http://gerrit.cloudera.org:8080/#/c/3865/2/testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test File testdata/workloads/functional-planner/queries/PlannerTest/tpch-views.test: Line 1: # TPCH-Q1 > this is going to be hard to verify by hand. Good question. I verified the plans manually with meld and the only difference are in how slot refs are printed. In the view version all slots refs are fully qualified. In the original version some slot refs are unqualified, some are fully qualified and some are qualified with an explicit table alias (like n1 or n2 in TPCH-Q7). I tried to implement a search+replace alternative, but unfortunately, it is not easy to do due. The regex gets increasingly complicated because some column names are prefixes of other column names and the original version has slot refs with explicit aliases like "n1", "n2". My feeling is that the search+replace path is complicated enough to be brittle itself. As a compromise, maybe we can just keep the single-node plans in the new view-based TPCH test? -- To view, visit http://gerrit.cloudera.org:8080/3865 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3b62a5e7e7d0e84850749108c13991647cedce6 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
