[ 
https://issues.apache.org/jira/browse/CALCITE-6010?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Vladimir Sitnikov updated CALCITE-6010:
---------------------------------------
    Description: 
It takes a lot to implement RelRule.Config even for trivial cases when no extra 
configuration options needed.

Sample:

https://github.com/apache/calcite/blob/d9dd3ac8a9f695e111a0a5e77f45b61b90f4b5b6/core/src/main/java/org/apache/calcite/rel/rules/TableScanRule.java#L72C35-L78

It requires users:
1) Use immutables or implement Config manually. Adding immutables processor 
adds compile-time overhead, and implementing the interface manually is not 
trivial
2) Implement CustomRule(Config) constructor
3) Implement Config.toRule() by calling CustomRule(Config)

---

I suggest:
1) Drop method Config.toRule(), and suggest users to call new ...Rule(config) 
directly. Config.toRule() adds no value, and it creates excessive coupling of 
Config with the Rule.
2) Provide default Config implementation along with Calcite. For instance 
DefaultConfig.EMPTY, DefaultConfigBuilder...
3) Use composition for custom configurations, in other words, let custom rules 
have their own attributes, and one of the attributes could be default config.
For instance: {{data class AggregateExpandDistinctAggregatesRule.Config(config 
RelRule.Config, usingGroupingSets Boolean)}}

It would make it easier for users to implement config objects, and it would 
reduce the code size (generated bytecode and native image size) as the current 
Calcite approach duplicates the same "Config" methods like withOperandSupplier 
across all the Config implementations 

  was:
It takes a lot to implement RelRule.Config even for trivial cases when no extra 
configuration options needed.

Sample:

https://github.com/apache/calcite/blob/d9dd3ac8a9f695e111a0a5e77f45b61b90f4b5b6/core/src/main/java/org/apache/calcite/rel/rules/TableScanRule.java#L72C35-L78

It requires users:
1) Use immutables or implement Config manually. Adding immutables processor 
adds compile-time overhead, and implementing the interface manually is not 
trivial
2) Implement CustomRule(Config) constructor
3) Implement Config.toRule() by calling CustomRule(Config)

---

I suggest:
1) Drop method Config.toRule(), and suggest users to call new ...Rule(config) 
directly.
2) Provide default Config implementation along with Calcite. For instance 
DefaultConfig.EMPTY, DefaultConfigBuilder...
3) Use composition for custom configurations, in other words, let custom rules 
have their own attributes, and one of the attributes could be default config.
For instance: {{data class AggregateExpandDistinctAggregatesRule.Config(config 
RelRule.Config, usingGroupingSets Boolean)}}

It would make it easier for users to implement config objects, and it would 
reduce the code size (generated bytecode and native image size) as the current 
Calcite approach duplicates the same "Config" methods like withOperandSupplier 
across all the Config implementations 


> RelRule.Config requires too much ceremony for trivial cases
> -----------------------------------------------------------
>
>                 Key: CALCITE-6010
>                 URL: https://issues.apache.org/jira/browse/CALCITE-6010
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>            Reporter: Vladimir Sitnikov
>            Priority: Major
>
> It takes a lot to implement RelRule.Config even for trivial cases when no 
> extra configuration options needed.
> Sample:
> https://github.com/apache/calcite/blob/d9dd3ac8a9f695e111a0a5e77f45b61b90f4b5b6/core/src/main/java/org/apache/calcite/rel/rules/TableScanRule.java#L72C35-L78
> It requires users:
> 1) Use immutables or implement Config manually. Adding immutables processor 
> adds compile-time overhead, and implementing the interface manually is not 
> trivial
> 2) Implement CustomRule(Config) constructor
> 3) Implement Config.toRule() by calling CustomRule(Config)
> ---
> I suggest:
> 1) Drop method Config.toRule(), and suggest users to call new ...Rule(config) 
> directly. Config.toRule() adds no value, and it creates excessive coupling of 
> Config with the Rule.
> 2) Provide default Config implementation along with Calcite. For instance 
> DefaultConfig.EMPTY, DefaultConfigBuilder...
> 3) Use composition for custom configurations, in other words, let custom 
> rules have their own attributes, and one of the attributes could be default 
> config.
> For instance: {{data class 
> AggregateExpandDistinctAggregatesRule.Config(config RelRule.Config, 
> usingGroupingSets Boolean)}}
> It would make it easier for users to implement config objects, and it would 
> reduce the code size (generated bytecode and native image size) as the 
> current Calcite approach duplicates the same "Config" methods like 
> withOperandSupplier across all the Config implementations 



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

Reply via email to