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

Change return type of JoinFactory::create*Join()

    Details

    • Type: Task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.9.1-incubating
    • Component/s: None
    • Labels:
      None

      Description

      This provides tighter type checking than RelNode and is consistent with RelFieldTrimmer, only consumer of this factory which assumes these nodes to be of type JoinRelBase

      1. OPTIQ-396.patch
        4 kB
        Ashutosh Chauhan

        Issue Links

          Activity

          Show
          ashutoshc Ashutosh Chauhan added a comment - https://github.com/apache/incubator-optiq/pull/11
          Hide
          julianhyde Julian Hyde added a comment -

          This patch makes createJoin more specific (from RelNode to JoinRelBase) and createSemiJoin less specific (from SemiJoinRel to JoinRelBase). The problem with making factories too specific is that you can no longer use a novel implementation. So we should be careful.

          Also, I am considering making SemiJoin not inherit from JoinRelBase. In many ways (such as its output row type) it behaves more like a filter. As part of the same change I would create a new interface SemiJoinFactory.

          Net net... please justify why these changes are necessary, given that they might create compatibility issues in future.

          Show
          julianhyde Julian Hyde added a comment - This patch makes createJoin more specific (from RelNode to JoinRelBase) and createSemiJoin less specific (from SemiJoinRel to JoinRelBase). The problem with making factories too specific is that you can no longer use a novel implementation. So we should be careful. Also, I am considering making SemiJoin not inherit from JoinRelBase. In many ways (such as its output row type) it behaves more like a filter. As part of the same change I would create a new interface SemiJoinFactory. Net net... please justify why these changes are necessary, given that they might create compatibility issues in future.
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-optiq/commit/e34f7f0f .

            People

            • Assignee:
              ashutoshc Ashutosh Chauhan
              Reporter:
              ashutoshc Ashutosh Chauhan
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development