hsyuan commented on a change in pull request #1860: [CALCITE-2970] Add
abstractConverter only between derived and required traitset
URL: https://github.com/apache/calcite/pull/1860#discussion_r398088611
##########
File path: core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
##########
@@ -160,107 +162,106 @@ void obliterateRelNode(RelNode rel) {
public RelSubset add(RelNode rel) {
assert equivalentSet == null : "adding to a dead set";
final RelTraitSet traitSet = rel.getTraitSet().simplify();
- final RelSubset subset = getOrCreateSubset(rel.getCluster(), traitSet);
+ final RelSubset subset = getOrCreateSubset(
+ rel.getCluster(), traitSet, rel.isEnforcer());
subset.add(rel);
return subset;
}
+ /**
+ * If the subset is required, convert derived subsets to this subset.
+ * Otherwise, convert this subset to required subsets in this RelSet.
+ * The subset can be both required and derived.
+ */
private void addAbstractConverters(
- VolcanoPlanner planner, RelOptCluster cluster, RelSubset subset, boolean
subsetToOthers) {
- // Converters from newly introduced subset to all the remaining one (vice
versa), only if
- // we can convert. No point adding converters if it is not possible.
- for (RelSubset other : subsets) {
+ RelOptCluster cluster, RelSubset subset, boolean required) {
+ List<RelSubset> others = subsets.stream().filter(
+ n -> required ? n.isDerived() : n.isRequired())
+ .collect(Collectors.toList());
+ for (RelSubset other : others) {
assert other.getTraitSet().size() == subset.getTraitSet().size();
+ RelSubset from = subset;
+ RelSubset to = other;
+
+ if (required) {
+ from = other;
+ to = subset;
+ }
- if ((other == subset)
- || (subsetToOthers
- && !subset.getConvention().useAbstractConvertersForConversion(
- subset.getTraitSet(), other.getTraitSet()))
- || (!subsetToOthers
- && !other.getConvention().useAbstractConvertersForConversion(
- other.getTraitSet(), subset.getTraitSet()))) {
+ if (from == to || !from.getConvention()
+ .useAbstractConvertersForConversion(
+ from.getTraitSet(), to.getTraitSet())) {
continue;
}
final ImmutableList<RelTrait> difference =
- subset.getTraitSet().difference(other.getTraitSet());
+ to.getTraitSet().difference(from.getTraitSet());
- boolean addAbstractConverter = true;
- int numTraitNeedConvert = 0;
-
- for (RelTrait curOtherTrait : difference) {
- RelTraitDef traitDef = curOtherTrait.getTraitDef();
- RelTrait curRelTrait = subset.getTraitSet().getTrait(traitDef);
-
- if (curRelTrait == null) {
- addAbstractConverter = false;
- break;
- }
+ boolean needsConverter = false;
- assert curRelTrait.getTraitDef() == traitDef;
-
- boolean canConvert = false;
- boolean needConvert = false;
- if (subsetToOthers) {
- // We can convert from subset to other. So, add converter with
subset as child and
- // traitset as the other's traitset.
- canConvert = traitDef.canConvert(
- cluster.getPlanner(), curRelTrait, curOtherTrait, subset);
- needConvert = !curRelTrait.satisfies(curOtherTrait);
- } else {
- // We can convert from others to subset.
- canConvert = traitDef.canConvert(
- cluster.getPlanner(), curOtherTrait, curRelTrait, other);
- needConvert = !curOtherTrait.satisfies(curRelTrait);
- }
+ for (RelTrait fromTrait : difference) {
+ RelTraitDef traitDef = fromTrait.getTraitDef();
+ RelTrait toTrait = to.getTraitSet().getTrait(traitDef);
- if (!canConvert) {
- addAbstractConverter = false;
+ if (toTrait == null || !traitDef.canConvert(
+ cluster.getPlanner(), fromTrait, toTrait, from)) {
+ needsConverter = false;
break;
}
- if (needConvert) {
- numTraitNeedConvert++;
+ if (!fromTrait.satisfies(toTrait)) {
+ needsConverter = true;
}
}
- if (addAbstractConverter && numTraitNeedConvert > 0) {
- if (subsetToOthers) {
- final AbstractConverter converter =
- new AbstractConverter(cluster, subset, null,
other.getTraitSet());
- planner.register(converter, other);
- } else {
- final AbstractConverter converter =
- new AbstractConverter(cluster, other, null,
subset.getTraitSet());
- planner.register(converter, subset);
- }
+ if (needsConverter) {
+ final AbstractConverter converter =
+ new AbstractConverter(cluster, from, null, to.getTraitSet());
+ cluster.getPlanner().register(converter, to);
}
}
}
+ RelSubset getOrCreateSubset(RelOptCluster cluster, RelTraitSet traits) {
+ return getOrCreateSubset(cluster, traits, false /* required */);
+ }
+
Review comment:
done, thanks
----------------------------------------------------------------
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
With regards,
Apache Git Services
