[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17117935#comment-17117935 ] Xiening Dai commented on CALCITE-3972: -- I agree collation can be expressed as a required property instead of a RelNode. But that would be a much bigger change to current design. I will follow Haisheng's suggestion to update this rule. Thx. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17116960#comment-17116960
]
Haisheng Yuan commented on CALCITE-3972:
Agree with [~rkondakov]. My take is more radical, {{LogicalSort}} should not
exist at all. Enforcers should only exist in physical world, not logical world.
> Allow RelBuilder to create RelNode with convention and use it for trait
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
> Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Time Spent: 2h
> Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the
> underlying RelFactories and create a new builder.
> 3. Use the new interface in RelCollationTraitDef for converting into
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17116954#comment-17116954 ] Roman Kondakov commented on CALCITE-3972: - I agree with [~hyuan]. I also think we should not register logical converters (LogicalSort or LogicalExchange) in the planner's MEMO at all. As it implemented in my pet cascades-style optimizer. It helps to avoid not-needed multiple RelSets merging and rules firing. All physical properties should be enforced by physical converters only at the final step of Group (RelSet) optimization. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17116911#comment-17116911
]
Haisheng Yuan commented on CALCITE-3972:
The fact that Sort can participate rule matching is the culprit.
Sort changes the relation's physical property, but doesn't change the logical
property.
Use
[testSortJoinTranspose2|https://github.com/apache/calcite/commit/0715f5b55f363a58e3dd8c20caac0024e19be413#diff-de15ea9da479ca31d38de70365967392R4070]
as example,
{code:java}
Before:
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], DEPTNO0=[$9], NAME=[$10])
LogicalSort(sort0=[$10], dir0=[ASC])
LogicalJoin(condition=[=($7, $9)], joinType=[right])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalProject(DEPTNO=[$0], NAME=[$1])
LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
After:
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4],
SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], DEPTNO0=[$9], NAME=[$10])
LogicalSort(sort0=[$10], dir0=[ASC])
LogicalJoin(condition=[=($7, $9)], joinType=[right])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
LogicalSort(sort0=[$1], dir0=[ASC])
LogicalProject(DEPTNO=[$0], NAME=[$1])
LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
{code}
If we combine the sort with any operator in the original plan, the logical
properties are all the same. After the rule execution, LogicalJoin has a new
right input, even the sort in the right input can be the same RelSet as
LogicalProject (unfortunately it isn't), the new join right input changed (the
logical join is requesting a collation on right input), the join's digest
changed, it will be viewed as a whole new join, then will apply all the logical
transformations that it can apply.
Although the rule above only applies on outer join, the same problem happens on
SortProjectTransposeRule.
Now come back to the problem in JDBCTest.testJoinManyWays(),
JoinPushThroughJoinRule's one rule operand is RelNode.class, which means any
new node in the join's input RelSet will trigger the rule. But in this rule, we
don't care about what exact relnode it is, we just want the whole group as a
placeholder. Any new logical sort, physical sort, and abstract converter will
all trigger the matches of JoinPushThroughJoinRule. This is extremely
unnecessary.
If we change
{code:java}
operand(RelNode.class, any())),
{code}
to
{code:java}
operandJ(RelNode.class, null, n -> !n.isEnforcer(), any())),
{code}
It will achieve the same effect as generating EnumerableSort directly, but
still generating LogicalSort in RelCollationTraitDef, without affecting rules
like, SortProjectTranspose, SortJoinTranspose, SortJoinCopy.
The total rule apply count of JoinPushThroughJoinRule cut from 9000 to 900,
reduced by 90%. This will again reduce the ProjectMergeRule a lot, because
every join reorder generate at least a new LogicalProject in Calcite.
Now the rule count is:
{code:java}
Rules
Attempts Time (us)
ProjectMergeRule:force_mode
14,064 2,680,177
EnumerableProjectRule(in:NONE,out:ENUMERABLE)
974 271,608
JoinPushThroughJoinRule:left
449 209,768
JoinPushThroughJoinRule:right
449 2,949
AggregatePullUpConstantsRule
291 17,947
AggregateProjectMergeRule
277 83,288
ProjectFilterTransposeRule
207 30,300
EnumerableJoinRule(in:NONE,out:ENUMERABLE)
108 70,179
EnumerableMergeJoinRule(in:NONE,out:ENUMERABLE)
108 46,111
JoinPushExpressionsRule
108 10,807
{code}
> Allow RelBuilder to create RelNode with convention and use it for trait
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
> Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Time Spent: 2h
> Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the
> underlying RelFactories
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17115004#comment-17115004 ] Xiening Dai commented on CALCITE-3972: -- [~zabetak] this is separate, and has nothing to do with CALCITE-3997. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17114988#comment-17114988 ] Stamatis Zampetakis commented on CALCITE-3972: -- [~xndai] Is it this change alone the combination with the commit of CALCITE-3997? > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17114280#comment-17114280 ] Xiening Dai commented on CALCITE-3972: -- We discover that with this change there are a few rules (SortProjectTranspose, SortJoinTranspose, SortJoinCopy, etc) which choose to match LogicalSort won't fire on the EnumerableSort enforcer. Although all the UTs are passed, potentially there are missing opportunities for better plans. And it cannot be fixed by simply adding an enumerable instance of the rule (to match Enumerable join and sort instead) because the rule implementation today doesn't consider join input trait requirement, and would generate a wrong alternative. So it feels to me that to solve this problem completely, we should use the trait propagation framework instead (CALCITE-3896) and deprecate these rules in long run. So part of this change - generating EnumerableSort during trait convert, would have to be removed. But I still see other usage scenarios of RelBuilder with conventions. I would like to still commit this change, so I can work on subsequent issues (such as CALCITE-3455). > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17105678#comment-17105678 ] Julian Hyde commented on CALCITE-3972: -- We've discussed this on the dev list. We prefer the simplicity of having releases on the master branch, even though development needs to pause during the release process. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17105666#comment-17105666 ] Xiening Dai commented on CALCITE-3972: -- Going forward, do you think it makes sense to fork a release branch while keeping master for daily development? > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17105663#comment-17105663 ] Haisheng Yuan commented on CALCITE-3972: Make sense. Will keep master for regression fix only until the release is out. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17105656#comment-17105656 ] Julian Hyde commented on CALCITE-3972: -- As release manager, it's your call. But to be fair to others, you should also allow PRs that are in a similar state. And that might add time + instability to the release process. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17105638#comment-17105638 ] Haisheng Yuan commented on CALCITE-3972: 1.23.0 RC0 is cancelled. Is there any chance we can get this into 1.23.0 rc1? > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17104091#comment-17104091
]
Julian Hyde commented on CALCITE-3972:
--
Simplest thing for the user would be
{{RelBuilder.adoptConvention(convention)}}. User isn't interested in
{{RelFactories}} or {{Struct}}. A {{Convention}} ought to know what factories
it needs.
I added {{Struct}} to make the implementation more efficient - so that only one
pointer would need to be assigned when creating a {{RelBuilder}} rather than a
dozen or so, one for each factory - and I don't think that {{Struct}} should
become a user concept.
> Allow RelBuilder to create RelNode with convention and use it for trait
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
> Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the
> underlying RelFactories and create a new builder.
> 3. Use the new interface in RelCollationTraitDef for converting into
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103644#comment-17103644 ] Xiening Dai commented on CALCITE-3972: -- The thing is you don't want people to update RelFactories.java whenever there's a new convention available. Convention#getRelFactories seperates the logics, and can totally lives in user code base, instead of Calcite core. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 1h 10m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103635#comment-17103635 ] Danny Chen commented on CALCITE-3972: - Instead of add a `getRelFactories` interface to Convention, how about add a tool method `RelFactories#getRelFactories(Convention)`, it seems more straight forward because `RelFactories` is the factory to create all kinds of factory STRUCT. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 1h 10m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17102747#comment-17102747 ] Xiening Dai commented on CALCITE-3972: -- RelFactories.StructwithRelFactories() is not really needed, which I removed. adoptConvention() is rearmed to getRelFactories() to return RelFactories.Struct. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 50m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17102464#comment-17102464
]
Stamatis Zampetakis commented on CALCITE-3972:
--
Do we need all three new public API methods?
{noformat}
RelBuilder#withRelFactories(RelFactories.Struct struct)
RelBuilder#adoptConvention(Convention convention)
Convention#transformRelBuilder(RelBuilder oldRelBuilder)
{noformat}
Furthermore all new APIs should have associated tests.
> Allow RelBuilder to create RelNode with convention and use it for trait
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
> Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the
> underlying RelFactories and create a new builder.
> 3. Use the new interface in RelCollationTraitDef for converting into
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17102245#comment-17102245 ] Xiening Dai commented on CALCITE-3972: -- [~hyuan] [~julianhyde] if you have a chance, can you please take another look? Thanks. https://github.com/apache/calcite/pull/1884/files > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 50m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101260#comment-17101260 ] Xiening Dai commented on CALCITE-3972: -- Tried push/pop approach. It's a little tricky to implement. The push() would have to call Convention#transformRelBuilder, but pop() would directly pop the RelFactory stack. I can see some inconsistency here. On the other hand, unlike the RelNode stack, convention doesn't really need to maintain the previous status, which also makes it easier to jump from one to another (without having to pop() multiple times). I keep adoptConvention() interface. Latest PR is here - https://github.com/apache/calcite/pull/1884/files Thanks. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 40m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101221#comment-17101221 ] Xiening Dai commented on CALCITE-3972: -- [~hyuan] I also gave an example in CALCITE-2970. [~julianhyde] I lean towards keeping convention as part of the mutable state so the example would just work. Otherwise we would have to break up the steps. I can try 'pop/push' syntax instead. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 40m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101205#comment-17101205 ] Julian Hyde commented on CALCITE-3972: -- RelBuilder isn't just used in rules. It is used for larger tasks, like SqlToRelConverter. And it is the recommended translator if someone has built a front end based on their own query language. We want RelBuilder's lifecycle to be simple and predictable when translating a complex query into dozens or hundreds of RelNodes. There might be multiple conventions in such queries. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 40m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101186#comment-17101186 ] Haisheng Yuan commented on CALCITE-3972: I am curious when do we need to preserve the stack while switching convention. Even in ProjectMergeRule, there is only 1 convention needed for the RelBuilder for each rule execution. It is either logical or physical RelBuilder. We won't need a RelBuilder for 2 conventions in the same rule instance at the same time. I think I am missing something. Any use cases? > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > Time Spent: 40m > Remaining Estimate: 0h > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101170#comment-17101170
]
Julian Hyde commented on CALCITE-3972:
--
I don't mind if {{RelBuilder#transform(UnaryOperator transform)}} - or
a variant of it - preserves the stack. I planned to go in that direction anyway.
I agree that, whatever we decide for the semantics of {{adoptConvention}}, your
example {code}RelNode root =
builder.adoptConvention(EnumerableConvention.INSTANCE)
.scan("EMP")
.filter(
builder.equals(builder.field("DEPTNO"), builder.literal(20)))
.build();{code} should work.
If we're going to make convention part of the mutable state, perhaps a
'push'/'pop' model is better than a 'set' model. Then it's easier to get back
to the previous convention.
> Allow RelBuilder to create RelNode with convention and use it for trait
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
> Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the
> underlying RelFactories and create a new builder.
> 3. Use the new interface in RelCollationTraitDef for converting into
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101144#comment-17101144
]
Xiening Dai commented on CALCITE-3972:
--
If convention is considered as part of the config, then it looks weird that
adoptConvention() preserves builder stack while
RelBuilder#transform(UnaryOperator transform) does not.
Furthermore, a common usage pattern like below won't work because the builder
object referenced by filter is not the one after adoptConvention() -
{code:java}
RelNode root =
builder.adoptConvention(EnumerableConvention.INSTANCE)
.scan("EMP")
.filter(
builder.equals(builder.field("DEPTNO"), builder.literal(20)))
.build();
{code}
In my view, if we'd like to support switching convention in the middle of
building nodes, we should treat convention as a mutable state - just like
stake. Or we abandon such capability, and don't preserve stake when switching
convention. In that case, we don't need adoptConvention(), modifying current
transform() should be enough.
> Allow RelBuilder to create RelNode with convention and use it for trait
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
> Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the
> underlying RelFactories and create a new builder.
> 3. Use the new interface in RelCollationTraitDef for converting into
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[
https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17101043#comment-17101043
]
Julian Hyde commented on CALCITE-3972:
--
[~xndai] wrote:
bq. Also the RelBuilder is not immutable today. The stack can change after it's
passed into a function. So it's not a new problem...
I acknowledged that earlier in the comment thread. The stack of {{RelNode}}s
(and ancillary data like aliases) is mutable. The config is not mutable. This
is intentional - I consider config and the rel stack to be different classes of
state.
Any class called 'builder' of course has mutable state. Builders are useful
because they allow mutation in a controlled way.
bq. I mean with the adoptConvention() we create a new builder every time. It
could be problematic if this gets call frequently (such in high frequent rules).
Creating a {{RelBuilder}} is fairly efficient - 7 fields to assign, two object
creations ({{ArrayDeque}} and {{RexSimplify}}). A typical planning process
might fire 10k rules. 10k object creations is not going to move the dial.
> Allow RelBuilder to create RelNode with convention and use it for trait
> convert
> ---
>
> Key: CALCITE-3972
> URL: https://issues.apache.org/jira/browse/CALCITE-3972
> Project: Calcite
> Issue Type: Bug
>Reporter: Xiening Dai
>Assignee: Xiening Dai
>Priority: Major
> Fix For: 1.23.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> 1. Provide Convention.transformRelBuilder() to transform an existing
> RelBuilder into one with specific convention.
> 2. RelBuilder provides withRelFactories() method to allow caller swap the
> underlying RelFactories and create a new builder.
> 3. Use the new interface in RelCollationTraitDef for converting into
> RelCollation traits
> We can avoid ~1/3 of total rule firings in a N way join case with this change.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100394#comment-17100394 ] Chunwei Lei commented on CALCITE-3972: -- Add the PR link: [https://github.com/apache/calcite/pull/1884.] > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100341#comment-17100341 ] Haisheng Yuan commented on CALCITE-3972: [~julianhyde] [~zabetak] The new issue is here. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-3972) Allow RelBuilder to create RelNode with convention and use it for trait convert
[ https://issues.apache.org/jira/browse/CALCITE-3972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100340#comment-17100340 ] Haisheng Yuan commented on CALCITE-3972: Spin out issue of CALCITE-2970. > Allow RelBuilder to create RelNode with convention and use it for trait > convert > --- > > Key: CALCITE-3972 > URL: https://issues.apache.org/jira/browse/CALCITE-3972 > Project: Calcite > Issue Type: Bug >Reporter: Xiening Dai >Assignee: Xiening Dai >Priority: Major > Fix For: 1.23.0 > > > 1. Provide Convention.transformRelBuilder() to transform an existing > RelBuilder into one with specific convention. > 2. RelBuilder provides withRelFactories() method to allow caller swap the > underlying RelFactories and create a new builder. > 3. Use the new interface in RelCollationTraitDef for converting into > RelCollation traits > We can avoid ~1/3 of total rule firings in a N way join case with this change. -- This message was sent by Atlassian Jira (v8.3.4#803005)
