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

        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
        Tony Stevenson made changes -
        Workflow no-reopen-closed, patch-avail, testing [ 12860106 ] Drill workflow [ 12934871 ]
        Mehant Baid made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Mehant Baid added a comment -

        Fixed in or before 709e976d2c583a62cc314df061d21c2b537b76a1

        Show
        Mehant Baid added a comment - Fixed in or before 709e976d2c583a62cc314df061d21c2b537b76a1
        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
        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.
        Mehant Baid made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Mehant Baid added a comment -

        Aman Sinha please review.

        Show
        Mehant Baid added a comment - Aman Sinha please review.
        Mehant Baid made changes -
        Attachment DRILL-584.patch [ 12685019 ]
        Jacques Nadeau made changes -
        Fix Version/s 0.8.0 [ 12328812 ]
        Fix Version/s 0.7.0 [ 12327473 ]
        Aman Sinha made changes -
        Assignee Aman Sinha [ amansinha100 ] Mehant Baid [ mehant ]
        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.
        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.
        Jacques Nadeau made changes -
        Fix Version/s 0.7.0 [ 12327473 ]
        Fix Version/s 0.6.0 [ 12327472 ]
        Jacques Nadeau made changes -
        Fix Version/s 0.6.0 [ 12327472 ]
        Fix Version/s 0.5.0 [ 12324880 ]
        Sudheesh Katkam made changes -
        Due Date 23/Aug/14
        Jacques Nadeau made changes -
        Assignee Mehant Baid [ mehant ] Aman Sinha [ amansinha100 ]
        Jacques Nadeau made changes -
        Fix Version/s 0.5.0 [ 12324880 ]
        Fix Version/s 0.4.0 [ 12324963 ]
        Mehant Baid made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Mehant Baid made changes -
        Assignee DrillCommitter [ drillcommitter ] Mehant Baid [ mehant ]
        Mehant Baid made changes -
        Attachment DRILL-584.patch [ 12643837 ]
        Hide
        Jinfeng Ni added a comment -

        Completed the code review.

        Show
        Jinfeng Ni added a comment - Completed the code review.
        Jinfeng Ni made changes -
        Assignee Jinfeng Ni [ jni ] DrillCommitter [ drillcommitter ]
        Jacques Nadeau made changes -
        Fix Version/s 1.0.0-BETA1 [ 12324963 ]
        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 -
        Assignee Mehant Baid [ mehant ] Jinfeng Ni [ jni ]
        Component/s Execution - Codegen [ 12322686 ]
        Mehant Baid made changes -
        Attachment DRILL-584.patch [ 12643837 ]
        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 [ 12642391 ]
        Jake Farrell made changes -
        Workflow no-reopen-closed, patch-avail [ 12858506 ] no-reopen-closed, patch-avail, testing [ 12860106 ]
        Mehant Baid made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Mehant Baid made changes -
        Field Original Value New Value
        Attachment DRILL-584.patch [ 12642391 ]
        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 created issue -

          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