[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17175747#comment-17175747
]
Ruben Q L commented on CALCITE-3923:
Yep, that solves the compilation issue. Thanks [~julianhyde]!
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17175678#comment-17175678
]
Julian Hyde commented on CALCITE-3923:
--
Java lets you define a public field whose type is not public. Java's rules for
this are messed up. You'd hope that Java would treat it as an instance of its
nearest public base class, but apparently it doesn't.
Have you tried instead
{code}
private static final Predicate MY_PREDICATE = ...;
private static final RelOptRule MY_ENUMERABLE_CALC_RULE = ((ConverterRule)
EnumerableRules.ENUMERABLE_CALC_RULE).config
.withConversion(LogicalCalc.class, MY_PREDICATE, Convention.NONE,
EnumerableConvention.INSTANCE, "MyEnumerableCalcRule")
.toRule();
{code}
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17175677#comment-17175677
]
Ruben Q L commented on CALCITE-3923:
You're are right [~julianhyde], I do have some shading going on, but even with
that, shouldn't it work?
I have tried the same code inside a Calcite test, and I get the same problem:
{{Cannot access 'config' in 'org.apache.calcite.plan.RelRule'}}.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17175666#comment-17175666
]
Julian Hyde commented on CALCITE-3923:
--
Judging by the “com.onwbp.” prefix, you have some shading going on.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17175537#comment-17175537
]
Ruben Q L commented on CALCITE-3923:
[~julianhyde], I have a small follow-up question about this change. I am trying
to implement my own version of EnumerableCalcRule, with a special predicate. My
code looks like:
{code:java}
private static final Predicate MY_PREDICATE = ...;
private static final RelOptRule MY_ENUMERABLE_CALC_RULE =
EnumerableRules.ENUMERABLE_CALC_RULE.config
.withConversion(LogicalCalc.class, MY_PREDICATE, Convention.NONE,
EnumerableConvention.INSTANCE, "MyEnumerableCalcRule")
.toRule();
{code}
However, I get a compilation error on
EnumerableRules.ENUMERABLE_CALC_RULE.config: {{Cannot access 'config' in
'com.onwbp.org.apache.calcite.plan.RelRule'}}. My assumption is that this error
comes from the fact that EnumerableCalcRule (and in fact several
Enumerable*Rules) is package private (if I try something similar with a public
rule, e.g. FilterIntoJoinRule via CoreRules.FILTER_INTO_JOIN), 'config' is
accessible.
Is my analysis correct? Is this the proper way to achieve my goal or am I doing
something wrong in my code?
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17167045#comment-17167045
]
Chunwei Lei commented on CALCITE-3923:
--
A minor fix of Site:
[https://github.com/apache/calcite/commit/b047cf5335442f9b5175cba0bb8cf33878539c52].
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17156370#comment-17156370
]
Julian Hyde commented on CALCITE-3923:
--
I've merged the first part as
[23b26b62|https://github.com/apache/calcite/commit/23b26b6287315cc2cd236e705bb651077488fc5c].
The rest will happen after release 1.24.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17154787#comment-17154787
]
Julian Hyde commented on CALCITE-3923:
--
CALCITE-3923 is blocked by CALCITE-4117, which means that certain parts of the
fix can go in only AFTER 1.24 is released.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.25.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17140762#comment-17140762
]
Julian Hyde commented on CALCITE-3923:
--
{quote}Comparing the new approach, the older one seems simpler and more
intuitive.
{quote}
[~Chunwei Lei], At first glance 2 lines of code may seem simpler than 8. But it
ain't necessarily so:
* Try writing the code in an IDE. The 8 lines of code are easy to write,
because of the fluent API.
* The 8 lines of code are easy to read, because each property has a
corresponding 'get' method.
* The 2 two lines of code for a slightly different example might be impossible
to write - if someone has forgotten to add a constructor parameter for
aggregateClass or RelBuilderFactory, or if you want to add a predicate to the
Aggregate operand. Those cases happen all the time - we have had dozens of
small PRs extending rule constructors. In the new code, there is always a way.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17140207#comment-17140207
]
Danny Chen commented on CALCITE-3923:
-
I think most of the lines in
{code:java}
final RelOptRule myRule =
AggregateExpandDistinctAggregatesRule.INSTANCE.config
.withRelBuilderFactory(f)
.withOperandSupplier(b ->
b.operand(MyAggregate.class).anyInputs())
.as(AggregateExpandDistinctAggregatesRule.Config.class)
.withUsingGroupingSets(false)
.toRule();
{code}
are straight-forward to understand, except for the as(class), at first glance,
i don't understand why we need that, shouldn't the
AggregateExpandDistinctAggregatesRule.INSTANCE.config be an instance of
AggregateExpandDistinctAggregatesRule.Config.class ? It seems this interface is
a requirement/limitation of code not a design.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17140190#comment-17140190
]
Chunwei Lei commented on CALCITE-3923:
--
Comparing the new approach, the older one seems simpler and more intuitive.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17139934#comment-17139934
]
Julian Hyde commented on CALCITE-3923:
--
If the downstream project has customized a few rules, they will probably find
that the constructors they are calling are now deprecated. The deprecated
constructor will now create a Config, set its properties from the constructor
arguments, and create a rule.
A typical example is [AggregateExpandDistinctAggregatesRule
|https://github.com/apache/calcite/pull/2024/files#diff-b6682c064afafefacca7520cd0eb06abR105]
which used to be
{code:java}
public AggregateExpandDistinctAggregatesRule(
Class clazz,
boolean useGroupingSets,
RelBuilderFactory relBuilderFactory) {
super(operand(clazz, any()), relBuilderFactory, null);
this.useGroupingSets = useGroupingSets;
}
{code}
and is now
{code:java}
@Deprecated
public AggregateExpandDistinctAggregatesRule(
Class clazz,
boolean useGroupingSets,
RelBuilderFactory relBuilderFactory) {
this(INSTANCE.config.withRelBuilderFactory(relBuilderFactory)
.withOperandSupplier(b ->
b.operand(clazz).anyInputs())
.as(Config.class)
.withUsingGroupingSets(useGroupingSets));
}
{code}
If the project has
{code:java}
final RelOptRule myRule =
new AggregateExpandDistinctAggregatesRule(MyAggregate.class, false, f);
{code}
that code will become
{code:java}
final RelOptRule myRule =
AggregateExpandDistinctAggregatesRule.INSTANCE.config
.withRelBuilderFactory(f)
.withOperandSupplier(b ->
b.operand(MyAggregate.class).anyInputs())
.as(AggregateExpandDistinctAggregatesRule.Config.class)
.withUsingGroupingSets(false)
.toRule();
{code}
If you are using IntelliJ, you can generate most of that code by applying
IntelliJ's "inline" refactoring to the constructor.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17139174#comment-17139174
]
Chunwei Lei commented on CALCITE-3923:
--
Thanks for working on this, [~julianhyde]. Would it lead to big changes for the
downstream project, especially those which customize quite a few rules?
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17134592#comment-17134592
]
Julian Hyde commented on CALCITE-3923:
--
There's now a PR for review: [https://github.com/apache/calcite/pull/2024]
Here's the TL;DR:
Previously, it was not easy to customize, re-use or extend planner rules. If
you wished to customize a rule (i.e. create a new instance of a rule with
different properties) you would have to call the rule's constructor. Frequently
the required constructor did not exist, so we would have to add a new
constructor and deprecate the old one.
After this change, you start off from an instance of the rule, modify its
configuration, and call toRule() on the configuration. (Rule constructors are
now private, because only the configuration ever calls them.)
A good illustration of this is {{DruidRules}}, which used to contain many
sub-classes. Those sub-classes are no longer needed. Old code:
{code:java}
public static final DruidSortProjectTransposeRule SORT_PROJECT_TRANSPOSE =
new DruidSortProjectTransposeRule(RelFactories.LOGICAL_BUILDER);
public static class DruidSortProjectTransposeRule
extends SortProjectTransposeRule {
public DruidSortProjectTransposeRule(RelBuilderFactory relBuilderFactory)
{
super(
operand(Sort.class,
operand(Project.class, operand(DruidQuery.class, none(,
relBuilderFactory, null);
}
}
{code}
New code:
{code:java}
public static final SortProjectTransposeRule SORT_PROJECT_TRANSPOSE =
SortProjectTransposeRule.INSTANCE.config
.withOperandFor(Sort.class, Project.class, DruidQuery.class)
.toRule();
{code}
The change maintains backwards compatibility to a large degree. In a few
places, I had to change rule instances from type {{RelOptRule}} to
{{Supplier}}, to avoid deadlocks during class loading. For
instance, instead of writing {{FilterJoinRule.FILTER_ON_JOIN}} you must now
write {{FilterJoinRule.FILTER_ON_JOIN.get()}}.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.24.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17097081#comment-17097081
]
Haisheng Yuan commented on CALCITE-3923:
topProject convention should be always same with botttomProject convention. So
the check is not needed.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17097074#comment-17097074
]
Xiening Dai commented on CALCITE-3923:
--
I am thinking about this. Once we have the RelBuilder change in place, we could
do such in ProjectMerge rule for example -
{code:java}
RelBuilder relBuilder = call.builder();
if (topProject.getConvention() == bottomProject.getConvention()) {
relBuilder = topProject.getConvention().transformRelBuilder(relBuilder);
}
{code}
Based on my test, a simple change like this will reduce 50% project merge rule
firings for an N-way join query. But I don't like the additional check here,
maybe we should provide "target convention" as a config parameter?
I understand this might not necessarily be part of your change. But we might
need to add that to config some point in the future.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17096904#comment-17096904
]
Julian Hyde commented on CALCITE-3923:
--
The single convention rule idea is worth pursuing. It is one possible solution
to the "RelNode.copy problem". (See also CALCITE-2064.) But unless solving it
completely removes the need to copy rule instances - which I think it won't -
this change will still be needed. So, can we please treat it as an orthogonal
issue?
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17087988#comment-17087988
]
Julian Hyde commented on CALCITE-3923:
--
The {{ImmutableBeans.Property}} annotation has a {{required}} attribute. If we
set {{required}} then {{preserveExprCondition}} will throw when called.
But maybe that's too late; we should add a {{Config.isValid(Litmus)}} method
that can be called in the rule constructor, and will automatically validate all
properties. It would call a new {{static ImmutableBeans.isValid(Object,
Litmus)}} method.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17085192#comment-17085192
]
Julian Hyde commented on CALCITE-3923:
--
There are now several commits on that branch. The main commit to review is
[c9fd10d5|https://github.com/julianhyde/calcite/commit/c9fd10d5bb5a53bade27474c9fb9c4bbdb726708].
Commit
[2ede648a|https://github.com/julianhyde/calcite/commit/2ede648ad07aaafb6aaa05a0b67b54f29b678ccb]
proposes a new way to create the immutable trees of {{RelOptRuleOperand}}
objects. It is separate but complementary, because operands are a central part
of the definition of a rule, and operands often need to be tweaked (e.g.
changing Filter.class to LogicalFilter.class) when you create a derivative rule.
The other commits illustrate how the API changes would affect the code base.
In
[5345c909|https://github.com/julianhyde/calcite/commit/5345c90974ccdd4d1af02236273b001e870006bb],
I'd love someone to check whether weak keys, soft values are the right way to
ensure that handlers can be garbage-collected when a class is unloaded.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-3923) Refactor how planner rules are parameterized
[
https://issues.apache.org/jira/browse/CALCITE-3923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084326#comment-17084326
]
Julian Hyde commented on CALCITE-3923:
--
I have created a prototype in my
[3923-rule-config|https://github.com/julianhyde/calcite/tree/3923-rule-config]
branch. Can some people review? Let's discuss.
> Refactor how planner rules are parameterized
>
>
> Key: CALCITE-3923
> URL: https://issues.apache.org/jira/browse/CALCITE-3923
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> People often want different variants of planner rules. An example is
> {{FilterJoinRule}}, which has a 'boolean smart’ parameter, a predicate (which
> returns whether to pull up filter conditions), operands (which determine the
> precise sub-classes of {{RelNode}} that the rule should match) and a
> {{RelBuilderFactory}} (which controls the type of {{RelNode}} created by this
> rule).
> Suppose you have an instance of {{FilterJoinRule}} and you want to change
> {{smart}} from true to false. The {{smart}} parameter is immutable (good!)
> but you can’t easily create a clone of the rule because you don’t know the
> values of the other parameters. Your instance might even be (unbeknownst to
> you) a sub-class with extra parameters and a private constructor.
> So, my proposal is to put all of the config information of a {{RelOptRule}}
> into a single {{config}} parameter that contains all relevant properties.
> Each sub-class of {{RelOptRule}} would have one constructor with just a
> ‘config’ parameter. Each config knows which sub-class of {{RelOptRule}} to
> create. Therefore it is easy to copy a config, change one or more properties,
> and create a new rule instance.
> Adding a property to a rule’s config does not require us to add or deprecate
> any constructors.
> The operands are part of the config, so if you have a rule that matches a
> {{EnumerableFilter}} on an {{EnumerableJoin}} and you want to make it match
> an {{EnumerableFilter}} on an {{EnumerableNestedLoopJoin}}, you can easily
> create one with one changed operand.
> The config is immutable and self-describing, so we can use it to
> automatically generate a unique description for each rule instance.
> (See the email thread [[DISCUSS] Refactor how planner rules are
> parameterized|https://lists.apache.org/thread.html/rfdf6f9b7821988bdd92b0377e3d293443a6376f4773c4c658c891cf9%40%3Cdev.calcite.apache.org%3E].)
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
