You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/12/15 10:58:01 UTC

[GitHub] [hive] kasakrisz opened a new pull request #1782: HIVE-24434: Filter out materialized views for rewriting if plan pattern is not allowed

kasakrisz opened a new pull request #1782:
URL: https://github.com/apache/hive/pull/1782


   ### What changes were proposed in this pull request?
   1. Store the scope of each materialized view in the cache/registry. It specifies whether the MV can be used in both Calcite and Sql text based query rewrites or Sql text based only.
   2. Scope value is calculated when the MV definition is parsed during adding the MV to the Registry.
   
   ### Why are the changes needed?
   Number of Materialized views added to the Calcite based rewrite planner affects query compilation performance. Too much view definition increases the planning time without any benefit. There are plan patterns which are not supported by the planner. MV having such patterns in its definition can be filtered out before even added to the planner.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   ```
   mvn test -DskipSparkTests -Dtest=TestMiniLlapLocalCliDriver -Dqfile=materialized_view_rewrite_11.q -pl itests/qtest -Pitests
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kasakrisz commented on a change in pull request #1782: HIVE-24434: Filter out materialized views for rewriting if plan pattern is not allowed

Posted by GitBox <gi...@apache.org>.
kasakrisz commented on a change in pull request #1782:
URL: https://github.com/apache/hive/pull/1782#discussion_r551106837



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Materialization.java
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.metadata;
+
+import org.apache.calcite.plan.RelOptMaterialization;
+
+import java.util.EnumSet;
+
+import static org.apache.commons.collections.CollectionUtils.intersection;
+
+/**
+ * Wrapper class of {@link RelOptMaterialization} and corresponding flags.
+ */
+public class Materialization {

Review comment:
       Changed this to extend `RelOptMaterialization`. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kasakrisz merged pull request #1782: HIVE-24434: Filter out materialized views for rewriting if plan pattern is not allowed

Posted by GitBox <gi...@apache.org>.
kasakrisz merged pull request #1782:
URL: https://github.com/apache/hive/pull/1782


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] jcamachor commented on a change in pull request #1782: HIVE-24434: Filter out materialized views for rewriting if plan pattern is not allowed

Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #1782:
URL: https://github.com/apache/hive/pull/1782#discussion_r548025308



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##########
@@ -1831,7 +1832,8 @@ public RelOptMaterialization getMaterializedViewForRebuild(String dbName, String
 
   private List<RelOptMaterialization> getValidMaterializedViews(List<Table> materializedViewTables,
       List<String> tablesUsed, boolean forceMVContentsUpToDate, boolean expandGroupingSets,
-      HiveTxnManager txnMgr) throws HiveException {
+      HiveTxnManager txnMgr, EnumSet<org.apache.hadoop.hive.ql.metadata.Materialization.RewriteAlgorithm> scope)

Review comment:
       nit. Import to avoid using fully-qualified class name here?

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Materialization.java
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.metadata;
+
+import org.apache.calcite.plan.RelOptMaterialization;
+
+import java.util.EnumSet;
+
+import static org.apache.commons.collections.CollectionUtils.intersection;
+
+/**
+ * Wrapper class of {@link RelOptMaterialization} and corresponding flags.
+ */
+public class Materialization {

Review comment:
       Should this class extend `RelOptMaterialization` rather than wrapping it? I think it would make sense since we may extend it with other properties in the future and may be convenient to be accessible anywhere in Calcite code.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CBOPlan.java
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.parse;
+
+import org.apache.calcite.rel.RelNode;
+
+/**
+ * Wrapper of Calcite plan.
+ */
+public class CBOPlan {
+  private final RelNode plan;
+  private final String invalidAutomaticRewritingMaterializationReason;
+
+  public CBOPlan(RelNode plan, String invalidAutomaticRewritingMaterializationReason) {
+    this.plan = plan;
+    this.invalidAutomaticRewritingMaterializationReason = invalidAutomaticRewritingMaterializationReason;
+  }
+
+  /**
+   * Root node of plan.
+   * @return Root {@link RelNode}
+   */
+  public RelNode getPlan() {
+    return plan;
+  }
+
+  /**
+   * Returns an error message if this plan can not be a definition of a Materialized view which is an input of
+   * Calcite based materialized view query rewrite.
+   * null or empty string otherwise.

Review comment:
       nit. null -> Null




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kasakrisz commented on a change in pull request #1782: HIVE-24434: Filter out materialized views for rewriting if plan pattern is not allowed

Posted by GitBox <gi...@apache.org>.
kasakrisz commented on a change in pull request #1782:
URL: https://github.com/apache/hive/pull/1782#discussion_r551106677



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CBOPlan.java
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.parse;
+
+import org.apache.calcite.rel.RelNode;
+
+/**
+ * Wrapper of Calcite plan.
+ */
+public class CBOPlan {
+  private final RelNode plan;
+  private final String invalidAutomaticRewritingMaterializationReason;
+
+  public CBOPlan(RelNode plan, String invalidAutomaticRewritingMaterializationReason) {
+    this.plan = plan;
+    this.invalidAutomaticRewritingMaterializationReason = invalidAutomaticRewritingMaterializationReason;
+  }
+
+  /**
+   * Root node of plan.
+   * @return Root {@link RelNode}
+   */
+  public RelNode getPlan() {
+    return plan;
+  }
+
+  /**
+   * Returns an error message if this plan can not be a definition of a Materialized view which is an input of
+   * Calcite based materialized view query rewrite.
+   * null or empty string otherwise.

Review comment:
       fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kasakrisz commented on a change in pull request #1782: HIVE-24434: Filter out materialized views for rewriting if plan pattern is not allowed

Posted by GitBox <gi...@apache.org>.
kasakrisz commented on a change in pull request #1782:
URL: https://github.com/apache/hive/pull/1782#discussion_r551107074



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##########
@@ -1831,7 +1832,8 @@ public RelOptMaterialization getMaterializedViewForRebuild(String dbName, String
 
   private List<RelOptMaterialization> getValidMaterializedViews(List<Table> materializedViewTables,
       List<String> tablesUsed, boolean forceMVContentsUpToDate, boolean expandGroupingSets,
-      HiveTxnManager txnMgr) throws HiveException {
+      HiveTxnManager txnMgr, EnumSet<org.apache.hadoop.hive.ql.metadata.Materialization.RewriteAlgorithm> scope)

Review comment:
       We already have a `Materialization` class so renamed this one to `HiveRelOptMaterialization` which fixed this.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org