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

Make RelDistribution extend RelMultipleTrait

    Details

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

      Description

      In Distributed System, RelDistribution is used for Exchange or SortExchange. for some operators it may deliver RelDistribution Trait, but some operator like SortedMergeJoin may deliver multiple traits.
      eg:

      Query:
      select * from T1 join T2 on T1.c1=T2.d1;
      
      Suppose Plan:
      SortedMergeJoin
          Exchange(c1)
              T1(c1)
          Exchange(d1)
              T2(d1)
      

      than SortedMergeJoin can deliver RelDistribution(hash[c1]) or RelDistribution(hash[d1]).
      we can consider RelDistribution extend RelMultipleTrait like RelCollation.
      EnumerableMergeJoin is the case for RelCollation, and RelDistribution is also fit for SortedMergeJoin in Distributed system

        Activity

        Hide
        perid007 LeoWangLZ added a comment -

        Thanks Julian Hyde, Thank for your test cases and guava's Orderings

        Show
        perid007 LeoWangLZ added a comment - Thanks Julian Hyde , Thank for your test cases and guava's Orderings
        Hide
        julianhyde Julian Hyde added a comment -

        Resolved in release 1.15.0 (2017-12-11).

        Show
        julianhyde Julian Hyde added a comment - Resolved in release 1.15.0 (2017-12-11).
        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/9c5ebe34 ; thanks for the PR, LeoWangLZ !
        Hide
        julianhyde Julian Hyde added a comment -

        Reviewing and testing https://github.com/apache/calcite/pull/541/commits/fed879b3f2cdea53d742b90786407328a3a49f02 now. I will commit when tests pass.

        I added RelDistributionTest to CalciteSuite, and changed your compareTo method to use guava's Ordering.lexicographical().

        Show
        julianhyde Julian Hyde added a comment - Reviewing and testing https://github.com/apache/calcite/pull/541/commits/fed879b3f2cdea53d742b90786407328a3a49f02 now. I will commit when tests pass. I added RelDistributionTest to CalciteSuite , and changed your compareTo method to use guava's Ordering.lexicographical() .
        Hide
        perid007 LeoWangLZ added a comment - - edited

        Thanks, Julian Hyde https://github.com/apache/calcite/pull/541
        The patch is for RelDistribution extends RelMultipleTrait. I think Existing user will not be affected. It only add some API(isTop and CompareTo) because of RelDistribution extends RelMultipleTrait, and it don't affect the trait satisfy currently. The Distribution can be composed by RelCompositeTrait, and enhances the satisfy.

        Show
        perid007 LeoWangLZ added a comment - - edited Thanks, Julian Hyde https://github.com/apache/calcite/pull/541 The patch is for RelDistribution extends RelMultipleTrait. I think Existing user will not be affected. It only add some API(isTop and CompareTo) because of RelDistribution extends RelMultipleTrait, and it don't affect the trait satisfy currently. The Distribution can be composed by RelCompositeTrait, and enhances the satisfy.
        Hide
        julianhyde Julian Hyde added a comment -

        The idea is OK in principle, but can you describe the impact of this change on existing users? Would it be breaking? What tests would you add to make sure this works fully?

        Show
        julianhyde Julian Hyde added a comment - The idea is OK in principle, but can you describe the impact of this change on existing users? Would it be breaking? What tests would you add to make sure this works fully?

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            perid007 LeoWangLZ
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 0.2h
              0.2h
              Remaining:
              Remaining Estimate - 0.2h
              0.2h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development