Apache Drill
  1. Apache Drill
  2. DRILL-584

ExpressionTreeMaterializer injects incorrect implicit casts

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: Execution - Codegen
    • Labels:
      None

      Description

      In ExpressionTreeMaterializer.visitFunctionCall() we try to get a best match between the DrillFuncHolder and the input call. We may match the input call with a DrillFuncHolder whose argument type is different from that of the input, but is allowed to be implicitly cast as per the precedence rules.

      However when we inject implicit casts to compensate for the difference in argument types, we again use the same matching method to resolve the implicit cast to DrillFuncHolder. In the case of implicit casts should not match with a DrillFuncHolder that has different argument types than the input. We should return only the DrillFuncHolder that exactly matches the argument types and if no such holder is present should return NULL.

        Activity

        Mehant Baid created issue -
        Show
        Mehant Baid added a comment - Review board link: https://reviews.apache.org/r/20825/ Github link: https://github.com/mehant/incubator-drill/tree/DRILL-584
        Mehant Baid made changes -
        Field Original Value New Value
        Attachment DRILL-584.patch [ 12642391 ]
        Mehant Baid made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Jake Farrell made changes -
        Workflow no-reopen-closed, patch-avail [ 12858506 ] no-reopen-closed, patch-avail, testing [ 12860106 ]
        Mehant Baid made changes -
        Attachment DRILL-584.patch [ 12642391 ]
        Hide
        Mehant Baid added a comment -

        rebased on latest master

        Show
        Mehant Baid added a comment - rebased on latest master
        Mehant Baid made changes -
        Attachment DRILL-584.patch [ 12643837 ]
        Jacques Nadeau made changes -
        Assignee Mehant Baid [ mehant ] Jinfeng Ni [ jni ]
        Component/s Execution - Codegen [ 12322686 ]
        Hide
        Jacques Nadeau added a comment -

        Jinfeng, please review and decide next steps

        Show
        Jacques Nadeau added a comment - Jinfeng, please review and decide next steps
        Jacques Nadeau made changes -
        Fix Version/s 1.0.0-BETA1 [ 12324963 ]
        Jinfeng Ni made changes -
        Assignee Jinfeng Ni [ jni ] DrillCommitter [ drillcommitter ]
        Hide
        Jinfeng Ni added a comment -

        Completed the code review.

        Show
        Jinfeng Ni added a comment - Completed the code review.
        Mehant Baid made changes -
        Attachment DRILL-584.patch [ 12643837 ]
        Mehant Baid made changes -
        Assignee DrillCommitter [ drillcommitter ] Mehant Baid [ mehant ]
        Mehant Baid made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Jacques Nadeau made changes -
        Fix Version/s 0.5.0 [ 12324880 ]
        Fix Version/s 0.4.0 [ 12324963 ]
        Jacques Nadeau made changes -
        Assignee Mehant Baid [ mehant ] Aman Sinha [ amansinha100 ]
        Sudheesh Katkam made changes -
        Due Date 23/Aug/14
        Jacques Nadeau made changes -
        Fix Version/s 0.6.0 [ 12327472 ]
        Fix Version/s 0.5.0 [ 12324880 ]
        Jacques Nadeau made changes -
        Fix Version/s 0.7.0 [ 12327473 ]
        Fix Version/s 0.6.0 [ 12327472 ]
        Hide
        Jacques Nadeau added a comment -

        Aman, please review and decide whether we should continue forward.

        Show
        Jacques Nadeau added a comment - Aman, please review and decide whether we should continue forward.
        Hide
        Aman Sinha added a comment -

        The patch itself looks ok to me; however I talked to Mehant and there's likely additional function implementations needed to convert varbinary to date such that existing TPCH queries work with this patch. As agreed, I am assigning this to him.

        Show
        Aman Sinha added a comment - The patch itself looks ok to me; however I talked to Mehant and there's likely additional function implementations needed to convert varbinary to date such that existing TPCH queries work with this patch. As agreed, I am assigning this to him.
        Aman Sinha made changes -
        Assignee Aman Sinha [ amansinha100 ] Mehant Baid [ mehant ]
        Jacques Nadeau made changes -
        Fix Version/s 0.8.0 [ 12328812 ]
        Fix Version/s 0.7.0 [ 12327473 ]
        Mehant Baid made changes -
        Attachment DRILL-584.patch [ 12685019 ]
        Hide
        Mehant Baid added a comment -

        Aman Sinha please review.

        Show
        Mehant Baid added a comment - Aman Sinha please review.
        Mehant Baid made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Aman Sinha added a comment -

        +1 . looks good. I did verify that CastVarBinaryToDate already exists, so your exact resolver will make use of that for the TPCH queries.

        Show
        Aman Sinha added a comment - +1 . looks good. I did verify that CastVarBinaryToDate already exists, so your exact resolver will make use of that for the TPCH queries.
        Hide
        Mehant Baid added a comment -

        CastVarbinaryToDate exists however casting to timestamp and time does not exist. This causes regressions in tpch queries. I have filed DRILL-1882, DRILL-1883, DRILL-1884 for minor issues that were exposed by this patch. Will merge them together.

        Show
        Mehant Baid added a comment - CastVarbinaryToDate exists however casting to timestamp and time does not exist. This causes regressions in tpch queries. I have filed DRILL-1882 , DRILL-1883 , DRILL-1884 for minor issues that were exposed by this patch. Will merge them together.
        Hide
        Mehant Baid added a comment -

        Fixed in or before 709e976d2c583a62cc314df061d21c2b537b76a1

        Show
        Mehant Baid added a comment - Fixed in or before 709e976d2c583a62cc314df061d21c2b537b76a1
        Mehant Baid made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Tony Stevenson made changes -
        Workflow no-reopen-closed, patch-avail, testing [ 12860106 ] Drill workflow [ 12934871 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        48d 17h 54m 1 Mehant Baid 16/Jun/14 21:53
        Open Open Patch Available Patch Available
        170d 5h 23m 2 Mehant Baid 04/Dec/14 02:02
        Patch Available Patch Available Resolved Resolved
        14d 2h 44m 1 Mehant Baid 18/Dec/14 04:46

          People

          • Assignee:
            Mehant Baid
            Reporter:
            Mehant Baid
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved:

              Development