Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CALCITE-5665] Reducing ineffective matches for MaterializedViewRules #3169

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[CALCITE-5665] Reducing ineffective matches for MaterializedViewRules
  • Loading branch information
xinqiu.hu committed Apr 22, 2023
commit 30f6991e0fa3af1db8c8232921060b7837577ec1
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@
import org.apache.calcite.util.mapping.IntPair;
import org.apache.calcite.util.mapping.Mapping;

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
Expand All @@ -76,6 +79,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import static org.apache.calcite.linq4j.Nullness.castNonNull;

Expand All @@ -92,6 +96,16 @@
public abstract class MaterializedViewRule<C extends MaterializedViewRule.Config>
extends RelRule<C> {

/**
* Cache of RelMetadataQuery to MaterializationMetadata.
*/
private final LoadingCache<RelMetadataQuery, MaterializationMetadata>
materializationMetadataCache =
CacheBuilder.newBuilder()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set some parameters?

For example: initialCapacity, maximumSize, refreshAfterWrite, ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set some parameters?

For example: initialCapacity, maximumSize, refreshAfterWrite, ...

Through Gc Roots analysis, I found that weakKeys cannot take effect. Even if I delete MaterializationMetadata#RelMetadataQuery, there is still a strong reference pointing to RelMetadataQuery in MaterializationMetadata#RelOptMaterialization#tableRel, so we should really add two parameters, maximumSize and expireAfterAccess, to materializationMetadataCache. Thanks for your suggestion and review!

.expireAfterAccess(5, TimeUnit.MINUTES)
.maximumSize(1_024)
.build(CacheLoader.from(MaterializationMetadata::new));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 1_024 a typo?


//~ Constructors -----------------------------------------------------------

/** Creates a MaterializedViewRule. */
Expand All @@ -100,7 +114,20 @@ public abstract class MaterializedViewRule<C extends MaterializedViewRule.Config
}

@Override public boolean matches(RelOptRuleCall call) {
return !call.getPlanner().getMaterializations().isEmpty();
final List<RelOptMaterialization> materializations =
call.getPlanner().getMaterializations();
if (materializations.isEmpty()) {
return false;
}
RelMetadataQuery mq = call.getMetadataQuery();
MaterializationMetadata materializationMetadata =
materializationMetadataCache.getUnchecked(mq);
for (RelOptMaterialization materialization : materializations) {
if (materializationMetadata.isValidMaterialization(mq, materialization)) {
return true;
}
}
return false;
}

/**
Expand Down Expand Up @@ -197,7 +224,15 @@ protected void perform(RelOptRuleCall call, @Nullable Project topProject, RelNod

// 3. We iterate through all applicable materializations trying to
// rewrite the given query
MaterializationMetadata materializationMetadata =
materializationMetadataCache.getUnchecked(mq);
for (RelOptMaterialization materialization : materializations) {
// 3.1. View checks before proceeding
if (!materializationMetadata.isValidMaterialization(mq, materialization)) {
// Skip it
continue;
}

RelNode view = materialization.tableRel;
Project topViewProject;
RelNode viewNode;
Expand All @@ -210,11 +245,8 @@ protected void perform(RelOptRuleCall call, @Nullable Project topProject, RelNod
}

// Extract view table references
final Set<RelTableRef> viewTableRefs = mq.getTableReferences(viewNode);
if (viewTableRefs == null) {
// Skip it
continue;
}
final Set<RelTableRef> viewTableRefs =
materializationMetadata.viewTableRefsMap.get(materialization);

// Filter relevant materializations. Currently, we only check whether
// the materialization contains any table that is used by the query
Expand All @@ -231,21 +263,11 @@ protected void perform(RelOptRuleCall call, @Nullable Project topProject, RelNod
continue;
}

// 3.1. View checks before proceeding
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code comments are no longer complete:
3.1 was deleted, but 3.2 was reserved.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code comments are no longer complete: 3.1 was deleted, but 3.2 was reserved.

fixed

if (!isValidPlan(topViewProject, viewNode, mq)) {
// Skip it
continue;
}

// 3.2. Initialize all query related auxiliary data structures
// that will be used throughout query rewriting process
// Extract view predicates
final RelOptPredicateList viewPredicateList =
mq.getAllPredicates(viewNode);
if (viewPredicateList == null) {
// Skip it
continue;
}
materializationMetadata.viewPredicateListMap.get(materialization);
final RexNode viewPred =
simplify.simplifyUnknownAsFalse(
RexUtil.composeConjunction(rexBuilder,
Expand Down Expand Up @@ -1426,6 +1448,61 @@ protected enum MatchModality {
QUERY_PARTIAL
}

/**
* Check if the materialization is valid for this rule.
* If it does, cache the materialization's RelTableRef and RelOptPredicateList.
*/
protected class MaterializationMetadata {
private final Set<RelOptMaterialization> notValidMaterializations = new HashSet<>();
private final Map<RelOptMaterialization, Set<RelTableRef>> viewTableRefsMap = new HashMap<>();
private final Map<RelOptMaterialization, RelOptPredicateList>
viewPredicateListMap = new HashMap<>();

protected boolean isValidMaterialization(
RelMetadataQuery mq, RelOptMaterialization materialization) {
if (notValidMaterializations.contains(materialization)) {
return false;
} else if (viewTableRefsMap.containsKey(materialization)) {
return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use viewTableRefsMap.containsKey(materialization) && viewPredicateListMap.containsKey(materialization) to be more accurate?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use viewTableRefsMap.containsKey(materialization) && viewPredicateListMap.containsKey(materialization) to be more accurate?

Indeed we should.Thanks for your suggestion and review!

} else {
Project topViewProject;
RelNode viewNode;
if (materialization.queryRel instanceof Project) {
topViewProject = (Project) materialization.queryRel;
viewNode = topViewProject.getInput();
} else {
topViewProject = null;
viewNode = materialization.queryRel;
}

final Set<RelTableRef> viewTableRefs = mq.getTableReferences(viewNode);
if (viewTableRefs == null) {
// Skip it
notValidMaterializations.add(materialization);
return false;
}

if (!isValidPlan(topViewProject, viewNode, mq)) {
// Skip it
notValidMaterializations.add(materialization);
return false;
}

final RelOptPredicateList viewPredicateList =
mq.getAllPredicates(viewNode);
if (viewPredicateList == null) {
// Skip it
notValidMaterializations.add(materialization);
return false;
}

viewTableRefsMap.put(materialization, viewTableRefs);
viewPredicateListMap.put(materialization, viewPredicateList);
return true;
}
}
}

/** Rule configuration. */
public interface Config extends RelRule.Config {
/** Whether to generate rewritings containing union if the query results
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,19 @@ protected final MaterializedViewFixture sql(String materialize,
.noMat();
}

@Test void testMultiMaterializationsNoAggregateFuncs() {
fixture("select \"empid\", \"deptno\" from \"emps\" group by \"empid\", \"deptno\"")
.withMaterializations(
ImmutableList.of(
Pair.of(
"select \"empid\", \"deptno\" from \"emps\" group by \"empid\", \"deptno\"",
"MV0"),
Pair.of(
"select \"empid\" from \"emps\" group by \"empid\", \"deptno\"",
"MV1")))
.ok();
}

@Test void testAggregateMaterializationAggregateFuncs1() {
sql("select \"empid\", \"deptno\", count(*) as c, sum(\"empid\") as s\n"
+ "from \"emps\" group by \"empid\", \"deptno\"",
Expand Down