[ 
https://issues.apache.org/jira/browse/CALCITE-2912?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16791046#comment-16791046
 ] 

Jesus Camacho Rodriguez commented on CALCITE-2912:
--------------------------------------------------

A few additional notes after investigating further. There is an assumption that 
is not documented anywhere concerning constants, that should have been removed 
from Aggregate operator before going into the materialized view rewriting.

However, even in that case, the rewriting is not produced because the query 
will always contain at least a single grouping column in the Aggregate operator 
after {{AggregateProjectPullUpConstantsRule}} runs. Then, as [~swtalbot] 
described, the rewriting fails, as it assumes that if one of the columns is a 
constant in the query, it should already have been removed from it (that is why 
it also pulls them from the materialized view definition). In this case, in an 
intermediate step before bailing out, the view plan contains 
{{Aggregate([deptno])}}, while the query plan contains {{Aggregate([name])}}.

In addition, for multiple columns, the rewriting fails because constants in 
that case are not pull up in the query using the Volcano planner due to 
CALCITE-1048. For instance:
{code:java}
@Test public void testAggregateMaterializationWithConstantFilter2() {
  checkMaterialize(
      "select \"deptno\", \"name\", \"salary\", count(*) as c\n"
          + "from \"emps\" group by \"deptno\", \"name\", \"salary\"",
      "select \"deptno\", \"name\", count(*) as c\n"
          + "from \"emps\" where \"name\" = 'a_name' group by \"deptno\", 
\"name\"");
}
{code}
This problem does not appear in Hive, since we run 
{{AggregateProjectPullUpConstantsRule}} using HepPlanner.

Removing the call to {{AggregateProjectPullUpConstantsRule}} in L1735 does not 
seem to be the correct solution to the full problem. In Hive, I verified that 
this causes other regressions when we rollup expressions; there are two 
rewriting tests that fail and that I will try shortly to move to Calcite.

If the call remains, I think that a possible solution is to fix CALCITE-1048 
(for Volcano), and modify {{AggregateProjectPullUpConstantsRule}} to be 
rewritten as Aggregate with count + Filter if number of grouping columns 
becomes zero. I have an initial patch and this seems to work. However, the 
scope of the change would be all Calcite, hence maybe we do not want to make 
this change...? Any feedback/ideas would be appreciated.

> MaterializedViewAggregateRule.pushFilterToOriginalViewPlan fails to use valid 
> materialization given query with certain kinds of constant filters
> ------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-2912
>                 URL: https://issues.apache.org/jira/browse/CALCITE-2912
>             Project: Calcite
>          Issue Type: Bug
>            Reporter: Steven Talbot
>            Assignee: Jesus Camacho Rodriguez
>            Priority: Major
>
> This seems to stem from 
> [https://github.com/apache/calcite/blob/d6fb6abd9a17b81a5d8d592860ecd5c2f20ce9b1/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java#L1735]
> {quote}.addRuleInstance(this.aggregateProjectPullUpConstantsRule)
> {quote}
> If commented out, the bug does not occur. Also, when that line is commented 
> out, all tests in `MaterializationTest.java` pass. The following test 
> demonstrates the issue (fails on master, passes with line commented out):
> {code:java}
> @Test public void testAggregateMaterializationWithConstantFilter() {
>   checkMaterialize(
>           "select \"deptno\", \"name\", count(*) as c\n"
>                   + "from \"depts\" group by \"name\", \"deptno\"",
>           "select \"name\", count(*) as c\n"
>                   + "from \"depts\" where \"name\" = 'a_name' group by 
> \"name\"");
> }{code}
> From my understanding, the materialization fails to be used because 
> `aggregateProjectPullUpConstantsRule` rewrites the Aggregate in `topNode` at 
> [https://github.com/apache/calcite/blob/d6fb6abd9a17b81a5d8d592860ecd5c2f20ce9b1/core/src/main/java/org/apache/calcite/rel/rules/AbstractMaterializedViewRule.java#L1743]
>  to not include the known constant "name" in its group set, rather groups on 
> just deptno and then projects `'a_name'` as a constant. This make that 
> aggregate no longer a superset of the query aggregate and `rewriteView` then 
> fails to properly match the aggregates.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to