> On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/cost/HiveVolcanoPlanner.java > > Lines 126-128 (patched) > > <https://reviews.apache.org/r/66285/diff/1/?file=1987621#file1987621line126> > > > > I don't follow this. No where in logic cost becomes zero (or lower) for > > heuristic. > > Further, method should break out of recursion as soon as there is a MV, > > instead of recursing further. Since in hueristic strategy as soon as we > > find a MV we will use that plan
The idea here is that if there is a possible rewriting, we should choose it over the original plan. But if there are multiple plans rewritings (e.g., multiple MVs, multiple ways of rewriting with same MV, partial rewritings with union, etc.), we should use the one with the lower cost among them. Hence, for the heuristic, 1) we assign a tiny cost to the TS that reads the MV and to all operators that are on top of the MV, and 2) we multiply by a certain factor the cost of the operators that are not directly on top of a TS with a materialized view (e.g., in a partial rewriting, the branch of the union that contains execution on the original sources, or in a bushy join tree the join operators that do not read any MV). This will help us select plans that replace as many joins as possible, and plans that contain full rewritings better than partial rewritings. Does it make sense? In any case, I will add additional comments to the class. > On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java > > Lines 97 (patched) > > <https://reviews.apache.org/r/66285/diff/1/?file=1987623#file1987623line97> > > > > Doesn't look this rule is used anywhere. This is used in CalcitePlanner (L1910). > On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java > > Lines 106 (patched) > > <https://reviews.apache.org/r/66285/diff/1/?file=1987623#file1987623line106> > > > > Same rule as previous? The rule definition is here, but the rule instantiation is above. From CalcitePlanner, we only reference the final static instance, but we still need this rule. The rule basically overrides the _getFloorSqlFunction_ and _getRollup_ methods. > On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java > > Lines 123 (patched) > > <https://reviews.apache.org/r/66285/diff/1/?file=1987623#file1987623line123> > > > > Unused rule? We instantiate it in L102 in this same class. The final static instance is then referenced in CalcitePlanner. > On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java > > Lines 108 (patched) > > <https://reviews.apache.org/r/66285/diff/1/?file=1987624#file1987624line109> > > > > Unused method. If you intend to use it, > > better name: addNotNullProject() This methods are not used because they are called via reflection. In any case, I have reverted the changes here because they were not needed (they were coming from previous version of the patch that was not changing the nullability of the input columns). > On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java > > Lines 124 (patched) > > <https://reviews.apache.org/r/66285/diff/1/?file=1987624#file1987624line125> > > > > addNotNullProjects() Same as above. > On March 26, 2018, 8:24 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java > > Line 1494 (original), 1489 (patched) > > <https://reviews.apache.org/r/66285/diff/1/?file=1987625#file1987625line1498> > > > > Do we store optimized plan or unoptimized plan when loading MV registry > > for defined MVs? If its optimized plan invoking rewrite rule at the end of > > optimization will make it easier for rewriting rule, else this should be > > invoked without any optimization for same reason. We store the optimized plan. However, for matching purposes, the plan of the MV does not really matter because the rule can extract structural information from the top of the plan (filters present, columns present, joins, aggregation functions, etc). However, the shape of the plan for the (sub)query that we are trying to match matters, as prejoin optimization stage may help us to infer some new predicates, remove some unused columns, etc. - Jesús ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66285/#review199993 ----------------------------------------------------------- On March 26, 2018, 6:38 p.m., Jesús Camacho Rodríguez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66285/ > ----------------------------------------------------------- > > (Updated March 26, 2018, 6:38 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-18770 > https://issues.apache.org/jira/browse/HIVE-18770 > > > Repository: hive-git > > > Description > ------- > > HIVE-18770 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 8d9b5a3194708ffacabfdb69d6af7d6193dcf156 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java > 5ad4406ceff5d83bf74264c33947f207ff2c1a61 > > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java > 3f73fd7fcc2d6c52a2015bdd947c1708723058d6 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveConfPlannerContext.java > b0f1a8dfafa46f2cb06ca05c673ba37c7366666d > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelBuilder.java > efd8a35699ef2c4bb9c363925b8adc1e2ca3cbd3 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/cost/HiveVolcanoPlanner.java > 88aedb6381a293c0dd0f7d4e767df6726a86f40f > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveTableScan.java > 94a3bac1a7df35c825247e51946ee6ef1b0b6342 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewRule.java > df9c1802c8983279500d3a06c1c526ce20af6146 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdPredicates.java > 4dc48f4710196acb68a9df5331244827b212aefe > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java > 612deb8327d85966751834257ab686cfa74f9feb > ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_2.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_8.q > PRE-CREATION > ql/src/test/queries/clientpositive/materialized_view_rewrite_9.q > PRE-CREATION > ql/src/test/results/clientpositive/druid/druidmini_mv.q.out > 97f6d844806cf33ea4403b33665142c612da6e84 > ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out > 4da3d0930fd30cc3ab74155efb4d82a910ea6944 > > ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out > d7ee468b49af904da93a74c86f0898c310970cab > ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out > PRE-CREATION > ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out > PRE-CREATION > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 3e1fea9d4fe707c59ee99781bd4c5aacdbd9d381 > > > Diff: https://reviews.apache.org/r/66285/diff/2/ > > > Testing > ------- > > > Thanks, > > Jesús Camacho Rodríguez > >
