[ 
https://issues.apache.org/jira/browse/CALCITE-7129?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18012350#comment-18012350
 ] 

Stamatis Zampetakis commented on CALCITE-7129:
----------------------------------------------

I would prefer to invest effort on trying to see if we can get rid off the 
{{RuleConfig}} annotation rather than complicating the code even more just for 
the sake of testing purposes. 

The annotation was introduced by CALCITE-6998 but from the description of the 
ticket it is not clear to me why it was necessary to do so. The ticket mentions 
something about concurrent loading of rules but its not obvious what the 
problem is.

> Check that all non-default rule configuration are annotated with @RuleConfig
> ----------------------------------------------------------------------------
>
>                 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
>
> 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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to