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

Modify Frameworks to allow Schema to be re-used

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      `Frameworks` currently assumes that a `Schema` can only be created inside a connection, and therefore each optimizer run needs to create a new schema. The `getPlanner` method reflects this:

      ```java
      public static Planner getPlanner(
      Lex lex,
      Function1<SchemaPlus, Schema> schemaFactory,
      List<RelTraitDef> traitDefs,
      SqlStdOperatorTable operatorTable,
      RuleSet... ruleSets);
      ```

      Instead we should add

      ```java
      public static SchemaPlus createRootSchema()
      ```

      and let clients build their own schemas under that root schema that can be re-used. The signature of `getPlanner` would become

      ```java
      public static Planner getPlanner(
      Lex lex,
      SchemaPlus rootSchema,
      Schema defaultSchema,
      List<RelTraitDef> traitDefs,
      SqlStdOperatorTable operatorTable,
      RuleSet... ruleSets);
      ```

      `defaultSchema` can be null or a schema under `rootSchema`.

      ---------------- Imported from GitHub ----------------
      Url: https://github.com/julianhyde/optiq/issues/214
      Created by: julianhyde
      Labels:
      Created at: Sat Mar 29 22:50:22 CET 2014
      State: closed

        Activity

        Hide
        github-import GitHub Import added a comment -

        [Date: Sat Mar 29 22:53:37 CET 2014, Author: julianhyde]

        Jacques, assigning to you. Please comment whether this would meet your requirements.

        Show
        github-import GitHub Import added a comment - [Date: Sat Mar 29 22:53:37 CET 2014, Author: julianhyde ] Jacques, assigning to you. Please comment whether this would meet your requirements.
        Hide
        github-import GitHub Import added a comment -

        [Date: Sat Mar 29 22:59:37 CET 2014, Author: jacques-n]

        Looks great. Only question, why is defaultSchema a Schema rather than a SchemaPlus? Since we've built the schemas already, it seems like we should hand you a SchemaPlus so you don't try to put this something else.

        Show
        github-import GitHub Import added a comment - [Date: Sat Mar 29 22:59:37 CET 2014, Author: jacques-n ] Looks great. Only question, why is defaultSchema a Schema rather than a SchemaPlus? Since we've built the schemas already, it seems like we should hand you a SchemaPlus so you don't try to put this something else.
        Hide
        github-import GitHub Import added a comment -

        [Date: Sun Mar 30 10:17:24 CEST 2014, Author: julianhyde]

        You're right. `defaultSchema` should be a `SchemaPlus`.

        I just fixed https://github.com/julianhyde/optiq/issues/215, and now `Schema` has even less information than before, so `SchemaPlus` is definitely needed.

        Show
        github-import GitHub Import added a comment - [Date: Sun Mar 30 10:17:24 CEST 2014, Author: julianhyde ] You're right. `defaultSchema` should be a `SchemaPlus`. I just fixed https://github.com/julianhyde/optiq/issues/215 , and now `Schema` has even less information than before, so `SchemaPlus` is definitely needed.
        Hide
        github-import GitHub Import added a comment -

        [Date: Tue Apr 08 17:50:13 CEST 2014, Author: vkorukanti]

        Is this issue planned for Optiq 0.6 release?

        Show
        github-import GitHub Import added a comment - [Date: Tue Apr 08 17:50:13 CEST 2014, Author: vkorukanti ] Is this issue planned for Optiq 0.6 release?
        Hide
        github-import GitHub Import added a comment -

        [Date: Tue Apr 08 20:17:22 CEST 2014, Author: julianhyde]

        Stretch goal. I don't know whether I'll have time to do it. Contributions welcome.

        Show
        github-import GitHub Import added a comment - [Date: Tue Apr 08 20:17:22 CEST 2014, Author: julianhyde ] Stretch goal. I don't know whether I'll have time to do it. Contributions welcome.
        Hide
        github-import GitHub Import added a comment -

        [Date: Wed Apr 09 20:25:09 CEST 2014, Author: vkorukanti]

        Here is a change that adds createRootSchema which gets the OptiqRootSchema through PrepareAction: https://github.com/vkorukanti/optiq/commit/ae3164e969e638e8bc1a4499ad175d6d69d6ffe3. It still needs javadoc and probably a builder for Planner.

        Jacques and I were discussing about the way PlannerImpl gets RelOptPlanner and JavaTypeFactory. Currently it gets through PrepareAction, as part of OptiqPrepareImpl.perform(), we create the planner, add rules and traits to it and bunch of other stuff which is not used in PlannerImpl. In PlannerImpl, we reset rules in RelOptPlanner. So we were wondering whether it is a good idea to directly create the VolcanoPlanner and JavaTypeFactoryImpl directly in PlannerImpl which is a few lines of code, so not much copying of code from OptiqPrepareImpl. Here is the change: https://github.com/vkorukanti/optiq/commit/a417cf5dd7db0e616350d4d7071bb9a544cbe87b.

        Appreciate your feedback on both patches.

        Show
        github-import GitHub Import added a comment - [Date: Wed Apr 09 20:25:09 CEST 2014, Author: vkorukanti ] Here is a change that adds createRootSchema which gets the OptiqRootSchema through PrepareAction: https://github.com/vkorukanti/optiq/commit/ae3164e969e638e8bc1a4499ad175d6d69d6ffe3 . It still needs javadoc and probably a builder for Planner. Jacques and I were discussing about the way PlannerImpl gets RelOptPlanner and JavaTypeFactory. Currently it gets through PrepareAction, as part of OptiqPrepareImpl.perform(), we create the planner, add rules and traits to it and bunch of other stuff which is not used in PlannerImpl. In PlannerImpl, we reset rules in RelOptPlanner. So we were wondering whether it is a good idea to directly create the VolcanoPlanner and JavaTypeFactoryImpl directly in PlannerImpl which is a few lines of code, so not much copying of code from OptiqPrepareImpl. Here is the change: https://github.com/vkorukanti/optiq/commit/a417cf5dd7db0e616350d4d7071bb9a544cbe87b . Appreciate your feedback on both patches.
        Hide
        github-import GitHub Import added a comment -

        [Date: Thu Apr 10 20:14:02 CEST 2014, Author: julianhyde]

        I went for the approach that continues to use the 'withPlanner' callback. But this isn't visible to the user of Planner, which was the whole goal of Planner.

        Show
        github-import GitHub Import added a comment - [Date: Thu Apr 10 20:14:02 CEST 2014, Author: julianhyde ] I went for the approach that continues to use the 'withPlanner' callback. But this isn't visible to the user of Planner, which was the whole goal of Planner.

          People

          • Assignee:
            Unassigned
            Reporter:
            github-import GitHub Import
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development