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

Type inference when converting IN clause to semijoin

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.4.0-incubating, 1.3.0-incubating, 1.2.0-incubating, 1.1.0-incubating
    • Fix Version/s: 1.5.0
    • Component/s: core
    • Labels:
      None

      Description

      I hit an issue where using an IN clause of int literals would work properly when compared against a BIGINT column when the IN clause had less than 20 values, but would fail when processing more than 20 values. I traced the issue to the way target row types are handled inside of the code that converts the IN clause into a semijoin.

        Activity

        Hide
        jwills Josh Wills added a comment -

        Here's the patch I wrote for this and the test I added to CsvTest that will fail w/o the fix from the patch. I kinda doubt that my implementation is the best way to fix this bug, so improvements are welcome.

        Show
        jwills Josh Wills added a comment - Here's the patch I wrote for this and the test I added to CsvTest that will fail w/o the fix from the patch. I kinda doubt that my implementation is the best way to fix this bug, so improvements are welcome.
        Hide
        julianhyde Julian Hyde added a comment -

        Looks great. Yes, it is the right fix. I hardened the test case a bit, and I have committed as my branch https://github.com/julianhyde/incubator-calcite/tree/824-semijoin-inference. I will commit to apache when release 1.4 is done. Thanks for the patch!

        By the way, it is a propos... I am re-working IN in CALCITE-816, so I'll make sure that your test case continues to work.

        Show
        julianhyde Julian Hyde added a comment - Looks great. Yes, it is the right fix. I hardened the test case a bit, and I have committed as my branch https://github.com/julianhyde/incubator-calcite/tree/824-semijoin-inference . I will commit to apache when release 1.4 is done. Thanks for the patch! By the way, it is a propos... I am re-working IN in CALCITE-816 , so I'll make sure that your test case continues to work.
        Hide
        jwills Josh Wills added a comment -

        Thanks so much! I saw the discussion on the mailing list about CALCITE-816, so I appreciate you keeping that case around in all of its forms.

        Show
        jwills Josh Wills added a comment - Thanks so much! I saw the discussion on the mailing list about CALCITE-816 , so I appreciate you keeping that case around in all of its forms.
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/5d8a90b8 . Thanks for the patch, Josh Wills !
        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:
            julianhyde Julian Hyde
            Reporter:
            jwills Josh Wills
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development