jcamachor commented on a change in pull request #1561:
URL: https://github.com/apache/hive/pull/1561#discussion_r502955735
##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -1839,6 +1839,9 @@ private static void populateLlapDaemonVarsSet(Set<String>
llapDaemonVarsSetLocal
// materialized views
HIVE_MATERIALIZED_VIEW_ENABLE_AUTO_REWRITING("hive.materializedview.rewriting",
true,
"Whether to try to rewrite queries using the materialized views
enabled for rewriting"),
+
HIVE_MATERIALIZED_VIEW_ENABLE_AUTO_REWRITING_BY_QUERY_TEXT("hive.materializedview.rewriting.by.query.text",
true,
Review comment:
nit. can remove `by.`
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -1871,6 +1876,17 @@ public RelNode apply(RelOptCluster cluster, RelOptSchema
relOptSchema, SchemaPlu
}
perfLogger.perfLogEnd(this.getClass().getName(), PerfLogger.OPTIMIZER,
"Calcite: Plan generation");
+ if
(conf.getBoolVar(ConfVars.HIVE_MATERIALIZED_VIEW_ENABLE_AUTO_REWRITING_BY_QUERY_TEXT)
&&
+ !getQB().isMaterializedView() &&
!ctx.isLoadingMaterializedView() && !getQB().isCTAS() &&
+ getQB().hasTableDefined()) {
+ unparseTranslator.applyTranslations(ctx.getTokenRewriteStream());
+ String expandedQueryText =
ctx.getTokenRewriteStream().toString(ast.getTokenStartIndex(),
ast.getTokenStopIndex());
+ RelOptMaterialization relOptMaterialization =
db.getMaterialization(expandedQueryText);
Review comment:
This needs to be slightly more complicated. If you find a matching
materialized view, you will need to verify that it is still up-to-date. I
believe `validateMaterializedViewsFromRegistry` does exactly that, but you can
check the usual MV rewriting workflow to make sure.
On an additional note, you could have multiple MVs with the same definition.
You would want to iterate through them in case one of them is up-to-date.
If none are up-to-date, you can simply continue with usual optimization flow.
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -472,7 +474,7 @@ public RelNode genLogicalPlan(ASTNode ast) throws
SemanticException {
}
profilesCBO = obtainCBOProfiles(queryProperties);
disableJoinMerge = true;
- final RelNode resPlan = logicalPlan();
+ final RelNode resPlan = logicalPlan(ast);
Review comment:
Maybe we could try to avoid passing the ast as a reference, e.g., if it
can be retrieved from the context?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/MaterializedViews.java
##########
@@ -0,0 +1,149 @@
+/*
+ * 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
org.apache.hadoop.hive.ql.optimizer.calcite.rules.views.HiveMaterializedViewUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+public class MaterializedViews {
+ private static final Logger LOG =
LoggerFactory.getLogger(MaterializedViews.class);
+
+ /* Key is the database name. Value a map from the qualified name to the view
object. */
+ private final ConcurrentMap<String, ConcurrentMap<String,
RelOptMaterialization>> materializedViews =
+ new ConcurrentHashMap<>();
+ // Map for looking up materialization by view query text
+ private final Map<String, RelOptMaterialization> sqlToMaterializedView = new
ConcurrentHashMap<>();
+
+
+ public void putIfAbsent(Table materializedViewTable, RelOptMaterialization
materialization) {
+ ConcurrentMap<String, RelOptMaterialization> dbMap =
ensureDbMap(materializedViewTable);
+
+ // You store the materialized view
+ dbMap.compute(materializedViewTable.getTableName(), (mvTableName,
relOptMaterialization) -> {
+ sqlToMaterializedView.put(materializedViewTable.getViewExpandedText(),
materialization);
+ return materialization;
+ });
+
+ LOG.debug("Materialized view {}.{} added to registry",
+ materializedViewTable.getDbName(),
materializedViewTable.getTableName());
+ }
+
+ private ConcurrentMap<String, RelOptMaterialization> ensureDbMap(Table
materializedViewTable) {
+ // We are going to create the map for each view in the given database
+ ConcurrentMap<String, RelOptMaterialization> dbMap =
+ new ConcurrentHashMap<String, RelOptMaterialization>();
+ // If we are caching the MV, we include it in the cache
+ final ConcurrentMap<String, RelOptMaterialization> prevDbMap =
materializedViews.putIfAbsent(
+ materializedViewTable.getDbName(), dbMap);
+ if (prevDbMap != null) {
+ dbMap = prevDbMap;
+ }
+ return dbMap;
+ }
+
+ public void refresh(
+ Table oldMaterializedViewTable, Table materializedViewTable,
RelOptMaterialization newMaterialization) {
+ ConcurrentMap<String, RelOptMaterialization> dbMap =
ensureDbMap(materializedViewTable);
+
+ dbMap.compute(materializedViewTable.getTableName(), (mvTableName,
existingMaterialization) -> {
+ if (existingMaterialization == null) {
+ // If it was not existing, we just create it
+ sqlToMaterializedView.put(materializedViewTable.getViewExpandedText(),
newMaterialization);
+ return newMaterialization;
+ }
+ Table existingMaterializedViewTable =
HiveMaterializedViewUtils.extractTable(existingMaterialization);
+ if (existingMaterializedViewTable.equals(oldMaterializedViewTable)) {
+ // If the old version is the same, we replace it
+ sqlToMaterializedView.put(materializedViewTable.getViewExpandedText(),
newMaterialization);
+ return newMaterialization;
+ }
+ // Otherwise, we return existing materialization
+ sqlToMaterializedView.put(materializedViewTable.getViewExpandedText(),
existingMaterialization);
+ return existingMaterialization;
+ });
+
+ LOG.debug("Refreshed materialized view {}.{} -> {}.{}",
+ oldMaterializedViewTable.getDbName(),
oldMaterializedViewTable.getTableName(),
+ materializedViewTable.getDbName(),
materializedViewTable.getTableName());
+ }
+
+ public void remove(Table materializedViewTable) {
Review comment:
Not very strong opinion, but maybe we should use same names that we have
in the registry (e.g., `drop`) to make it easier to link.
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
##########
@@ -94,9 +91,7 @@
/* Singleton */
private static final HiveMaterializedViewsRegistry SINGLETON = new
HiveMaterializedViewsRegistry();
- /* Key is the database name. Value a map from the qualified name to the view
object. */
- private final ConcurrentMap<String, ConcurrentMap<String,
RelOptMaterialization>> materializedViews =
- new ConcurrentHashMap<>();
+ private final MaterializedViews materializedViews = new MaterializedViews();
Review comment:
`MaterializedViews` -> `MaterializedViewsCache` ?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/MaterializedViews.java
##########
@@ -0,0 +1,149 @@
+/*
+ * 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
org.apache.hadoop.hive.ql.optimizer.calcite.rules.views.HiveMaterializedViewUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+public class MaterializedViews {
+ private static final Logger LOG =
LoggerFactory.getLogger(MaterializedViews.class);
+
+ /* Key is the database name. Value a map from the qualified name to the view
object. */
+ private final ConcurrentMap<String, ConcurrentMap<String,
RelOptMaterialization>> materializedViews =
+ new ConcurrentHashMap<>();
+ // Map for looking up materialization by view query text
+ private final Map<String, RelOptMaterialization> sqlToMaterializedView = new
ConcurrentHashMap<>();
Review comment:
I mentioned it below: This should be a multi-value.
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
##########
@@ -292,35 +277,12 @@ public void refreshMaterializedView(HiveConf conf, Table
oldMaterializedViewTabl
return;
}
- // We are going to create the map for each view in the given database
- ConcurrentMap<String, RelOptMaterialization> dbMap =
- new ConcurrentHashMap<String, RelOptMaterialization>();
- // If we are caching the MV, we include it in the cache
- final ConcurrentMap<String, RelOptMaterialization> prevDbMap =
materializedViews.putIfAbsent(
- materializedViewTable.getDbName(), dbMap);
- if (prevDbMap != null) {
- dbMap = prevDbMap;
- }
final RelOptMaterialization newMaterialization =
createMaterialization(conf, materializedViewTable);
if (newMaterialization == null) {
return;
}
- dbMap.compute(materializedViewTable.getTableName(), new BiFunction<String,
RelOptMaterialization, RelOptMaterialization>() {
- @Override
- public RelOptMaterialization apply(String tableName,
RelOptMaterialization existingMaterialization) {
- if (existingMaterialization == null) {
- // If it was not existing, we just create it
- return newMaterialization;
- }
- Table existingMaterializedViewTable =
HiveMaterializedViewUtils.extractTable(existingMaterialization);
- if (existingMaterializedViewTable.equals(oldMaterializedViewTable)) {
- // If the old version is the same, we replace it
- return newMaterialization;
- }
- // Otherwise, we return existing materialization
- return existingMaterialization;
- }
- });
+ materializedViews.refresh(oldMaterializedViewTable, materializedViewTable,
newMaterialization);
+;
Review comment:
nit. can be removed
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/MaterializedViews.java
##########
@@ -0,0 +1,149 @@
+/*
+ * 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
org.apache.hadoop.hive.ql.optimizer.calcite.rules.views.HiveMaterializedViewUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+public class MaterializedViews {
Review comment:
Please add comments on the methods of this class, since it is quite
critical.
##########
File path:
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestMaterializedViews.java
##########
@@ -0,0 +1,342 @@
+/*
+ * 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.Convention;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCost;
+import org.apache.calcite.plan.RelOptMaterialization;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelOptQuery;
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollation;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelShuttle;
+import org.apache.calcite.rel.RelVisitor;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.metadata.Metadata;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Litmus;
+import org.apache.calcite.util.Pair;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import static org.apache.derby.vti.XmlVTI.asList;
Review comment:
nit. weird dependency
##########
File path:
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestMaterializedViews.java
##########
@@ -0,0 +1,342 @@
+/*
+ * 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.Convention;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCost;
+import org.apache.calcite.plan.RelOptMaterialization;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelOptQuery;
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollation;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelShuttle;
+import org.apache.calcite.rel.RelVisitor;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.metadata.Metadata;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Litmus;
+import org.apache.calcite.util.Pair;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import static org.apache.derby.vti.XmlVTI.asList;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.jupiter.api.Assertions.*;
+
+class TestMaterializedViews {
Review comment:
Please add general comment with the test purpose.
##########
File path:
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestMaterializedViews.java
##########
@@ -0,0 +1,342 @@
+/*
+ * 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.Convention;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCost;
+import org.apache.calcite.plan.RelOptMaterialization;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelOptQuery;
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollation;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelShuttle;
+import org.apache.calcite.rel.RelVisitor;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.core.CorrelationId;
+import org.apache.calcite.rel.metadata.Metadata;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Litmus;
+import org.apache.calcite.util.Pair;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import static org.apache.derby.vti.XmlVTI.asList;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.jupiter.api.Assertions.*;
Review comment:
nit. expand
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
