Skip to content

Comments

[CALCITE-3948] Improve operand's RelSubset matching handling in VolcanoRuleCall#1935

Closed
hbtoo wants to merge 1 commit intoapache:masterfrom
hbtoo:CALCITE-3948
Closed

[CALCITE-3948] Improve operand's RelSubset matching handling in VolcanoRuleCall#1935
hbtoo wants to merge 1 commit intoapache:masterfrom
hbtoo:CALCITE-3948

Conversation

@hbtoo
Copy link
Contributor

@hbtoo hbtoo commented Apr 21, 2020

if (clazz == RelSubset.class) {
// Only supports matching subset for leaf operand
assert children.size() == 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we don't need this, rule operand should be HepPlanner/VolcanoPlanner agnostic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, removed

public Stream<RelSubset> getSubsetsSatisfingThis() {
return set.subsets.stream()
.filter(s -> s.getTraitSet().satisfies(traitSet));
}
Copy link
Member

Choose a reason for hiding this comment

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

should be non-public

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we add this as a public api? This one is very similar to getParentRels(), getRels(), except that it is looking for relationships of RelSubset rather than rels.

Copy link
Member

Choose a reason for hiding this comment

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

satisfying?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@hbtoo hbtoo force-pushed the CALCITE-3948 branch 2 times, most recently from d755d67 to 18dd467 Compare April 22, 2020 03:19
@danny0405 danny0405 changed the title [CALCITE-3948] improve operand's RelSubset matching handling in VolcanoRuleCall [CALCITE-3948] Improve operand's RelSubset matching handling in VolcanoRuleCall Apr 22, 2020
}

@API(since = "1.23", status = API.Status.EXPERIMENTAL)
public Stream<RelSubset> getSatisfyingSubsets() {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add comments?

@hsyuan hsyuan added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Apr 23, 2020
@hsyuan hsyuan closed this in d3286ad Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants