You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Charles Givre <cg...@gmail.com> on 2020/05/05 12:53:49 UTC
Re: Druid Storage Plugin for Drill
Hi Ankush,
Thanks for all your work on the Druid plugin. I'm going to try to do a review over the weekend. I had a question/suggestion. Recently we committed a storage plugin for REST APIs that use the OkHTTP3 library as the basis for HTTP requests. In the quest to get it committed there was some discussion about the fact that Drill uses several different libraries for making HTTP REST requests.
I wanted to ask your thoughts about perhaps moving this class from the HTTP plugin to a utility class and then having both the Druid and HTTP plugin use this class for REST requests?
What do you think?
-- C
> On Apr 27, 2020, at 12:55 AM, Ankush Kapur <an...@gmail.com> wrote:
>
> Making good progress with the changes requested from the review(s).
>
> - Ankush
>
> On Sun, Apr 19, 2020 at 12:36 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
> I don't mean to be a pain in the you know where about this... I very much would like to see this as a part of Drill.
> Best,
> -- C
>
>
>
>> On Apr 18, 2020, at 4:54 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>
>> Apologies for being MIA, got hung-up at work really bad.
>>
>> Yes, I did start working on the changes requested in the review. Will act on the other things you mentioned, as well.
>>
>> Thank you for following-up though, helps if someone is watching over :)
>>
>> - Ankush
>>
>> On Fri, Apr 17, 2020 at 12:56 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>> Hi Ankush,
>> How's it going? I hope you are doing ok with all that is happening around us. I hate to be a pest but I was wondering if you've had the chance to work on the Druid storage plugin review? I'm happy to help with some of the changes, but I didn't want to step on your toes or re-do things you're currently working on.
>>
>> Would it be possible for you to rebase on the current master as well? There have been some improvements unrelated to this plugin which will be helpful as we move towards committing this.
>> Stay safe!
>>
>> -- C
>>
>>
>>
>>
>>
>>> On Mar 22, 2020, at 8:34 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>
>>> Yes, thank you. both for your time reviewing...
>>>
>>> I am working on changes based on it.
>>>
>>> - Ankush
>>>
>>> On Fri, Mar 20, 2020 at 11:25 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>> Hi Ankush.
>>> I hope all is well. Were you able to see Paul and my review comments for the Druid plugin?
>>> Thanks,
>>> -- C
>>>
>>>> On Mar 13, 2020, at 8:17 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>
>>>> HI Ankush,
>>>> I hope you're doing ok. I live in Maryland and all our schools are now closed for the next two weeks, so it will be interesting.... Anyway, I tagged you in a few comments, but it's weird that my comments didn't show up on the main page. Take a look here and you should see them: https://github.com/apache/drill/pull/1888/files <https://github.com/apache/drill/pull/1888/files>
>>>>
>>>> General comments:
>>>> 1. Please verify that all logger creations are in the correct format. Also, I'd strongly suggest avoiding info messages and use either debug, warn or error as the case may be.
>>>> 2. I had a general question about security and passing credentials to Druid. I don't really know how Druid handles authentication, but it didn't seem like there was any way to pass creds to Druid. In any event, this will need to be documented in the README file.
>>>> 3. Please go through and remove any commented out code.
>>>>
>>>> Writing a storage plugin is not easy, and I'm really impressed that your first contribution to Drill is something of this scale. This is really nice work and I'm really hoping we can get it committed for the next release.
>>>> Thanks,
>>>> -- C
>>>>
>>>>
>>>>> On Mar 13, 2020, at 5:56 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>
>>>>> Hi Charles,
>>>>>
>>>>> Things are good here, and hope the same for you and your family.
>>>>>
>>>>> Pardon me, but I do not see your review comments on the four files mentioned.
>>>>>
>>>>> - Ankush
>>>>>
>>>>> On Thu, Mar 12, 2020 at 8:36 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>> Hi Ankush,
>>>>> I hope all is well. I started the review on your plugin. I've reviewed about four files so far. When you have a chance, please take a look at the review comments and if you have any questions, please let me know.
>>>>> Best,
>>>>> -- C
>>>>>
>>>>>
>>>>>> On Feb 28, 2020, at 8:33 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>
>>>>>> Hi Ankush,
>>>>>> I hope all is well. I've been speaking with Paul about your plugin and I'm going to do the code review. I usually don't do big code reviews like this, so I was hoping someone else would pick it up, but that hasn't happened and I really want to see this get committed to the next version of Drill.
>>>>>>
>>>>>> Since this is a large PR, I'm going to do this in small reviews rather than one massive review so please expect waves of review. I think it's best that way it doesn't get overwhelming. My game plan will be to focus on a few files at a time rather than trying to look at the entire plugin and give a few comments here and there. I'll send future correspondence via the dev alias in the interest of transparency as well. I'm going to ask Paul to look at the pushdown code as he is a lot more familiar with that than I am, but we're not there yet.
>>>>>>
>>>>>> Thanks for all your work and patience on this and I am excited about getting it into Drill. I know it will be a benefit for the community.
>>>>>> Best,
>>>>>> -- C
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Jan 21, 2020, at 8:43 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>
>>>>>>> Hey Ankush,
>>>>>>> See responses inline.
>>>>>>>
>>>>>>>
>>>>>>>> On Jan 21, 2020, at 8:12 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>
>>>>>>>> Hi Charles,
>>>>>>>>
>>>>>>>> Thanks for taking time to make the changes.
>>>>>>>
>>>>>>> No problem. Also FYI, Drill uses 2 space indents not 4. I just pushed an update with all the spacing on 2 space. Is there anything that I can assist with?
>>>>>>>
>>>>>>>>
>>>>>>>> Could you please point me to the "commented-out" code you are referring too. That's actually an issue i was trying to track down last weekend while writing tests. I noticed that the "where" clause was not being pushed down. This was keeping me from writing the tests I have been working on.
>>>>>>>
>>>>>>> The commented out code was in the DruidStoragePlugin, but it does not appear to be commented out anymore. Have you rebased on the most current branch? When I attempted to build your branch, I got an error in the RecordReader class. I'll take another look today or tomorrow. Regarding pushdowns, have you seen the Calcite adapter for Druid (https://calcite.apache.org/docs/druid_adapter.html <https://calcite.apache.org/docs/druid_adapter.html>). I'm wondering if you can basically extend or import the pushdowns from the Calcite adapter so that you don't have to recreate all that.
>>>>>>>
>>>>>>> I've been working on a pluign for Cassandra and am attempting to do that as well. Take a look at the Drill JDBC Storage Plugin because I think that's sort of what it does. Potentially this could save you a lot of work.
>>>>>>>
>>>>>>>>
>>>>>>>> Also, can you provide a link to the file where it's failing to compile for you? I was not able to repro this.
>>>>>>>
>>>>>>> The specific error I am getting is on line 151 in the DruidRecordReader. The error is that writer is a VectorContainerWriter and the required type is a ComplexWriter.
>>>>>>>
>>>>>>>>
>>>>>>>> Fyi, I pushed in some changes I was working on recently, still more work to do on the testing side, though.
>>>>>>>
>>>>>>> Awesome!
>>>>>>>
>>>>>>>>
>>>>>>>> Sincerely,
>>>>>>>> Ankush
>>>>>>>>
>>>>>>>> On Mon, Jan 20, 2020 at 10:01 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>> Hi Ankush,
>>>>>>>> I have a question for you. I was looking at your plugin and it looks like you commented out the bit which causes the filter pushdown to happen. Is this not working properly?
>>>>>>>>
>>>>>>>> Also, I attempted to build it and I got a compilation error in the RecordReader, line 151. I corrected the license issues in the test suite, and pushed that to your branch so hopefully it will pass TravisCI.
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> -- C
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Jan 12, 2020, at 10:37 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>
>>>>>>>>> Great to hear! I'm excited about getting this committed into Drill!
>>>>>>>>> -- C
>>>>>>>>>
>>>>>>>>>> On Jan 12, 2020, at 9:06 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Charles,
>>>>>>>>>>
>>>>>>>>>> Thanks for reaching out. Hope you are having a good 2020 as well.
>>>>>>>>>>
>>>>>>>>>> Yes, I am actively working on putting together more tests for the druid storage plugin.
>>>>>>>>>>
>>>>>>>>>> I will have something ready very soon, and also make sure the style guide is followed.
>>>>>>>>>>
>>>>>>>>>> Will get back soon.
>>>>>>>>>>
>>>>>>>>>> Have a wonderful day.
>>>>>>>>>>
>>>>>>>>>> Sincerely,
>>>>>>>>>> Ankush
>>>>>>>>>>
>>>>>>>>>> On Fri, Jan 10, 2020 at 10:08 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>> Hi Ankush,
>>>>>>>>>> How's it going? I hope you had a good new year! I wanted to follow up with you to see if you've had a chance to work on the Druid storage plugin. I don't mean to be a pest, but I'm excited about this capability and would love to see it integrated into Drill.
>>>>>>>>>>
>>>>>>>>>> Regarding unit tests, I'm currently working on an Elasticsearch storage plugin for Drill (https://github.com/cgivre/drill/tree/storage-elastic2/contrib/storage-elastic/src/test/java/org/apache/drill/exec/store/elasticsearch <https://github.com/cgivre/drill/tree/storage-elastic2/contrib/storage-elastic/src/test/java/org/apache/drill/exec/store/elasticsearch>) that someone else wrote a few years ago. I got it to work with ES 5.6 and Drill 1.17. If you want to borrow the unit tests, please feel free.
>>>>>>>>>>
>>>>>>>>>> One other thing that I know the reviewers will say is that Drill has a formatter (https://drill.apache.org/docs/apache-drill-contribution-guidelines/ <https://drill.apache.org/docs/apache-drill-contribution-guidelines/>) for source code (spacing, etc.). Could you please run your code through that to make sure that the style is consistent with Drill?
>>>>>>>>>>
>>>>>>>>>> If you need any other assistance, please let me know. Thanks!
>>>>>>>>>> -- C
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On Dec 15, 2019, at 10:13 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>
>>>>>>>>>>> 1. Yes, I was referring the pushing down aggregations to native druid aggregation queries. Also, DRUID now supports a new flavour of "SELECT" query called, "SCAN" query, which is expected to be more memory efficient. But I will leave for another revision.
>>>>>>>>>>>
>>>>>>>>>>> 2. I will add more unit tests. Also will add integration tests to exercise the queries.
>>>>>>>>>>>
>>>>>>>>>>> - Ankush
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Dec 9, 2019 at 7:23 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>> Thanks for the contribution!! This looks like a great start. I have a few questions:
>>>>>>>>>>> 1. When you say it only supports SELECT queries, what exactly do you mean here? Just FYI, most storage plugins are query-only, so most only support SELECT queries. If you are referring to pushing down GROUP BY and the like to the plugin, then I think that is fine to worry about later ;-). If your plugin works with SELECT queries, Drill should be able to perform the aggregation after the query has executed.
>>>>>>>>>>>
>>>>>>>>>>> 2. I saw there is only one file of unit tests. I know that the first review comment will be that you need more unit tests. Please take a look at the other plugins and see what kind of unit tests they have. At a minimum, I would expect to see some query tests. I'm wondering though, what is the best way to test this?
>>>>>>>>>>>
>>>>>>>>>>> Best,
>>>>>>>>>>> -- C
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> On Dec 8, 2019, at 5:36 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>
>>>>>>>>>>>> Apologies for the late reply. Been some crazy times here.
>>>>>>>>>>>>
>>>>>>>>>>>> I had the plugin working again and it supports the select queries(https://druid.apache.org/docs/latest/querying/select-query.html <https://druid.apache.org/docs/latest/querying/select-query.html>).
>>>>>>>>>>>> https://github.com/apache/drill/pull/1888 <https://github.com/apache/drill/pull/1888>
>>>>>>>>>>>>
>>>>>>>>>>>> Let me know if this is good for an MVP.
>>>>>>>>>>>>
>>>>>>>>>>>> People at DRUID now support a lot more rich queries(Scan, Aggregations, SQL syntax) since I first wrote the plugin. So I am thinking future changes would be required, which I would love to work on, iteratively.
>>>>>>>>>>>>
>>>>>>>>>>>> Let me know if this works for you.
>>>>>>>>>>>>
>>>>>>>>>>>> Sincerely,
>>>>>>>>>>>> Ankush
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Dec 3, 2019 at 11:38 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>> How's it going? I thought I'd check in with you to see how the plugin is coming and if you've made any progress? There is currently a change freeze because we're getting ready to release Drill 1.17, but I'd really love to see this get committed for version 1.18. If you're stuck or need assistance, please let me know.
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>> -- C
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> On Nov 1, 2019, at 3:26 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I just started a draft PR for this work.
>>>>>>>>>>>>> https://github.com/apache/drill/pull/1888 <https://github.com/apache/drill/pull/1888>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Sincerely,
>>>>>>>>>>>>> Ankush
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Sat, Oct 12, 2019 at 3:30 PM Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I apologize for such a late reply. I have been caught up with product releases at work.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I will begin working on the PR. Hopefully, it should not be too long.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hope you have a wonderful weekend.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Sincerely,
>>>>>>>>>>>>> Ankush
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Sep 19, 2019 at 10:16 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>>> Thanks for getting back with me! I'm really glad you're open to contributing this to Drill! I'm also really impressed that you were able to do this at all. Writing storage plugins is extremely difficult, especially given the lack of documentation, and that you have to have a really good understanding of both Druid and Drill's internals. So my hat's off to you!
>>>>>>>>>>>>> Anyway, I had a look at our JIRA board and it turns out that there is an open JIRA for a Drill/Druid integration: https://issues.apache.org/jira/browse/DRILL-5956 <https://issues.apache.org/jira/browse/DRILL-5956>
>>>>>>>>>>>>>
>>>>>>>>>>>>> When you're ready to do so, please create a pull request on the Drill github repo with the title: Drill-5956: Add Storage Plugin for Druid and add me as a reviewer. Thank you very much!!
>>>>>>>>>>>>> Best,
>>>>>>>>>>>>> -- C
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Sep 17, 2019, at 5:23 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I am good, thanks for asking. Hope the same for you.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yes, I would be happy to submit a pr. However. It will take sometime to get it in shape. The druid connector is a couple of years old now.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> - Ankush
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, Sep 17, 2019, 2:27 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>>>> I hope all is well. I'm the PMC chair for Apache Drill and I I saw your Druid plugin on github. I wanted to ask if you would consider contributing that to Drill and submitting a pull request. I'm happy to help with the review process.
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> -- Charles
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: Druid Storage Plugin for Drill
Posted by Charles Givre <cg...@gmail.com>.
There actually is in the same package. Take a look here for the tests:
1: https://github.com/apache/drill/blob/master/contrib/storage-http/src/test/java/org/apache/drill/exec/store/http/TestHttpPlugin.java <https://github.com/apache/drill/blob/master/contrib/storage-http/src/test/java/org/apache/drill/exec/store/http/TestHttpPlugin.java>
and here for the utility class:
https://github.com/apache/drill/blob/master/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java <https://github.com/apache/drill/blob/master/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java>
> On May 5, 2020, at 9:07 AM, Ankush Kapur <an...@gmail.com> wrote:
>
> Hi Charles,
>
> That makes sense to me. It would be helpful to use a common HTTP module. I used the `DefaultHttpClient`, which i am not too stoked about.
>
> As long as there is an interface to mock for the new http module, it would work great.
>
> - Ankush
>
> On Tue, May 5, 2020 at 8:53 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
> Hi Ankush,
> Thanks for all your work on the Druid plugin. I'm going to try to do a review over the weekend. I had a question/suggestion. Recently we committed a storage plugin for REST APIs that use the OkHTTP3 library as the basis for HTTP requests. In the quest to get it committed there was some discussion about the fact that Drill uses several different libraries for making HTTP REST requests.
>
> I wanted to ask your thoughts about perhaps moving this class from the HTTP plugin to a utility class and then having both the Druid and HTTP plugin use this class for REST requests?
> What do you think?
> -- C
>
>
>
>> On Apr 27, 2020, at 12:55 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>
>> Making good progress with the changes requested from the review(s).
>>
>> - Ankush
>>
>> On Sun, Apr 19, 2020 at 12:36 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>> I don't mean to be a pain in the you know where about this... I very much would like to see this as a part of Drill.
>> Best,
>> -- C
>>
>>
>>
>>> On Apr 18, 2020, at 4:54 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>
>>> Apologies for being MIA, got hung-up at work really bad.
>>>
>>> Yes, I did start working on the changes requested in the review. Will act on the other things you mentioned, as well.
>>>
>>> Thank you for following-up though, helps if someone is watching over :)
>>>
>>> - Ankush
>>>
>>> On Fri, Apr 17, 2020 at 12:56 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>> Hi Ankush,
>>> How's it going? I hope you are doing ok with all that is happening around us. I hate to be a pest but I was wondering if you've had the chance to work on the Druid storage plugin review? I'm happy to help with some of the changes, but I didn't want to step on your toes or re-do things you're currently working on.
>>>
>>> Would it be possible for you to rebase on the current master as well? There have been some improvements unrelated to this plugin which will be helpful as we move towards committing this.
>>> Stay safe!
>>>
>>> -- C
>>>
>>>
>>>
>>>
>>>
>>>> On Mar 22, 2020, at 8:34 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>
>>>> Yes, thank you. both for your time reviewing...
>>>>
>>>> I am working on changes based on it.
>>>>
>>>> - Ankush
>>>>
>>>> On Fri, Mar 20, 2020 at 11:25 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>> Hi Ankush.
>>>> I hope all is well. Were you able to see Paul and my review comments for the Druid plugin?
>>>> Thanks,
>>>> -- C
>>>>
>>>>> On Mar 13, 2020, at 8:17 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>
>>>>> HI Ankush,
>>>>> I hope you're doing ok. I live in Maryland and all our schools are now closed for the next two weeks, so it will be interesting.... Anyway, I tagged you in a few comments, but it's weird that my comments didn't show up on the main page. Take a look here and you should see them: https://github.com/apache/drill/pull/1888/files <https://github.com/apache/drill/pull/1888/files>
>>>>>
>>>>> General comments:
>>>>> 1. Please verify that all logger creations are in the correct format. Also, I'd strongly suggest avoiding info messages and use either debug, warn or error as the case may be.
>>>>> 2. I had a general question about security and passing credentials to Druid. I don't really know how Druid handles authentication, but it didn't seem like there was any way to pass creds to Druid. In any event, this will need to be documented in the README file.
>>>>> 3. Please go through and remove any commented out code.
>>>>>
>>>>> Writing a storage plugin is not easy, and I'm really impressed that your first contribution to Drill is something of this scale. This is really nice work and I'm really hoping we can get it committed for the next release.
>>>>> Thanks,
>>>>> -- C
>>>>>
>>>>>
>>>>>> On Mar 13, 2020, at 5:56 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>
>>>>>> Hi Charles,
>>>>>>
>>>>>> Things are good here, and hope the same for you and your family.
>>>>>>
>>>>>> Pardon me, but I do not see your review comments on the four files mentioned.
>>>>>>
>>>>>> - Ankush
>>>>>>
>>>>>> On Thu, Mar 12, 2020 at 8:36 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>> Hi Ankush,
>>>>>> I hope all is well. I started the review on your plugin. I've reviewed about four files so far. When you have a chance, please take a look at the review comments and if you have any questions, please let me know.
>>>>>> Best,
>>>>>> -- C
>>>>>>
>>>>>>
>>>>>>> On Feb 28, 2020, at 8:33 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>
>>>>>>> Hi Ankush,
>>>>>>> I hope all is well. I've been speaking with Paul about your plugin and I'm going to do the code review. I usually don't do big code reviews like this, so I was hoping someone else would pick it up, but that hasn't happened and I really want to see this get committed to the next version of Drill.
>>>>>>>
>>>>>>> Since this is a large PR, I'm going to do this in small reviews rather than one massive review so please expect waves of review. I think it's best that way it doesn't get overwhelming. My game plan will be to focus on a few files at a time rather than trying to look at the entire plugin and give a few comments here and there. I'll send future correspondence via the dev alias in the interest of transparency as well. I'm going to ask Paul to look at the pushdown code as he is a lot more familiar with that than I am, but we're not there yet.
>>>>>>>
>>>>>>> Thanks for all your work and patience on this and I am excited about getting it into Drill. I know it will be a benefit for the community.
>>>>>>> Best,
>>>>>>> -- C
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On Jan 21, 2020, at 8:43 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>
>>>>>>>> Hey Ankush,
>>>>>>>> See responses inline.
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Jan 21, 2020, at 8:12 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>
>>>>>>>>> Hi Charles,
>>>>>>>>>
>>>>>>>>> Thanks for taking time to make the changes.
>>>>>>>>
>>>>>>>> No problem. Also FYI, Drill uses 2 space indents not 4. I just pushed an update with all the spacing on 2 space. Is there anything that I can assist with?
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Could you please point me to the "commented-out" code you are referring too. That's actually an issue i was trying to track down last weekend while writing tests. I noticed that the "where" clause was not being pushed down. This was keeping me from writing the tests I have been working on.
>>>>>>>>
>>>>>>>> The commented out code was in the DruidStoragePlugin, but it does not appear to be commented out anymore. Have you rebased on the most current branch? When I attempted to build your branch, I got an error in the RecordReader class. I'll take another look today or tomorrow. Regarding pushdowns, have you seen the Calcite adapter for Druid (https://calcite.apache.org/docs/druid_adapter.html <https://calcite.apache.org/docs/druid_adapter.html>). I'm wondering if you can basically extend or import the pushdowns from the Calcite adapter so that you don't have to recreate all that.
>>>>>>>>
>>>>>>>> I've been working on a pluign for Cassandra and am attempting to do that as well. Take a look at the Drill JDBC Storage Plugin because I think that's sort of what it does. Potentially this could save you a lot of work.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Also, can you provide a link to the file where it's failing to compile for you? I was not able to repro this.
>>>>>>>>
>>>>>>>> The specific error I am getting is on line 151 in the DruidRecordReader. The error is that writer is a VectorContainerWriter and the required type is a ComplexWriter.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Fyi, I pushed in some changes I was working on recently, still more work to do on the testing side, though.
>>>>>>>>
>>>>>>>> Awesome!
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sincerely,
>>>>>>>>> Ankush
>>>>>>>>>
>>>>>>>>> On Mon, Jan 20, 2020 at 10:01 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>> Hi Ankush,
>>>>>>>>> I have a question for you. I was looking at your plugin and it looks like you commented out the bit which causes the filter pushdown to happen. Is this not working properly?
>>>>>>>>>
>>>>>>>>> Also, I attempted to build it and I got a compilation error in the RecordReader, line 151. I corrected the license issues in the test suite, and pushed that to your branch so hopefully it will pass TravisCI.
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>> -- C
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On Jan 12, 2020, at 10:37 AM, Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>
>>>>>>>>>> Great to hear! I'm excited about getting this committed into Drill!
>>>>>>>>>> -- C
>>>>>>>>>>
>>>>>>>>>>> On Jan 12, 2020, at 9:06 AM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for reaching out. Hope you are having a good 2020 as well.
>>>>>>>>>>>
>>>>>>>>>>> Yes, I am actively working on putting together more tests for the druid storage plugin.
>>>>>>>>>>>
>>>>>>>>>>> I will have something ready very soon, and also make sure the style guide is followed.
>>>>>>>>>>>
>>>>>>>>>>> Will get back soon.
>>>>>>>>>>>
>>>>>>>>>>> Have a wonderful day.
>>>>>>>>>>>
>>>>>>>>>>> Sincerely,
>>>>>>>>>>> Ankush
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Jan 10, 2020 at 10:08 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>> How's it going? I hope you had a good new year! I wanted to follow up with you to see if you've had a chance to work on the Druid storage plugin. I don't mean to be a pest, but I'm excited about this capability and would love to see it integrated into Drill.
>>>>>>>>>>>
>>>>>>>>>>> Regarding unit tests, I'm currently working on an Elasticsearch storage plugin for Drill (https://github.com/cgivre/drill/tree/storage-elastic2/contrib/storage-elastic/src/test/java/org/apache/drill/exec/store/elasticsearch <https://github.com/cgivre/drill/tree/storage-elastic2/contrib/storage-elastic/src/test/java/org/apache/drill/exec/store/elasticsearch>) that someone else wrote a few years ago. I got it to work with ES 5.6 and Drill 1.17. If you want to borrow the unit tests, please feel free.
>>>>>>>>>>>
>>>>>>>>>>> One other thing that I know the reviewers will say is that Drill has a formatter (https://drill.apache.org/docs/apache-drill-contribution-guidelines/ <https://drill.apache.org/docs/apache-drill-contribution-guidelines/>) for source code (spacing, etc.). Could you please run your code through that to make sure that the style is consistent with Drill?
>>>>>>>>>>>
>>>>>>>>>>> If you need any other assistance, please let me know. Thanks!
>>>>>>>>>>> -- C
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> On Dec 15, 2019, at 10:13 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>
>>>>>>>>>>>> 1. Yes, I was referring the pushing down aggregations to native druid aggregation queries. Also, DRUID now supports a new flavour of "SELECT" query called, "SCAN" query, which is expected to be more memory efficient. But I will leave for another revision.
>>>>>>>>>>>>
>>>>>>>>>>>> 2. I will add more unit tests. Also will add integration tests to exercise the queries.
>>>>>>>>>>>>
>>>>>>>>>>>> - Ankush
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, Dec 9, 2019 at 7:23 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>> Thanks for the contribution!! This looks like a great start. I have a few questions:
>>>>>>>>>>>> 1. When you say it only supports SELECT queries, what exactly do you mean here? Just FYI, most storage plugins are query-only, so most only support SELECT queries. If you are referring to pushing down GROUP BY and the like to the plugin, then I think that is fine to worry about later ;-). If your plugin works with SELECT queries, Drill should be able to perform the aggregation after the query has executed.
>>>>>>>>>>>>
>>>>>>>>>>>> 2. I saw there is only one file of unit tests. I know that the first review comment will be that you need more unit tests. Please take a look at the other plugins and see what kind of unit tests they have. At a minimum, I would expect to see some query tests. I'm wondering though, what is the best way to test this?
>>>>>>>>>>>>
>>>>>>>>>>>> Best,
>>>>>>>>>>>> -- C
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> On Dec 8, 2019, at 5:36 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Apologies for the late reply. Been some crazy times here.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I had the plugin working again and it supports the select queries(https://druid.apache.org/docs/latest/querying/select-query.html <https://druid.apache.org/docs/latest/querying/select-query.html>).
>>>>>>>>>>>>> https://github.com/apache/drill/pull/1888 <https://github.com/apache/drill/pull/1888>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Let me know if this is good for an MVP.
>>>>>>>>>>>>>
>>>>>>>>>>>>> People at DRUID now support a lot more rich queries(Scan, Aggregations, SQL syntax) since I first wrote the plugin. So I am thinking future changes would be required, which I would love to work on, iteratively.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Let me know if this works for you.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Sincerely,
>>>>>>>>>>>>> Ankush
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Dec 3, 2019 at 11:38 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>>> How's it going? I thought I'd check in with you to see how the plugin is coming and if you've made any progress? There is currently a change freeze because we're getting ready to release Drill 1.17, but I'd really love to see this get committed for version 1.18. If you're stuck or need assistance, please let me know.
>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>> -- C
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Nov 1, 2019, at 3:26 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I just started a draft PR for this work.
>>>>>>>>>>>>>> https://github.com/apache/drill/pull/1888 <https://github.com/apache/drill/pull/1888>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Sincerely,
>>>>>>>>>>>>>> Ankush
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Sat, Oct 12, 2019 at 3:30 PM Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I apologize for such a late reply. I have been caught up with product releases at work.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I will begin working on the PR. Hopefully, it should not be too long.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hope you have a wonderful weekend.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Sincerely,
>>>>>>>>>>>>>> Ankush
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Thu, Sep 19, 2019 at 10:16 AM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>>>> Thanks for getting back with me! I'm really glad you're open to contributing this to Drill! I'm also really impressed that you were able to do this at all. Writing storage plugins is extremely difficult, especially given the lack of documentation, and that you have to have a really good understanding of both Druid and Drill's internals. So my hat's off to you!
>>>>>>>>>>>>>> Anyway, I had a look at our JIRA board and it turns out that there is an open JIRA for a Drill/Druid integration: https://issues.apache.org/jira/browse/DRILL-5956 <https://issues.apache.org/jira/browse/DRILL-5956>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> When you're ready to do so, please create a pull request on the Drill github repo with the title: Drill-5956: Add Storage Plugin for Druid and add me as a reviewer. Thank you very much!!
>>>>>>>>>>>>>> Best,
>>>>>>>>>>>>>> -- C
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Sep 17, 2019, at 5:23 PM, Ankush Kapur <ankush.kapur@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I am good, thanks for asking. Hope the same for you.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yes, I would be happy to submit a pr. However. It will take sometime to get it in shape. The druid connector is a couple of years old now.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> - Ankush
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Tue, Sep 17, 2019, 2:27 PM Charles Givre <cgivre@gmail.com <ma...@gmail.com>> wrote:
>>>>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>>>>> I hope all is well. I'm the PMC chair for Apache Drill and I I saw your Druid plugin on github. I wanted to ask if you would consider contributing that to Drill and submitting a pull request. I'm happy to help with the review process.
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> -- Charles
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: Druid Storage Plugin for Drill
Posted by Ankush Kapur <an...@gmail.com>.
Hi Charles,
That makes sense to me. It would be helpful to use a common HTTP module. I
used the `DefaultHttpClient`, which i am not too stoked about.
As long as there is an interface to mock for the new http module, it would
work great.
- Ankush
On Tue, May 5, 2020 at 8:53 AM Charles Givre <cg...@gmail.com> wrote:
> Hi Ankush,
> Thanks for all your work on the Druid plugin. I'm going to try to do a
> review over the weekend. I had a question/suggestion. Recently we
> committed a storage plugin for REST APIs that use the OkHTTP3 library as
> the basis for HTTP requests. In the quest to get it committed there was
> some discussion about the fact that Drill uses several different libraries
> for making HTTP REST requests.
>
> I wanted to ask your thoughts about perhaps moving this class from the
> HTTP plugin to a utility class and then having both the Druid and HTTP
> plugin use this class for REST requests?
> What do you think?
> -- C
>
>
>
> On Apr 27, 2020, at 12:55 AM, Ankush Kapur <an...@gmail.com> wrote:
>
> Making good progress with the changes requested from the review(s).
>
> - Ankush
>
> On Sun, Apr 19, 2020 at 12:36 AM Charles Givre <cg...@gmail.com> wrote:
>
>> I don't mean to be a pain in the you know where about this... I very
>> much would like to see this as a part of Drill.
>> Best,
>> -- C
>>
>>
>>
>> On Apr 18, 2020, at 4:54 PM, Ankush Kapur <an...@gmail.com> wrote:
>>
>> Apologies for being MIA, got hung-up at work really bad.
>>
>> Yes, I did start working on the changes requested in the review. Will act
>> on the other things you mentioned, as well.
>>
>> Thank you for following-up though, helps if someone is watching over :)
>>
>> - Ankush
>>
>> On Fri, Apr 17, 2020 at 12:56 AM Charles Givre <cg...@gmail.com> wrote:
>>
>>> Hi Ankush,
>>> How's it going? I hope you are doing ok with all that is happening
>>> around us. I hate to be a pest but I was wondering if you've had the
>>> chance to work on the Druid storage plugin review? I'm happy to help with
>>> some of the changes, but I didn't want to step on your toes or re-do things
>>> you're currently working on.
>>>
>>> Would it be possible for you to rebase on the current master as well?
>>> There have been some improvements unrelated to this plugin which will be
>>> helpful as we move towards committing this.
>>> Stay safe!
>>>
>>> -- C
>>>
>>>
>>>
>>>
>>>
>>> On Mar 22, 2020, at 8:34 AM, Ankush Kapur <an...@gmail.com>
>>> wrote:
>>>
>>> Yes, thank you. both for your time reviewing...
>>>
>>> I am working on changes based on it.
>>>
>>> - Ankush
>>>
>>> On Fri, Mar 20, 2020 at 11:25 AM Charles Givre <cg...@gmail.com> wrote:
>>>
>>>> Hi Ankush.
>>>> I hope all is well. Were you able to see Paul and my review comments
>>>> for the Druid plugin?
>>>> Thanks,
>>>> -- C
>>>>
>>>> On Mar 13, 2020, at 8:17 AM, Charles Givre <cg...@gmail.com> wrote:
>>>>
>>>> HI Ankush,
>>>> I hope you're doing ok. I live in Maryland and all our schools are now
>>>> closed for the next two weeks, so it will be interesting.... Anyway, I
>>>> tagged you in a few comments, but it's weird that my comments didn't show
>>>> up on the main page. Take a look here and you should see them:
>>>> https://github.com/apache/drill/pull/1888/files
>>>>
>>>> General comments:
>>>> 1. Please verify that all logger creations are in the correct format.
>>>> Also, I'd strongly suggest avoiding info messages and use either debug,
>>>> warn or error as the case may be.
>>>> 2. I had a general question about security and passing credentials to
>>>> Druid. I don't really know how Druid handles authentication, but it didn't
>>>> seem like there was any way to pass creds to Druid. In any event, this
>>>> will need to be documented in the README file.
>>>> 3. Please go through and remove any commented out code.
>>>>
>>>> Writing a storage plugin is not easy, and I'm really impressed that
>>>> your first contribution to Drill is something of this scale. This is
>>>> really nice work and I'm really hoping we can get it committed for the next
>>>> release.
>>>> Thanks,
>>>> -- C
>>>>
>>>>
>>>> On Mar 13, 2020, at 5:56 AM, Ankush Kapur <an...@gmail.com>
>>>> wrote:
>>>>
>>>> Hi Charles,
>>>>
>>>> Things are good here, and hope the same for you and your family.
>>>>
>>>> Pardon me, but I do not see your review comments on the four files
>>>> mentioned.
>>>>
>>>> - Ankush
>>>>
>>>> On Thu, Mar 12, 2020 at 8:36 PM Charles Givre <cg...@gmail.com> wrote:
>>>>
>>>>> Hi Ankush,
>>>>> I hope all is well. I started the review on your plugin. I've
>>>>> reviewed about four files so far. When you have a chance, please take a
>>>>> look at the review comments and if you have any questions, please let me
>>>>> know.
>>>>> Best,
>>>>> -- C
>>>>>
>>>>>
>>>>> On Feb 28, 2020, at 8:33 AM, Charles Givre <cg...@gmail.com> wrote:
>>>>>
>>>>> Hi Ankush,
>>>>> I hope all is well. I've been speaking with Paul about your plugin and
>>>>> I'm going to do the code review. I usually don't do big code reviews like
>>>>> this, so I was hoping someone else would pick it up, but that hasn't
>>>>> happened and I really want to see this get committed to the next version of
>>>>> Drill.
>>>>>
>>>>> Since this is a large PR, I'm going to do this in small reviews rather
>>>>> than one massive review so please expect waves of review. I think it's
>>>>> best that way it doesn't get overwhelming. My game plan will be to focus
>>>>> on a few files at a time rather than trying to look at the entire plugin
>>>>> and give a few comments here and there. I'll send future correspondence
>>>>> via the dev alias in the interest of transparency as well. I'm going to
>>>>> ask Paul to look at the pushdown code as he is a lot more familiar with
>>>>> that than I am, but we're not there yet.
>>>>>
>>>>> Thanks for all your work and patience on this and I am excited about
>>>>> getting it into Drill. I know it will be a benefit for the community.
>>>>> Best,
>>>>> -- C
>>>>>
>>>>>
>>>>>
>>>>> On Jan 21, 2020, at 8:43 AM, Charles Givre <cg...@gmail.com> wrote:
>>>>>
>>>>> Hey Ankush,
>>>>> See responses inline.
>>>>>
>>>>>
>>>>> On Jan 21, 2020, at 8:12 AM, Ankush Kapur <an...@gmail.com>
>>>>> wrote:
>>>>>
>>>>> Hi Charles,
>>>>>
>>>>> Thanks for taking time to make the changes.
>>>>>
>>>>>
>>>>> No problem. Also FYI, Drill uses 2 space indents not 4. I just
>>>>> pushed an update with all the spacing on 2 space. Is there anything that I
>>>>> can assist with?
>>>>>
>>>>>
>>>>> Could you please point me to the "commented-out" code you are
>>>>> referring too. That's actually an issue i was trying to track down last
>>>>> weekend while writing tests. I noticed that the "where" clause was not
>>>>> being pushed down. This was keeping me from writing the tests I have been
>>>>> working on.
>>>>>
>>>>>
>>>>> The commented out code was in the DruidStoragePlugin, but it does not
>>>>> appear to be commented out anymore. Have you rebased on the most current
>>>>> branch? When I attempted to build your branch, I got an error in the
>>>>> RecordReader class. I'll take another look today or tomorrow. Regarding
>>>>> pushdowns, have you seen the Calcite adapter for Druid (
>>>>> https://calcite.apache.org/docs/druid_adapter.html). I'm wondering
>>>>> if you can basically extend or import the pushdowns from the Calcite
>>>>> adapter so that you don't have to recreate all that.
>>>>>
>>>>> I've been working on a pluign for Cassandra and am attempting to do
>>>>> that as well. Take a look at the Drill JDBC Storage Plugin because I think
>>>>> that's sort of what it does. Potentially this could save you a lot of
>>>>> work.
>>>>>
>>>>>
>>>>> Also, can you provide a link to the file where it's failing to compile
>>>>> for you? I was not able to repro this.
>>>>>
>>>>>
>>>>> The specific error I am getting is on line 151 in the
>>>>> DruidRecordReader. The error is that writer is a VectorContainerWriter and
>>>>> the required type is a ComplexWriter.
>>>>>
>>>>>
>>>>> Fyi, I pushed in some changes I was working on recently, still more
>>>>> work to do on the testing side, though.
>>>>>
>>>>>
>>>>> Awesome!
>>>>>
>>>>>
>>>>> Sincerely,
>>>>> Ankush
>>>>>
>>>>> On Mon, Jan 20, 2020 at 10:01 PM Charles Givre <cg...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Hi Ankush,
>>>>>> I have a question for you. I was looking at your plugin and it looks
>>>>>> like you commented out the bit which causes the filter pushdown to happen.
>>>>>> Is this not working properly?
>>>>>>
>>>>>> Also, I attempted to build it and I got a compilation error in the
>>>>>> RecordReader, line 151. I corrected the license issues in the test suite,
>>>>>> and pushed that to your branch so hopefully it will pass TravisCI.
>>>>>>
>>>>>> Best,
>>>>>> -- C
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Jan 12, 2020, at 10:37 AM, Charles Givre <cg...@gmail.com> wrote:
>>>>>>
>>>>>> Great to hear! I'm excited about getting this committed into Drill!
>>>>>> -- C
>>>>>>
>>>>>> On Jan 12, 2020, at 9:06 AM, Ankush Kapur <an...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>> Hi Charles,
>>>>>>
>>>>>> Thanks for reaching out. Hope you are having a good 2020 as well.
>>>>>>
>>>>>> Yes, I am actively working on putting together more tests for the
>>>>>> druid storage plugin.
>>>>>>
>>>>>> I will have something ready very soon, and also make sure the style
>>>>>> guide is followed.
>>>>>>
>>>>>> Will get back soon.
>>>>>>
>>>>>> Have a wonderful day.
>>>>>>
>>>>>> Sincerely,
>>>>>> Ankush
>>>>>>
>>>>>> On Fri, Jan 10, 2020 at 10:08 AM Charles Givre <cg...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Ankush,
>>>>>>> How's it going? I hope you had a good new year! I wanted to follow
>>>>>>> up with you to see if you've had a chance to work on the Druid storage
>>>>>>> plugin. I don't mean to be a pest, but I'm excited about this capability
>>>>>>> and would love to see it integrated into Drill.
>>>>>>>
>>>>>>> Regarding unit tests, I'm currently working on an Elasticsearch
>>>>>>> storage plugin for Drill (
>>>>>>> https://github.com/cgivre/drill/tree/storage-elastic2/contrib/storage-elastic/src/test/java/org/apache/drill/exec/store/elasticsearch)
>>>>>>> that someone else wrote a few years ago. I got it to work with ES 5.6 and
>>>>>>> Drill 1.17. If you want to borrow the unit tests, please feel free.
>>>>>>>
>>>>>>> One other thing that I know the reviewers will say is that Drill has
>>>>>>> a formatter (
>>>>>>> https://drill.apache.org/docs/apache-drill-contribution-guidelines/)
>>>>>>> for source code (spacing, etc.). Could you please run your code through
>>>>>>> that to make sure that the style is consistent with Drill?
>>>>>>>
>>>>>>> If you need any other assistance, please let me know. Thanks!
>>>>>>> -- C
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Dec 15, 2019, at 10:13 PM, Ankush Kapur <an...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Charles,
>>>>>>>
>>>>>>> 1. Yes, I was referring the pushing down aggregations to native
>>>>>>> druid aggregation queries. Also, DRUID now supports a new flavour of
>>>>>>> "SELECT" query called, "SCAN" query, which is expected to be more memory
>>>>>>> efficient. But I will leave for another revision.
>>>>>>>
>>>>>>> 2. I will add more unit tests. Also will add integration tests to
>>>>>>> exercise the queries.
>>>>>>>
>>>>>>> - Ankush
>>>>>>>
>>>>>>> On Mon, Dec 9, 2019 at 7:23 PM Charles Givre <cg...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Ankush,
>>>>>>>> Thanks for the contribution!! This looks like a great start. I
>>>>>>>> have a few questions:
>>>>>>>> 1. When you say it only supports SELECT queries, what exactly do
>>>>>>>> you mean here? Just FYI, most storage plugins are query-only, so most only
>>>>>>>> support SELECT queries. If you are referring to pushing down GROUP BY and
>>>>>>>> the like to the plugin, then I think that is fine to worry about later ;-).
>>>>>>>> If your plugin works with SELECT queries, Drill should be able to perform
>>>>>>>> the aggregation after the query has executed.
>>>>>>>>
>>>>>>>> 2. I saw there is only one file of unit tests. I know that the
>>>>>>>> first review comment will be that you need more unit tests. Please take a
>>>>>>>> look at the other plugins and see what kind of unit tests they have. At a
>>>>>>>> minimum, I would expect to see some query tests. I'm wondering though,
>>>>>>>> what is the best way to test this?
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> -- C
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Dec 8, 2019, at 5:36 PM, Ankush Kapur <an...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi Charles,
>>>>>>>>
>>>>>>>> Apologies for the late reply. Been some crazy times here.
>>>>>>>>
>>>>>>>> I had the plugin working again and it supports the select queries(
>>>>>>>> https://druid.apache.org/docs/latest/querying/select-query.html).
>>>>>>>> https://github.com/apache/drill/pull/1888
>>>>>>>>
>>>>>>>> Let me know if this is good for an MVP.
>>>>>>>>
>>>>>>>> People at DRUID now support a lot more rich queries(Scan,
>>>>>>>> Aggregations, SQL syntax) since I first wrote the plugin. So I am thinking
>>>>>>>> future changes would be required, which I would love to work on,
>>>>>>>> iteratively.
>>>>>>>>
>>>>>>>> Let me know if this works for you.
>>>>>>>>
>>>>>>>> Sincerely,
>>>>>>>> Ankush
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Dec 3, 2019 at 11:38 AM Charles Givre <cg...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi Ankush,
>>>>>>>>> How's it going? I thought I'd check in with you to see how the
>>>>>>>>> plugin is coming and if you've made any progress? There is currently a
>>>>>>>>> change freeze because we're getting ready to release Drill 1.17, but I'd
>>>>>>>>> really love to see this get committed for version 1.18. If you're stuck or
>>>>>>>>> need assistance, please let me know.
>>>>>>>>> Thanks!
>>>>>>>>> -- C
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Nov 1, 2019, at 3:26 PM, Ankush Kapur <an...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi Charles,
>>>>>>>>>
>>>>>>>>> I just started a draft PR for this work.
>>>>>>>>> https://github.com/apache/drill/pull/1888
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sincerely,
>>>>>>>>> Ankush
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Sat, Oct 12, 2019 at 3:30 PM Ankush Kapur <
>>>>>>>>> ankush.kapur@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Charles,
>>>>>>>>>>
>>>>>>>>>> I apologize for such a late reply. I have been caught up with
>>>>>>>>>> product releases at work.
>>>>>>>>>>
>>>>>>>>>> I will begin working on the PR. Hopefully, it should not be too
>>>>>>>>>> long.
>>>>>>>>>>
>>>>>>>>>> Hope you have a wonderful weekend.
>>>>>>>>>>
>>>>>>>>>> Sincerely,
>>>>>>>>>> Ankush
>>>>>>>>>>
>>>>>>>>>> On Thu, Sep 19, 2019 at 10:16 AM Charles Givre <cg...@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>> Thanks for getting back with me! I'm really glad you're open to
>>>>>>>>>>> contributing this to Drill! I'm also really impressed that you were able
>>>>>>>>>>> to do this at all. Writing storage plugins is extremely difficult,
>>>>>>>>>>> especially given the lack of documentation, and that you have to have a
>>>>>>>>>>> really good understanding of both Druid and Drill's internals. So my
>>>>>>>>>>> hat's off to you!
>>>>>>>>>>> Anyway, I had a look at our JIRA board and it turns out that
>>>>>>>>>>> there is an open JIRA for a Drill/Druid integration:
>>>>>>>>>>> https://issues.apache.org/jira/browse/DRILL-5956
>>>>>>>>>>>
>>>>>>>>>>> When you're ready to do so, please create a pull request on the
>>>>>>>>>>> Drill github repo with the title: Drill-5956: Add Storage Plugin for Druid
>>>>>>>>>>> and add me as a reviewer. Thank you very much!!
>>>>>>>>>>> Best,
>>>>>>>>>>> -- C
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Sep 17, 2019, at 5:23 PM, Ankush Kapur <
>>>>>>>>>>> ankush.kapur@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Charles,
>>>>>>>>>>>
>>>>>>>>>>> I am good, thanks for asking. Hope the same for you.
>>>>>>>>>>>
>>>>>>>>>>> Yes, I would be happy to submit a pr. However. It will take
>>>>>>>>>>> sometime to get it in shape. The druid connector is a couple of years old
>>>>>>>>>>> now.
>>>>>>>>>>>
>>>>>>>>>>> - Ankush
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Sep 17, 2019, 2:27 PM Charles Givre <cg...@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Ankush,
>>>>>>>>>>>> I hope all is well. I'm the PMC chair for Apache Drill and I I
>>>>>>>>>>>> saw your Druid plugin on github. I wanted to ask if you would consider
>>>>>>>>>>>> contributing that to Drill and submitting a pull request. I'm happy to
>>>>>>>>>>>> help with the review process.
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> -- Charles
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>