Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-6010

RelRule.Config requires too much ceremony for trivial cases

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Open
    • Major
    • Resolution: Unresolved
    • None
    • None
    • core
    • None

    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

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              vladimirsitnikov Vladimir Sitnikov
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated: