[
https://issues.apache.org/jira/browse/CALCITE-7129?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alessandro Solimando updated CALCITE-7129:
------------------------------------------
Description:
-In the context of CALCITE-7125, I have introduced a variant of
[IntersectToDistinctRule|https://github.com/apache/calcite/blob/4fa0d5bbe987c4ae0779ca434fb71e1ae8637b77/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java]
with a non-default configuration.
When trying to use the rule with the new instructions for iq tests introduced
in CALCITE-6335 (and refined in CALCITE-6998 to support rules with multiple
configurations), the default configuration was being picked up,
_CoreRules.INTERSECT_TO_DISTINCT_ instead of that of
_CoreRules.INTERSECT_TO_DISTINCT_NO_AGGREGATE_PUSHDOWN_, and it took me a while
to realize what was happening.
Basically, I forgot to add the "@RuleConfig" annotation to the non-standard
configuration of _CoreRules.INTERSECT_TO_DISTINCT_NO_AGGREGATE_PUSHDOWN_, and
the test was silently using that of _CoreRules.INTERSECT_TO_DISTINCT_ instead,
I noticed only because I was specifically looking at plan differences between
the two.
Not sure it's doable easily, but ideally we should have a check somewhere that
is throwing an error if multiple variants of the same rule are present, and the
non-default configuration lacks the required annotation.
I am not sure, but more rules currently used might suffer from this problem.
-
The annotation can be safely removed without loss of functionality, so the
ticket finally aims at removing it entirely, no tests are needed as the change
is covered by those of CALCITE-6998 (planner.iq).
was:
In the context of CALCITE-7125, I have introduced a variant of
[IntersectToDistinctRule|https://github.com/apache/calcite/blob/4fa0d5bbe987c4ae0779ca434fb71e1ae8637b77/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java]
with a non-default configuration.
When trying to use the rule with the new instructions for iq tests introduced
in CALCITE-6335 (and refined in CALCITE-6998 to support rules with multiple
configurations), the default configuration was being picked up,
_CoreRules.INTERSECT_TO_DISTINCT_ instead of that of
_CoreRules.INTERSECT_TO_DISTINCT_NO_AGGREGATE_PUSHDOWN_, and it took me a while
to realize what was happening.
Basically, I forgot to add the "@RuleConfig" annotation to the non-standard
configuration of _CoreRules.INTERSECT_TO_DISTINCT_NO_AGGREGATE_PUSHDOWN_, and
the test was silently using that of _CoreRules.INTERSECT_TO_DISTINCT_ instead,
I noticed only because I was specifically looking at plan differences between
the two.
Not sure it's doable easily, but ideally we should have a check somewhere that
is throwing an error if multiple variants of the same rule are present, and the
non-default configuration lacks the required annotation.
I am not sure, but more rules currently used might suffer from this problem.
> Drop @RuleConfig annotation used in Quidem tests
> ------------------------------------------------
>
> Key: CALCITE-7129
> URL: https://issues.apache.org/jira/browse/CALCITE-7129
> Project: Calcite
> Issue Type: Improvement
> Components: tests
> Affects Versions: 1.40.0
> Reporter: Alessandro Solimando
> Priority: Major
> Labels: pull-request-available
>
> -In the context of CALCITE-7125, I have introduced a variant of
> [IntersectToDistinctRule|https://github.com/apache/calcite/blob/4fa0d5bbe987c4ae0779ca434fb71e1ae8637b77/core/src/main/java/org/apache/calcite/rel/rules/IntersectToDistinctRule.java]
> with a non-default configuration.
> When trying to use the rule with the new instructions for iq tests introduced
> in CALCITE-6335 (and refined in CALCITE-6998 to support rules with multiple
> configurations), the default configuration was being picked up,
> _CoreRules.INTERSECT_TO_DISTINCT_ instead of that of
> _CoreRules.INTERSECT_TO_DISTINCT_NO_AGGREGATE_PUSHDOWN_, and it took me a
> while to realize what was happening.
> Basically, I forgot to add the "@RuleConfig" annotation to the non-standard
> configuration of _CoreRules.INTERSECT_TO_DISTINCT_NO_AGGREGATE_PUSHDOWN_, and
> the test was silently using that of _CoreRules.INTERSECT_TO_DISTINCT_
> instead, I noticed only because I was specifically looking at plan
> differences between the two.
> Not sure it's doable easily, but ideally we should have a check somewhere
> that is throwing an error if multiple variants of the same rule are present,
> and the non-default configuration lacks the required annotation.
> I am not sure, but more rules currently used might suffer from this problem.
> -
> The annotation can be safely removed without loss of functionality, so the
> ticket finally aims at removing it entirely, no tests are needed as the
> change is covered by those of CALCITE-6998 (planner.iq).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
