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

Wrong collation trait in SortJoinTransposeRule for right joins

    Details

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

      Description

      The traitSet of the new Sort for Join's right child should be replaced with rightCollation.

            newRightInput = sort.copy(sort.getTraitSet(), join.getRight(), rightCollation,
                sort.offset, sort.fetch);
      

        Issue Links

          Activity

          Hide
          maryannxue Maryann Xue added a comment -

          Jesus Camacho Rodriguez and Julian Hyde, would you mind reviewing this patch?

          Show
          maryannxue Maryann Xue added a comment - Jesus Camacho Rodriguez and Julian Hyde , would you mind reviewing this patch?
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          Maryann Xue, the change makes sense; I just checked the Sort operator constructor and we enforce (via assertion) that the traits contain the given collation, so I'm wondering why we didn't catch this?

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Maryann Xue , the change makes sense; I just checked the Sort operator constructor and we enforce (via assertion) that the traits contain the given collation, so I'm wondering why we didn't catch this?
          Hide
          maryannxue Maryann Xue added a comment -

          Jesus Camacho Rodriguez The assertion is to test that the traitSet either does not contain the RelCollation trait at all or if it does have a RelCollation trait it must be equal to the "collation" parameter passed to the constructor. Meanwhile the default traitSet in the default "tester" does not have collation trait, which means the assert would succeed with the first condition.
          So you can see from the new test case in this patch that I created a customized "tester" to include the collation trait into the "cluster" so that the assertion would go to its second condition.

          Show
          maryannxue Maryann Xue added a comment - Jesus Camacho Rodriguez The assertion is to test that the traitSet either does not contain the RelCollation trait at all or if it does have a RelCollation trait it must be equal to the "collation" parameter passed to the constructor. Meanwhile the default traitSet in the default "tester" does not have collation trait, which means the assert would succeed with the first condition. So you can see from the new test case in this patch that I created a customized "tester" to include the collation trait into the "cluster" so that the assertion would go to its second condition.
          Hide
          julianhyde Julian Hyde added a comment -

          +1

          Show
          julianhyde Julian Hyde added a comment - +1
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/b8599379 . Thanks for the patch, Maryann Xue !
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          Resolved in release 1.5.0 (2015-11-10)

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.5.0 (2015-11-10)

            People

            • Assignee:
              maryannxue Maryann Xue
              Reporter:
              maryannxue Maryann Xue
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development