Details

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

      Description

      DruidAdapterIT is currently broken for me. When I run it using

      cd druid
      mvn -Pit integration-test
      

      I get

      Running org.apache.calcite.test.DruidAdapterIT
      Tests run: 35, Failures: 26, Errors: 3, Skipped: 1, Time elapsed: -673,099.469 sec <<< FAILURE!
      
      Results :
      
      Failed tests: 
        testFilterTimestamp(org.apache.calcite.test.DruidAdapterIT): 
      Expected: a string containing "EnumerableInterpreter\n  BindableAggregate(group=[{}], C=[COUNT()])\n    BindableFilter(condition=[AND(>=(/INT(Reinterpret($91), 86400000), 1997-01-01), <(/INT(Reinterpret($91), 86400000), 1998-01-01), >=(/INT(Reinterpret($91), 86400000), 1997-04-01), <(/INT(Reinterpret($91), 86400000), 1997-05-01))])\n      DruidQuery(table=[[foodmart, foodmart]], intervals=[[1900-01-09T00:00:00.000Z/2992-01-10T00:00:00.000Z]])"
           but: was "PLAN=EnumerableInterpreter
        BindableAggregate(group=[{}], C=[COUNT()])
          BindableFilter(condition=[AND(>=(/INT(Reinterpret($91), 86400000), 1997-01-01), <(/INT(Reinterpret($91), 86400000), 1998-01-01), >=(/INT(Reinterpret($91), 86400000), 1997-04-01), <(/INT(Reinterpret($91), 86400000), 1997-05-01))])
            DruidQuery(table=[[foodmart, foodmart]], intervals=[[1900-01-08T16:00:00.000-08:00/2992-01-09T16:00:00.000-08:00]])
      
      "
      

      Note the differences in the "intervals" string. Other tests have very similar errors. I am in Pacific time zone, but it shouldn't matter. I get similar errors when I run DruidAdapterIT from Intellij (i.e. without maven-surefire, and without explicitly setting a timezone).

        Activity

        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - - edited

        I am in BST, and in my case, only one test fails "testTopNDayGranularityFiltered". For that test, I think it has to do with daylight saving time difference; one date in summer ends up with -1 hour... Probably the result changed with the timezone fix that went in with CALCITE-1357.

        I think it is also related to the conversation that we started in CALCITE-1357 about Calcite and Druid being in different timezones. I guess if you are having those issues, it is probably because at some point we are creating an interval based on your local timezone, and then String conversion yields those results. I explored a bit and found a couple of places where the problem may be caused... One of them is the default Interval in DruidTable. Another occurrence is when we retrieve the Intervals for a given data source from Druid, in DruidTableFactory.

        I started working in a fix in the following branch:
        https://github.com/jcamachor/calcite/tree/CALCITE-druid-timezone

        Does all this make sense? Could you take a look if it solves your issues? We can continue working in that branch to solve this issue.

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - - edited I am in BST, and in my case, only one test fails "testTopNDayGranularityFiltered". For that test, I think it has to do with daylight saving time difference; one date in summer ends up with -1 hour... Probably the result changed with the timezone fix that went in with CALCITE-1357 . I think it is also related to the conversation that we started in CALCITE-1357 about Calcite and Druid being in different timezones. I guess if you are having those issues, it is probably because at some point we are creating an interval based on your local timezone, and then String conversion yields those results. I explored a bit and found a couple of places where the problem may be caused... One of them is the default Interval in DruidTable. Another occurrence is when we retrieve the Intervals for a given data source from Druid, in DruidTableFactory. I started working in a fix in the following branch: https://github.com/jcamachor/calcite/tree/CALCITE-druid-timezone Does all this make sense? Could you take a look if it solves your issues? We can continue working in that branch to solve this issue.
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Btw, that does not deal with the daylight saving time difference issue... I am not sure how that should be tackled. Julian Hyde, what is your take on that?

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Btw, that does not deal with the daylight saving time difference issue... I am not sure how that should be tackled. Julian Hyde , what is your take on that?
        Hide
        julianhyde Julian Hyde added a comment -

        Jesus Camacho Rodriguez, I didn't see your fix until now. I came up a similar fix (see https://github.com/julianhyde/calcite/tree/1403-druid-broken).

        But testGroupByMonthGranularity still fails for me. It gives an HTTP 500

        "error" : "Instantiation of [simple type, class io.druid.granularity.QueryGranularity] value failed: No enum constant io.druid.granularity.QueryGranularity.MillisIn.MONTH"
        

        which looks very much the same as an error in https://groups.google.com/forum/#!topic/druid-development/fhHYfbbbGUo. I think Druid will simply not accept "granularity": "month", only granularities of day or smaller.

        Show
        julianhyde Julian Hyde added a comment - Jesus Camacho Rodriguez , I didn't see your fix until now. I came up a similar fix (see https://github.com/julianhyde/calcite/tree/1403-druid-broken ). But testGroupByMonthGranularity still fails for me. It gives an HTTP 500 "error" : "Instantiation of [simple type, class io.druid.granularity.QueryGranularity] value failed: No enum constant io.druid.granularity.QueryGranularity.MillisIn.MONTH" which looks very much the same as an error in https://groups.google.com/forum/#!topic/druid-development/fhHYfbbbGUo . I think Druid will simply not accept "granularity": "month" , only granularities of day or smaller.
        Hide
        julianhyde Julian Hyde added a comment -

        Nishant Bangarwa, I saw your name on the aforementioned email thread, so may I ask: has granularity: month ever worked? Was it added since 0.9.0? I can't understand how this test ever passed for Jesus Camacho Rodriguez, because Druid-0.9.0 chokes on the query that we are generating.

        Show
        julianhyde Julian Hyde added a comment - Nishant Bangarwa , I saw your name on the aforementioned email thread, so may I ask: has granularity: month ever worked? Was it added since 0.9.0? I can't understand how this test ever passed for Jesus Camacho Rodriguez , because Druid-0.9.0 chokes on the query that we are generating.
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        It got to work because for Hive testing, I updated the Druid version in the vm to 0.9.1.1. I have double checked: I reused the same vm for Calcite testing without realizing, and I forgot to push the change to the calcite-test-dataset repository.

        I guess we can just upgrade the vm to use 0.9.1.1.

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - It got to work because for Hive testing, I updated the Druid version in the vm to 0.9.1.1. I have double checked: I reused the same vm for Calcite testing without realizing, and I forgot to push the change to the calcite-test-dataset repository. I guess we can just upgrade the vm to use 0.9.1.1.
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Julian Hyde, patch LGTM; once it goes in, I think we can cut the RC.

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Julian Hyde , patch LGTM; once it goes in, I think we can cut the RC.
        Hide
        nishantbangarwa Nishant Bangarwa added a comment -

        yes, support for month was added in 0.9.1.1.
        Related PR - https://github.com/druid-io/druid/pull/2614

        Show
        nishantbangarwa Nishant Bangarwa added a comment - yes, support for month was added in 0.9.1.1. Related PR - https://github.com/druid-io/druid/pull/2614
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        I have created a PR to update to 0.9.1.1 in:

        https://github.com/vlsi/calcite-test-dataset/pull/13

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - I have created a PR to update to 0.9.1.1 in: https://github.com/vlsi/calcite-test-dataset/pull/13
        Hide
        julianhyde Julian Hyde added a comment -

        Jesus Camacho Rodriguez Can you make sure 0.10's release notes mention somewhere that we need at least Druid 0.9.1?

        Show
        julianhyde Julian Hyde added a comment - Jesus Camacho Rodriguez Can you make sure 0.10's release notes mention somewhere that we need at least Druid 0.9.1?
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Sure, no problem.

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Sure, no problem.
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/73544eda .
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Resolved in release 1.10.0 (2016-10-12).

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.10.0 (2016-10-12).

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development