Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

[Matthew Burgess]

Is Travis   a viable option for the GitHub route? I
use it for my own projects to build pull requests (with additional code
quality targets like CheckStyle, PMD, etc.). Perhaps that would take some of
the burden off the reviewers and let them focus on the proposed
implementations, rather than some of the more tedious aspects of each
review.

From:  Jacques Nadeau 
Reply-To:  
Date:  Monday, June 22, 2015 at 10:22 PM
To:  "dev@drill.apache.org" 
Subject:  Re: [DISCUSS] Allowing the option to use github pull requests in
place of reviewboard

I'm up for this if we deprecate the old way.  Having two different
processes seems like overkill.  In general, I find the review interface of
GitHub less expressive/clear but everything else is way better.

On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips 
wrote:

>  +1
> 
>  I am in favor of giving this a try.
> 
>  If I remember correctly, the reason we abandoned pull requests originally
>  was because we couldn't close the pull requests through Github. A solution
>  could be for whoever pushes the commit to the apache git repo to add the
>  Line "Closes ". Github would then automatically close the
>  pull request.
> 
>  On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse >  >
>  wrote:
> 
>>  > Hello Drill developers,
>>  >
>>  > I am writing this message today to propose allowing the use of github
>  pull
>>  > requests to perform reviews in place of the apache reviewboard instance.
>>  >
>>  > Reviewboard has caused a number of headaches in the past few months, and
>  I
>>  > think its time to evaluate the benefits of the apache infrastructure
>>  > relative to the actual cost of using it in practice.
>>  >
>>  > For clarity of the discussion, we cannot use the complete github
>  workflow.
>>  > Comitters will still need to use patch files, or check out the branch
>  used
>>  > in the review request and push to apache master manually. I am not
>>  > advocating for using a merging strategy with git, just for using the
>  github
>>  > web UI for reviews. I expect anyone generating a chain of commits as
>>  > described below to use the rebasing workflow we do today. Additionally
>  devs
>>  > should only be breaking up work to make it easier to review, we will not
>  be
>>  > reviewing branches that contain a bunch of useless WIP commits.
>>  >
>>  > A few examples of problems I have experienced with reviewboard include:
>>  > corruption of patches when they are downloaded, the web interface showing
>>  > inconsistent content from the raw diff, and random rejection of patches
>>  > that are based directly on the head of apache master.
>>  >
>>  > These are all serious blockers for getting code reviewed and integrated
>>  > into the master branch in a timely manner.
>>  >
>>  > In addition to serious bugs in reviewboard, there are a number of
>>  > difficulties with the combination of our typical dev workflow and how
>>  > reviewboard works with patches. As we are still adding features to Drill,
>>  > we often have several weeks of work to submit in response to a JIRA or
>>  > series of related JIRAs. Sometimes this work can be broken up into
>>  > independent reviewable units, and other times it cannot. When a series of
>>  > changes requires a mixture of refactoring and additions, the process is
>>  > currently quite painful. Ether reviewers need to look through a giant
>  messy
>>  > diff, or the submitters need to do a lot of extra work. This involves not
>>  > only organizing their work into a reviewable series of commits, but also
>>  > generating redundant squashed versions of the intermediate work to make
>>  > reviewboard happy.
>>  >
>>  > For a relatively simple 3 part change, this involves creating 3
>  reviewboard
>>  > pages. The first will contain the first commit by itself. The second will
>>  > have the first commits patch as a parent patch with the next change in
>  the
>>  > series uploaded as the core change to review. For the third change, a
>>  > squashed version of the first two commits must be generated to serve as a
>>  > parent patch and then the third changeset uploaded as the reviewable
>>  > change. Frequently a change to the first commit requires regenerating all
>>  > of these patches and uploading them to the individual review pages.
>>  >
>>  > This gets even worse with larger chains of commits.
>>  >
>>  > It would be great if all of our changes could be small units of work, but
>>  > very frequently we want to make sure we are ready to merge a complete
>>  > feature before starting the review process. We need to have a better way
>  to
>>  > manage these large review units, as I do not see the possibility of
>>  > breaking up the work into smaller units as a likely solution. We still
>  have
>>  > lots of features and system cleanup to work on.
>>  >
>>  > For anyone unfamiliar, github pull requests are based on a branch you
>  push
>>  > to your personal fork. They give space for a general discussion, as well
>  a

Re: Review Request 35739: Patch for DRILL-3333

[Aman Sinha]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88916
---


For unit testing, you could adapt the tests in TestPartitionFilter and replace 
the dir0, dir1 references with the partition column references. Many of these 
tests do plan checking by testing inclusion or exclusion of the Filter node.

- Aman Sinha


On June 22, 2015, 10:22 p.m., Steven Phillips wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35739/
> ---
> 
> (Updated June 22, 2015, 10:22 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-
> https://issues.apache.org/jira/browse/DRILL-
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> DRILL-: Parquet writer auto-partitioning and partition pruning
> 
> Conflicts:
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WriterPrel.java
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>   exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
> 
> 
> Diffs
> -
> 
>   exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java 
> 6b6065f6b6c8469aa548acf194e0621b9f4ffea8 
>   exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java 
> 797f3cb8c83a89821ee46ce0b093f81406fa6067 
>   exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
> PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/RecordWriter.java 
> c6325fd0a5c7d7cb5f3628df1ecf9c01c264ed52 
>   exec/java-exec/src/main/codegen/templates/StringOutputRecordWriter.java 
> f704cca0e4d62ca1435df84d9eb1b07b32ea8b39 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java
>  5c4ee4da9e0542244b0f71a520cea1c3a2d49a66 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java
>  2d16cd01b94ed8a5463c0e2fb896f019133f7f03 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java
>  d5d64a722ed6d9b5d97158046e6838f07c0d5381 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
>  d9b1354492454dcd2630c72f5dbc1c3badf958c7 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  920b2848d8edb62667b880e81f5aee12b459d63a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
> PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java
>  a43a4a0f21bf11f29b6385e36db4d25003ffa98f 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
>  cf39518b2a8b4564504a3971d1f89c268aee4b30 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
>  621f05c4d50ecf83071a5df414be88e7471f0490 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java
>  31b1fbe9e03282161ee125cb7a4b2f53c8a8da63 
> 
> Diff: https://reviews.apache.org/r/35739/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

[Ted Dunning]

On Mon, Jun 22, 2015 at 7:22 PM, Jacques Nadeau  wrote:

> In general, I find the review interface of
> GitHub less expressive/clear but everything else is way better.
>

The trade-off is that more people can figure out how to put PR's up for
review than who can run reviewboard accurately.

Re: Review Request 35739: Patch for DRILL-3333

[Aman Sinha]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88903
---



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
 (line 79)


The name should be ParquetPruneScanRule instead of PruneScanRule.



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
 (line 103)


Same as above.



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
 (line 129)


Agree with Jacques's comment about refactoring and sharing with 
PruneScanRule. Also add comments about whether these 2 rules should eventually 
be consolidated together.



exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
(line 20)


This interface does not seem to be used..remove.



exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java 
(line 39)


At first it wasn't clear why these functions are not part of the template 
NewValueFunctions; I understand it is because these are variable width types 
that cause the template code to be more complex.  You could add some comments 
here to that effect.



exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 (line 320)


Is the purpose of the 'first' flag to find the first column in the metadata 
that qualifies as a 'partition' column ?  But if this column is not the one 
that we are looking for (i.e this column is not in the filter condition of the 
query) then we should keep looking...so in that case do we really need this 
flag ?



exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 (line 426)


This return should be inside the if (stats != null) block and I suppose you 
could return false outside.


- Aman Sinha


On June 22, 2015, 10:22 p.m., Steven Phillips wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35739/
> ---
> 
> (Updated June 22, 2015, 10:22 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-
> https://issues.apache.org/jira/browse/DRILL-
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> DRILL-: Parquet writer auto-partitioning and partition pruning
> 
> Conflicts:
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WriterPrel.java
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>   exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
> 
> 
> Diffs
> -
> 
>   exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java 
> 6b6065f6b6c8469aa548acf194e0621b9f4ffea8 
>   exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java 
> 797f3cb8c83a89821ee46ce0b093f81406fa6067 
>   exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
> PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/RecordWriter.java 
> c6325fd0a5c7d7cb5f3628df1ecf9c01c264ed52 
>   exec/java-exec/src/main/codegen/templates/StringOutputRecordWriter.java 
> f704cca0e4d62ca1435df84d9eb1b07b32ea8b39 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java
>  5c4ee4da9e0542244b0f71a520cea1c3a2d49a66 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java
>  2d16cd01b94ed8a5463c0e2fb896f019133f7f03 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java
>  d5d64a722ed6d9b5d97158046e6838f07c0d5381 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
>  d9b1354492454dcd2630c72f5dbc1c3badf958c7 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  920b2848d8edb62667b880e81f5aee12b459d63a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
> PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/dri

Re: Review Request 35719: DRILL-3242: Update RPC layer so that requests and response are managed on a secondary thread.

[Chris Westin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35719/#review88896
---


The use of the Recycler seems like an unnecessary extra complication, and it 
prevents making the constructor arguments final. It seems like that stuff will 
be so short-lived (because of the SynchronousQueues), that it'll aways be in 
Eden space.


exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 
(line 433)


one blank between closing parens and opening brace



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 58)


private



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 193)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 209)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 210)


Can these be final?



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 316)


Don't you need some empty braces in the text for substitution?



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 336)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 347)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 351)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 364)


} finally {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 365)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 369)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 375)


no blank line



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 377)


no blank line



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 395)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 405)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 409)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 414)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 416)


It'd be great if these were all final.



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 417)


string description please



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 427)


} finally {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 428)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 432)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java (line 438)


no blank line here



exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 
(line 65)


We need some kind of global planning and dependency tracking.



exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 
(line 118)


No blank lines here.



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
 (line 492)


) {



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
 (line 498)


private



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
 (line 501)

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

[Jacques Nadeau]

I'm up for this if we deprecate the old way.  Having two different
processes seems like overkill.  In general, I find the review interface of
GitHub less expressive/clear but everything else is way better.

On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips 
wrote:

> +1
>
> I am in favor of giving this a try.
>
> If I remember correctly, the reason we abandoned pull requests originally
> was because we couldn't close the pull requests through Github. A solution
> could be for whoever pushes the commit to the apache git repo to add the
> Line "Closes ". Github would then automatically close the
> pull request.
>
> On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse  >
> wrote:
>
> > Hello Drill developers,
> >
> > I am writing this message today to propose allowing the use of github
> pull
> > requests to perform reviews in place of the apache reviewboard instance.
> >
> > Reviewboard has caused a number of headaches in the past few months, and
> I
> > think its time to evaluate the benefits of the apache infrastructure
> > relative to the actual cost of using it in practice.
> >
> > For clarity of the discussion, we cannot use the complete github
> workflow.
> > Comitters will still need to use patch files, or check out the branch
> used
> > in the review request and push to apache master manually. I am not
> > advocating for using a merging strategy with git, just for using the
> github
> > web UI for reviews. I expect anyone generating a chain of commits as
> > described below to use the rebasing workflow we do today. Additionally
> devs
> > should only be breaking up work to make it easier to review, we will not
> be
> > reviewing branches that contain a bunch of useless WIP commits.
> >
> > A few examples of problems I have experienced with reviewboard include:
> > corruption of patches when they are downloaded, the web interface showing
> > inconsistent content from the raw diff, and random rejection of patches
> > that are based directly on the head of apache master.
> >
> > These are all serious blockers for getting code reviewed and integrated
> > into the master branch in a timely manner.
> >
> > In addition to serious bugs in reviewboard, there are a number of
> > difficulties with the combination of our typical dev workflow and how
> > reviewboard works with patches. As we are still adding features to Drill,
> > we often have several weeks of work to submit in response to a JIRA or
> > series of related JIRAs. Sometimes this work can be broken up into
> > independent reviewable units, and other times it cannot. When a series of
> > changes requires a mixture of refactoring and additions, the process is
> > currently quite painful. Ether reviewers need to look through a giant
> messy
> > diff, or the submitters need to do a lot of extra work. This involves not
> > only organizing their work into a reviewable series of commits, but also
> > generating redundant squashed versions of the intermediate work to make
> > reviewboard happy.
> >
> > For a relatively simple 3 part change, this involves creating 3
> reviewboard
> > pages. The first will contain the first commit by itself. The second will
> > have the first commits patch as a parent patch with the next change in
> the
> > series uploaded as the core change to review. For the third change, a
> > squashed version of the first two commits must be generated to serve as a
> > parent patch and then the third changeset uploaded as the reviewable
> > change. Frequently a change to the first commit requires regenerating all
> > of these patches and uploading them to the individual review pages.
> >
> > This gets even worse with larger chains of commits.
> >
> > It would be great if all of our changes could be small units of work, but
> > very frequently we want to make sure we are ready to merge a complete
> > feature before starting the review process. We need to have a better way
> to
> > manage these large review units, as I do not see the possibility of
> > breaking up the work into smaller units as a likely solution. We still
> have
> > lots of features and system cleanup to work on.
> >
> > For anyone unfamiliar, github pull requests are based on a branch you
> push
> > to your personal fork. They give space for a general discussion, as well
> as
> > allow commenting inline on the diff. They give a clear reference to each
> > commit in the branch, allowing reviewers to see each piece of work
> > individually as well as provide a "squashed" view to see the overall
> > differences.
> >
> > For the sake of keeping the project history connected to JIRA, we can see
> > if there is enough automatic github integration or possibly upload patch
> > files to JIRA each time we update a pull request. As an side note, if we
> > don't need individual patches for reviewboard we could just put patch
> files
> > on JIRA that contain several commits. These are much easier to generate
> an
> > apply than a bunch of individual files for each change. This should
> prev

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

[Steven Phillips]

+1

I am in favor of giving this a try.

If I remember correctly, the reason we abandoned pull requests originally
was because we couldn't close the pull requests through Github. A solution
could be for whoever pushes the commit to the apache git repo to add the
Line "Closes ". Github would then automatically close the
pull request.

On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse 
wrote:

> Hello Drill developers,
>
> I am writing this message today to propose allowing the use of github pull
> requests to perform reviews in place of the apache reviewboard instance.
>
> Reviewboard has caused a number of headaches in the past few months, and I
> think its time to evaluate the benefits of the apache infrastructure
> relative to the actual cost of using it in practice.
>
> For clarity of the discussion, we cannot use the complete github workflow.
> Comitters will still need to use patch files, or check out the branch used
> in the review request and push to apache master manually. I am not
> advocating for using a merging strategy with git, just for using the github
> web UI for reviews. I expect anyone generating a chain of commits as
> described below to use the rebasing workflow we do today. Additionally devs
> should only be breaking up work to make it easier to review, we will not be
> reviewing branches that contain a bunch of useless WIP commits.
>
> A few examples of problems I have experienced with reviewboard include:
> corruption of patches when they are downloaded, the web interface showing
> inconsistent content from the raw diff, and random rejection of patches
> that are based directly on the head of apache master.
>
> These are all serious blockers for getting code reviewed and integrated
> into the master branch in a timely manner.
>
> In addition to serious bugs in reviewboard, there are a number of
> difficulties with the combination of our typical dev workflow and how
> reviewboard works with patches. As we are still adding features to Drill,
> we often have several weeks of work to submit in response to a JIRA or
> series of related JIRAs. Sometimes this work can be broken up into
> independent reviewable units, and other times it cannot. When a series of
> changes requires a mixture of refactoring and additions, the process is
> currently quite painful. Ether reviewers need to look through a giant messy
> diff, or the submitters need to do a lot of extra work. This involves not
> only organizing their work into a reviewable series of commits, but also
> generating redundant squashed versions of the intermediate work to make
> reviewboard happy.
>
> For a relatively simple 3 part change, this involves creating 3 reviewboard
> pages. The first will contain the first commit by itself. The second will
> have the first commits patch as a parent patch with the next change in the
> series uploaded as the core change to review. For the third change, a
> squashed version of the first two commits must be generated to serve as a
> parent patch and then the third changeset uploaded as the reviewable
> change. Frequently a change to the first commit requires regenerating all
> of these patches and uploading them to the individual review pages.
>
> This gets even worse with larger chains of commits.
>
> It would be great if all of our changes could be small units of work, but
> very frequently we want to make sure we are ready to merge a complete
> feature before starting the review process. We need to have a better way to
> manage these large review units, as I do not see the possibility of
> breaking up the work into smaller units as a likely solution. We still have
> lots of features and system cleanup to work on.
>
> For anyone unfamiliar, github pull requests are based on a branch you push
> to your personal fork. They give space for a general discussion, as well as
> allow commenting inline on the diff. They give a clear reference to each
> commit in the branch, allowing reviewers to see each piece of work
> individually as well as provide a "squashed" view to see the overall
> differences.
>
> For the sake of keeping the project history connected to JIRA, we can see
> if there is enough automatic github integration or possibly upload patch
> files to JIRA each time we update a pull request. As an side note, if we
> don't need individual patches for reviewboard we could just put patch files
> on JIRA that contain several commits. These are much easier to generate an
> apply than a bunch of individual files for each change. This should prevent
> JIRAs needing long lists of patches with names like
> DRILL-3000-part1-version3.patch
>



-- 
 Steven Phillips
 Software Engineer

 mapr.com

[jira] [Resolved] (DRILL-3294) False schema change exception in CTAS with AVG window function

[Jinfeng Ni (JIRA)]

 [ 
https://issues.apache.org/jira/browse/DRILL-3294?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jinfeng Ni resolved DRILL-3294.
---
Resolution: Fixed

Fixed in commit :  ffae1691c0cd526ed1095fbabbc0855d016790d7. 



> False schema change exception in CTAS with AVG window function
> --
>
> Key: DRILL-3294
> URL: https://issues.apache.org/jira/browse/DRILL-3294
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Query Planning & Optimization
>Affects Versions: 1.0.0
>Reporter: Victoria Markman
>Assignee: Jinfeng Ni
>  Labels: window_function
> Fix For: 1.1.0
>
> Attachments: t1_parquet
>
>
> This bug could be related to DRILL-3293, but since it's a different function 
> and different symptom, I'm filing a new one.
> {code}
> 0: jdbc:drill:schema=dfs> create table wf_t1(a1) as select avg(a1) 
> over(partition by a1) from t1;
> Error: SYSTEM ERROR: org.apache.drill.exec.exception.SchemaChangeException: 
> Failure while trying to materialize incoming schema.  Errors:
>  
> Error in expression at index -1.  Error: Missing function implementation: 
> [castTINYINT(NULL-OPTIONAL)].  Full expression: --UNKNOWN EXPRESSION--..
> Fragment 0:0
> [Error Id: 1ca5af3a-0ea7-4b75-b493-74f6404d4894 on atsqa4-133.qa.lab:31010] 
> (state=,code=0)
> {code}
> Query works correctly by itself:
> {code}
> 0: jdbc:drill:schema=dfs> select avg(a1) over(partition by a1) from t1;
> +-+
> | EXPR$0  |
> +-+
> | 1   |
> | 2   |
> | 3   |
> | 4   |
> | 5   |
> | 6   |
> | 7   |
> | 9   |
> | 10  |
> | null|
> +-+
> 10 rows selected (0.181 seconds)
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 35484: DRILL-2851: set an upper-bound on # of bytes to re-allocate to prevent overflows

[Hanifi Gunes]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35484/
---

(Updated June 23, 2015, 1:20 a.m.)


Review request for drill, Mehant Baid and Venki Korukanti.


Changes
---

Added unit tests to cover various reallocation scenarios.
Fixed unit tests to avoid leaks via closing out resources.
VVs throw OversizedAllocationException if allocation demand is more than the 
allowed max.
Ensure that flatten handles OversizedAllocationException to split the batch and 
resume the execution.


Repository: drill-git


Description
---

DRILL-2851: set an upper-bound on # of bytes to re-allocate to prevent overflows
Vectors
- set an upper bound on # of bytes to allocate
- 
TestValueVector.java
- Add unit tests


Diffs (updated)
-

  exec/java-exec/src/main/codegen/includes/vv_imports.ftl 
92c80072cfcde4deb0bbb34bc3b688707541f2f6 
  exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 
7103a17108693d47839212c418d11d13fbb8f6f4 
  exec/java-exec/src/main/codegen/templates/NullableValueVectors.java 
7f835424b68a9d68b0a6c60749677a83ac486590 
  exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 
50ae770f24aff1e8eed1dfa800878ce92308c644 
  
exec/java-exec/src/main/java/org/apache/drill/exec/exception/OversizedAllocationException.java
 PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java
 999140498ab303d3f5ecf20695755bdfe943cb46 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenTemplate.java
 de67b62248a68c1f483808c4b575e0afa7854aca 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/Flattener.java
 92cf79d37da89864ab7702830fe078479773a73e 
  
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseDataValueVector.java
 0e38f3cad3792e936ff918ae970f4b40e478d516 
  
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java 
8129668b6ff5dc674e30dca6947bd93c87fb4d3d 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 
10bdf0752632c7577b9a6eb445c7101ec1a24730 
  
exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestValueVector.java
 037c8c6d3da94acf5c2ca300ce617338cacb0fb0 

Diff: https://reviews.apache.org/r/35484/diff/


Testing
---

all


Thanks,

Hanifi Gunes

Re: Review Request 34004: DRILL-1942: Improved memory allocator

[Jacques Nadeau]

> On June 18, 2015, 4:40 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java,
> >  line 699
> > 
> >
> > Can you discuss the change here?  It seems like we should actually just 
> > fail if the length.value is negative.
> > 
> > Should probably also localize the length.value given multiple 
> > references in perf-sensitive code.
> 
> Chris Westin wrote:
> Apparently a negative value was expected some of the time. See the cases 
> down below, which explicitly test for it. There are similar cases throughout 
> the file. Yeah, it seems wrong to me, but I don't know the calling code. I 
> don't know why it's like that, but I do know that the allocator started 
> complaining when I added the checked for negative sizes. (Who knows what it 
> did before???)
> 
> As requested, I localized the length.value in two of the functions that 
> had lots of references. Side note: you don't think that's excessive 
> optimization, but you do think release(1)/retain(1) is?

Release and retain are typically done once per batch.  This operation is done 
once per record.  My optimization expectation is much higher for records than 
batches.


> On June 18, 2015, 4:40 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java, line 
> > 78
> > 
> >
> > Not sure why this would be a system level setting.  It should be a user 
> > level setting.
> 
> Chris Westin wrote:
> Neither of the allocation policies we've discussed can be a user-level 
> policy.
> (1) Memory is divided equally across all fragments running on a drillbit.
> This is the currently implemented policy. This doesn't take users into 
> account. Because of that, we can't let one user have this setting, and 
> another choose something else (if we had anything else).
> (2) On each drillbit, memory is divided equally across all queries 
> running on that drillbit (regardless of how many fragments that includes). 
> Note that this also doesn't take users into account. And it wouldn't work to 
> combine this with (1), unless we also had something higher level that split 
> things across users in some way. For example, We could say that IF we were to 
> support splitting memory on a drillbit evenly across all users who have 
> anything running on that drillbit, then within those arenas, a user could 
> select (1) or (2), but only within the user's individual arena.

I'll do a better job of documenting my thoughts in the future as your 
understanding of what we discussed is different than what I attempted to 
express.


> On June 18, 2015, 4:40 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java, line 320
> > 
> >
> > Since we have a clean approach to the underlying buffers now, can we 
> > modify by this so the return is always (return == this) is always true?

I saw a drop here but not explanation.


> On June 18, 2015, 4:40 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java, line 
> > 142
> > 
> >
> > I generally think this is an excessive optimization throughout the code 
> > that you've added.
> 
> Chris Westin wrote:
> Opinion noted; what action do you want me to take? Do you want me to 
> remove them?

I'm fine with them but I wouldn't have a general expectation of this.


- Jacques


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34004/#review88232
---


On June 17, 2015, 1:07 a.m., Chris Westin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34004/
> ---
> 
> (Updated June 17, 2015, 1:07 a.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jason Altekruse.
> 
> 
> Bugs: DRILL-1942
> https://issues.apache.org/jira/browse/DRILL-1942
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> Rewritten direct memory allocator. Simplified interface, and use, along with 
> a means to support additional allocation policies in the future. There are 
> features in the allocator and in DrillBuf that make finding leaks easier, as 
> well as better enforcement of limits. New features include transfer of 
> buffers, and better slicing support.
> 
> This is a preliminary patch to get the review started because it touches a 
> lot of files (readers and record batches were made AutoCloseable in order t

Re: Review Request 34004: DRILL-1942: Improved memory allocator

[Jacques Nadeau]

> On June 22, 2015, 4:06 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 304
> > 
> >
> > Let's get rid of the int flags.
> 
> Chris Westin wrote:
> Disagree. More than one flag can be passed at a time. Calling f(FLAG_X | 
> FLAG_Y) is a lot more readable than f(true, false, true). (I also think this 
> is better than the Google mandated solution of creating a separate enum (with 
> XXX_ON and XXX_OFF) for each boolean.)

I understand the concept of multiple flags.  However, I'm not sure why we 
wouldn't use a configuration object rather than bits.  It isn't like we have 
huge amounts of these and want to save on memory.  If you want to optimize, we 
can hold booleans internally.  (It also seems like you really only have one 
flag once the other one is deprecated.)


> On June 22, 2015, 4:06 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 467
> > 
> >
> > Why would we grab a global allocator lock if we aren't releasing to the 
> > parent?  This seems rather expensive.  Even if we are releasing to the 
> > parent, we should avoid grabbing the lock for any longer than is necessary.
> > 
> > In fact, my thought would be that the parent should manage thread 
> > safety, not the child.  Why does the child have to protect the parent.  If 
> > this is actually for low variable use, we should use a local allocator lock 
> > rather than a global lock.
> 
> Chris Westin wrote:
> I've been through a bunch of variations on this, and the other clever 
> ones all led to problems. One attempt included per-allocator locks, and that 
> caused a lot of deadlocks as allocations were shuffled between allocators. In 
> that world, I added code to try to secure all conceivably required locks for 
> an operation before-hand, in a pre-determined order, and that was a mess; the 
> possibility of a line of descendant child allocators all having to return 
> things up the chain to their parent at the time the deepest one released 
> something caused problems. Ownership transfer and sharing (which tend to work 
> laterally, rather than up and down the allocator hierarchy) made things 
> worse. This is the simplest scheme I found that works. Given the relative 
> simplicity of the allocator's work compared to all the stuff around it, 
> contention doesn't seem to be a problem.

I'm very uncomfortable with this.  Adding a global allocation/deallocation lock 
seems like a sledgehammer.  Have you done any 50x or 100x concurrency testing 
showing that this is of nominal impact?  I'd much rather have a group 
discussion on the approaches you took and the issues of each than just add a 
global lock.


> On June 22, 2015, 4:06 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 496
> > 
> >
> > Method name is misleading.
> 
> Chris Westin wrote:
> Ok, what would you suggest instead?

I can't think of anything better.  onChildClosed?


- Jacques


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34004/#review88721
---


On June 17, 2015, 1:07 a.m., Chris Westin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34004/
> ---
> 
> (Updated June 17, 2015, 1:07 a.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jason Altekruse.
> 
> 
> Bugs: DRILL-1942
> https://issues.apache.org/jira/browse/DRILL-1942
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> Rewritten direct memory allocator. Simplified interface, and use, along with 
> a means to support additional allocation policies in the future. There are 
> features in the allocator and in DrillBuf that make finding leaks easier, as 
> well as better enforcement of limits. New features include transfer of 
> buffers, and better slicing support.
> 
> This is a preliminary patch to get the review started because it touches a 
> lot of files (readers and record batches were made AutoCloseable in order to 
> cover cleanup). Subsequent reviews can use the differential view to just see 
> additional changes. The new allocator is in BaseAllocator.java (along with 
> derived classes RootAllocator and ChildAllocator); DrillBuf also has 
> significant changes. Most other changes in other files are just to use newer 
> interfaces, or to change cleanup() to close(), or to close subordinate 
> objects tha

[jira] [Resolved] (DRILL-3293) CTAS with window function fails with UnsupportedOperationException

[Jinfeng Ni (JIRA)]

 [ 
https://issues.apache.org/jira/browse/DRILL-3293?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jinfeng Ni resolved DRILL-3293.
---
Resolution: Fixed

fixed in commit: ffae1691c0cd526ed1095fbabbc0855d016790d7

> CTAS with window function fails with UnsupportedOperationException
> --
>
> Key: DRILL-3293
> URL: https://issues.apache.org/jira/browse/DRILL-3293
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Execution - Flow
>Affects Versions: 1.0.0
>Reporter: Victoria Markman
>Assignee: Jinfeng Ni
>  Labels: window_function
> Fix For: 1.1.0
>
> Attachments: t1_parquet
>
>
> {code}
> 0: jdbc:drill:schema=dfs> create table wf_t1 as select sum(a1) over(partition 
> by a1) from t1;
> Error: SYSTEM ERROR:
> Fragment 0:0
> [Error Id: 96897b46-70c0-4373-9d85-ca7501cb1479 on atsqa4-133.qa.lab:31010] 
> (state=,code=0)
> {code}
> drillbit.log
> {code}
> [Error Id: bde0d90b-7eaa-4772-9316-9c58a46b01d2 on atsqa4-133.qa.lab:31010]
> org.apache.drill.common.exceptions.UserException: SYSTEM ERROR:
> Fragment 0:0
> [Error Id: bde0d90b-7eaa-4772-9316-9c58a46b01d2 on atsqa4-133.qa.lab:31010]
> at 
> org.apache.drill.common.exceptions.UserException$Builder.build(UserException.java:522)
>  ~[drill-common-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.work.fragment.FragmentExecutor.sendFinalState(FragmentExecutor.java:325)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.work.fragment.FragmentExecutor.cleanup(FragmentExecutor.java:181)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.work.fragment.FragmentExecutor.run(FragmentExecutor.java:294)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.common.SelfCleaningRunnable.run(SelfCleaningRunnable.java:38)
>  [drill-common-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>  [na:1.7.0_71]
> at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>  [na:1.7.0_71]
> at java.lang.Thread.run(Thread.java:745) [na:1.7.0_71]
> Caused by: java.lang.UnsupportedOperationException: null
> at 
> org.apache.drill.exec.expr.TypeHelper.getValueVectorClass(TypeHelper.java:674)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.VectorContainer.addOrGet(VectorContainer.java:82)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.physical.impl.project.ProjectRecordBatch.setupNewSchema(ProjectRecordBatch.java:421)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractSingleRecordBatch.innerNext(AbstractSingleRecordBatch.java:78)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.physical.impl.project.ProjectRecordBatch.innerNext(ProjectRecordBatch.java:129)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:146)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:105)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:95)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractSingleRecordBatch.innerNext(AbstractSingleRecordBatch.java:51)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.physical.impl.project.ProjectRecordBatch.innerNext(ProjectRecordBatch.java:129)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:146)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:105)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:95)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.physical.impl.WriterRecordBatch.innerNext(WriterRecordBatch.java:92)
>  ~[drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(Abstract

[jira] [Created] (DRILL-3341) Move OperatorWrapper list and FragmentWrapper list creation to ProfileWrapper ctor

[Sudheesh Katkam (JIRA)]

Sudheesh Katkam created DRILL-3341:
--

 Summary: Move OperatorWrapper list and FragmentWrapper list 
creation to ProfileWrapper ctor
 Key: DRILL-3341
 URL: https://issues.apache.org/jira/browse/DRILL-3341
 Project: Apache Drill
  Issue Type: Improvement
Reporter: Sudheesh Katkam
Assignee: Sudheesh Katkam
Priority: Minor
 Fix For: 1.1.0


+ avoid re-computation in some cases



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (DRILL-3340) Add metric name to MetricValue

[Sudheesh Katkam (JIRA)]

Sudheesh Katkam created DRILL-3340:
--

 Summary: Add metric name to MetricValue
 Key: DRILL-3340
 URL: https://issues.apache.org/jira/browse/DRILL-3340
 Project: Apache Drill
  Issue Type: Improvement
Reporter: Sudheesh Katkam
Assignee: Sudheesh Katkam
Priority: Minor
 Fix For: 1.1.0


Useful when reading JSON query profile.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 35739: Patch for DRILL-3333

[Jinfeng Ni]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88864
---



exec/java-exec/src/main/codegen/templates/NewValueFunctions.java (line 44)


What will happen if the partition column is Decimal28, Decimal38, or 
Interval type? Will parquet writer check if the column data type is supported 
or not?



exec/java-exec/src/main/codegen/templates/NewValueFunctions.java (line 55)


Are we going to create multiple files if the same values are across 
multiple record batch boundary?



exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
 (line 156)


The compare function uses "NULL_IF_NULL" null handing policy. In case the 
partitioning column is null-able, should we use NullableBitVector.class here?


- Jinfeng Ni


On June 22, 2015, 3:22 p.m., Steven Phillips wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35739/
> ---
> 
> (Updated June 22, 2015, 3:22 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-
> https://issues.apache.org/jira/browse/DRILL-
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> DRILL-: Parquet writer auto-partitioning and partition pruning
> 
> Conflicts:
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WriterPrel.java
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>   exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
> 
> 
> Diffs
> -
> 
>   exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java 
> 6b6065f6b6c8469aa548acf194e0621b9f4ffea8 
>   exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java 
> 797f3cb8c83a89821ee46ce0b093f81406fa6067 
>   exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
> PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/RecordWriter.java 
> c6325fd0a5c7d7cb5f3628df1ecf9c01c264ed52 
>   exec/java-exec/src/main/codegen/templates/StringOutputRecordWriter.java 
> f704cca0e4d62ca1435df84d9eb1b07b32ea8b39 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java
>  5c4ee4da9e0542244b0f71a520cea1c3a2d49a66 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java
>  2d16cd01b94ed8a5463c0e2fb896f019133f7f03 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java
>  d5d64a722ed6d9b5d97158046e6838f07c0d5381 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
>  d9b1354492454dcd2630c72f5dbc1c3badf958c7 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  920b2848d8edb62667b880e81f5aee12b459d63a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
> PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java
>  a43a4a0f21bf11f29b6385e36db4d25003ffa98f 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
>  cf39518b2a8b4564504a3971d1f89c268aee4b30 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
>  621f05c4d50ecf83071a5df414be88e7471f0490 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java
>  31b1fbe9e03282161ee125cb7a4b2f53c8a8da63 
> 
> Diff: https://reviews.apache.org/r/35739/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

[jira] [Resolved] (DRILL-3338) Error message must be modified when PERCENT_RANK window functions are used without ORDER BY

[Victoria Markman (JIRA)]

 [ 
https://issues.apache.org/jira/browse/DRILL-3338?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Victoria Markman resolved DRILL-3338.
-
Resolution: Duplicate

> Error message must be modified when PERCENT_RANK window functions are used 
> without ORDER BY 
> 
>
> Key: DRILL-3338
> URL: https://issues.apache.org/jira/browse/DRILL-3338
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Query Planning & Optimization
>Affects Versions: 1.1.0
>Reporter: Abhishek Girish
>Assignee: Sean Hsuan-Yi Chu
>Priority: Minor
>
> The following message is thrown when a query containing PERCENT_RANK window 
> function fails:
> {code:sql}
> SELECT PERCENT_RANK() OVER (PARTITION BY ss.ss_store_sk) FROM store_sales ss 
> LIMIT 20;
> Error: PARSE ERROR: From line 1, column 28 to line 1, column 56: RANK or 
> DENSE_RANK functions require ORDER BY clause in window specification
> {code}
> The error message must be modified to include PERCENT_RANK function



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Resolved] (DRILL-3265) Query with window function and sort below that spills to disk runs out of memory

[Victoria Markman (JIRA)]

 [ 
https://issues.apache.org/jira/browse/DRILL-3265?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Victoria Markman resolved DRILL-3265.
-
Resolution: Fixed

> Query with window function and sort below that spills to disk runs out of 
> memory
> 
>
> Key: DRILL-3265
> URL: https://issues.apache.org/jira/browse/DRILL-3265
> Project: Apache Drill
>  Issue Type: Bug
>  Components: Execution - Relational Operators
>Affects Versions: 1.0.0
>Reporter: Victoria Markman
>Assignee: Deneche A. Hakim
>  Labels: window_function
> Fix For: 1.1.0
>
> Attachments: drill-3265.log
>
>
> Failure:
> {code}
> 0: jdbc:drill:schema=dfs> select
> . . . . . . . . . . . . >   sum(ss_quantity) over(partition by ss_store_sk 
> order by ss_sold_date_sk)
> . . . . . . . . . . . . > from store_sales;
> java.lang.RuntimeException: java.sql.SQLException: RESOURCE ERROR: One or 
> more nodes ran out of memory while executing the query.
> Fragment 1:13
> [Error Id: 72609220-e431-41fc-8505-8f9740c96153 on atsqa4-133.qa.lab:31010]
> at sqlline.IncrementalRows.hasNext(IncrementalRows.java:73)
> at 
> sqlline.TableOutputFormat$ResizingRowsProvider.next(TableOutputFormat.java:85)
> at sqlline.TableOutputFormat.print(TableOutputFormat.java:116)
> at sqlline.SqlLine.print(SqlLine.java:1583)
> at sqlline.Commands.execute(Commands.java:852)
> at sqlline.Commands.sql(Commands.java:751)
> at sqlline.SqlLine.dispatch(SqlLine.java:738)
> at sqlline.SqlLine.begin(SqlLine.java:612)
> at sqlline.SqlLine.start(SqlLine.java:366)
> at sqlline.SqlLine.main(SqlLine.java:259)
> {code}
> Failure looks legitimate from customer point of view (granted that we know 
> that we can run out of memory in some cases)
> However, there are couple of problems with this scenario:
> 1. It looks like we are running out of memory during disk based sort
> {code}
> 2015-06-09 16:55:45,693 [2a88e633-b714-49a1-c36a-509a9c817c77:frag:1:19] WARN 
>  o.a.d.e.p.i.xsort.ExternalSortBatch - Starting to merge. 3311 batch groups. 
> Current allocated memory: 17456644
> 2015-06-09 16:55:45,763 [2a88e633-b714-49a1-c36a-509a9c817c77:frag:1:13] WARN 
>  o.a.d.exec.memory.BufferAllocator - Unable to allocate buffer of size 20833 
> due to memory limit. Current allocation: 19989451
> java.lang.Exception: null
> at 
> org.apache.drill.exec.memory.TopLevelAllocator$ChildAllocator.buffer(TopLevelAllocator.java:253)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.memory.TopLevelAllocator$ChildAllocator.buffer(TopLevelAllocator.java:273)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.vector.UInt1Vector.allocateNew(UInt1Vector.java:171) 
> [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.vector.NullableIntVector.allocateNew(NullableIntVector.java:204)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.vector.AllocationHelper.allocateNew(AllocationHelper.java:56)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.test.generated.PriorityQueueCopierGen139.allocateVectors(PriorityQueueCopierTemplate.java:123)
>  [na:na]
> at 
> org.apache.drill.exec.test.generated.PriorityQueueCopierGen139.next(PriorityQueueCopierTemplate.java:66)
>  [na:na]
> at 
> org.apache.drill.exec.physical.impl.xsort.ExternalSortBatch.innerNext(ExternalSortBatch.java:224)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:146)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:105)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:95)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractSingleRecordBatch.innerNext(AbstractSingleRecordBatch.java:51)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.physical.impl.svremover.RemovingRecordBatch.innerNext(RemovingRecordBatch.java:92)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:146)
>  [drill-java-exec-1.1.0-SNAPSHOT-rebuffed.jar:1.1.0-SNAPSHOT]
> at 
> org.apache.drill.exec.record.AbstractRecordBatch.next(Abstra

Re: Review Request 34004: DRILL-1942: Improved memory allocator

[Chris Westin]

> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/AllocationReservation.java,
> >  line 55
> > 
> >
> > Preconditions.checkArgument

For this it should be Preconditions.checkState().


> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 54
> > 
> >
> > should have caps name

I disagree for non-constants.


> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 304
> > 
> >
> > Let's get rid of the int flags.

Disagree. More than one flag can be passed at a time. Calling f(FLAG_X | 
FLAG_Y) is a lot more readable than f(true, false, true). (I also think this is 
better than the Google mandated solution of creating a separate enum (with 
XXX_ON and XXX_OFF) for each boolean.)


> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 467
> > 
> >
> > Why would we grab a global allocator lock if we aren't releasing to the 
> > parent?  This seems rather expensive.  Even if we are releasing to the 
> > parent, we should avoid grabbing the lock for any longer than is necessary.
> > 
> > In fact, my thought would be that the parent should manage thread 
> > safety, not the child.  Why does the child have to protect the parent.  If 
> > this is actually for low variable use, we should use a local allocator lock 
> > rather than a global lock.

I've been through a bunch of variations on this, and the other clever ones all 
led to problems. One attempt included per-allocator locks, and that caused a 
lot of deadlocks as allocations were shuffled between allocators. In that 
world, I added code to try to secure all conceivably required locks for an 
operation before-hand, in a pre-determined order, and that was a mess; the 
possibility of a line of descendant child allocators all having to return 
things up the chain to their parent at the time the deepest one released 
something caused problems. Ownership transfer and sharing (which tend to work 
laterally, rather than up and down the allocator hierarchy) made things worse. 
This is the simplest scheme I found that works. Given the relative simplicity 
of the allocator's work compared to all the stuff around it, contention doesn't 
seem to be a problem.


> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 496
> > 
> >
> > Method name is misleading.

Ok, what would you suggest instead?


> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 554
> > 
> >
> > Same as above, why not?

That will cause getEmpty() to have problems, as an allocator would no longer 
have it's own empty buffer; it could end up handing out another allocator's 
empty buffer, which could lead to complaints when that allocator is closed, 
even though the user thought they were using this allocator. This doesn't 
appear to cause any problems at this time, so I don't see a reason to make it 
work.


> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 548
> > 
> >
> > Why not?

I'm not sure this can't be done, but I don't have a good feeling about it. It 
doesn't seem to be required, so I don't see why we should support it at this 
time. See the comments re ownership change of the empty buffer.


> On June 21, 2015, 9:06 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/BaseAllocator.java,
> >  line 575
> > 
> >
> > I don't think we need resurrection.  Previously I thought so but upon 
> > further review, it seems overly complicated and unecessary.  Did you find a 
> > place where it was required?

It's not just overly complicated and unnecessary, but it's also not really 
doable. The problem was described in a comment there -- if a Fragment has been 
cancelled and cleaned up, and its allocator is resurrected, there's no handle 
to that to support closing it again, because its owning fragment is completely 
gone.


> On June 21, 2015, 9:

Re: Review Request 34004: DRILL-1942: Improved memory allocator

[Chris Westin]

> On June 18, 2015, 12:09 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java,
> >  line 80
> > 
> >
> > Using ExecutionControlsInjector implies executionsControls is non-null. 
> > On the latest master, there is a precondition check. 
> > 
> > Why is this change necessary?

I ran into a case where sometimes one wasn't available. I can't remember the 
details now.


> On June 18, 2015, 12:09 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java,
> >  line 74
> > 
> >
> > cancel() and isCancelled() need to be synchronized methods

Not necessary, because the variable is volatile. cancel() is already 
synchronized internally, but I changed to to be function-level to make it more 
obvious.


> On June 18, 2015, 12:09 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java,
> >  line 216
> > 
> >
> > Can you use fail(...) instead?

Can you be more specific? Use fail() how? This function does a fair amount.


- Chris


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34004/#review87844
---


On June 16, 2015, 6:07 p.m., Chris Westin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34004/
> ---
> 
> (Updated June 16, 2015, 6:07 p.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jason Altekruse.
> 
> 
> Bugs: DRILL-1942
> https://issues.apache.org/jira/browse/DRILL-1942
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> Rewritten direct memory allocator. Simplified interface, and use, along with 
> a means to support additional allocation policies in the future. There are 
> features in the allocator and in DrillBuf that make finding leaks easier, as 
> well as better enforcement of limits. New features include transfer of 
> buffers, and better slicing support.
> 
> This is a preliminary patch to get the review started because it touches a 
> lot of files (readers and record batches were made AutoCloseable in order to 
> cover cleanup). Subsequent reviews can use the differential view to just see 
> additional changes. The new allocator is in BaseAllocator.java (along with 
> derived classes RootAllocator and ChildAllocator); DrillBuf also has 
> significant changes. Most other changes in other files are just to use newer 
> interfaces, or to change cleanup() to close(), or to close subordinate 
> objects that are newly (Auto)Closeable. 1There are still a couple of things 
> to do:
> * Some TODO(cwestin)s to clean up tracing and debugging code, as well as 
> adding javadoc
> * Using the AllocatorOwner interface to replace the reallocation mechanism 
> for FragmentContext and OperatorContext so that the allocator doesn't know 
> anything about those objects.
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/drill/common/AutoCloseablePointer.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/DrillAutoCloseables.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/DrillCloseables.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/HistoricalLog.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/StackTrace.java 54068ec 
>   common/src/main/java/org/apache/drill/common/config/DrillConfig.java 
> 522303f 
>   common/src/main/java/org/apache/drill/common/config/NestedConfig.java 
> 3fd885f 
>   
> contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
>  9458db2 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java
>  3c8b9ba 
>   
> contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java
>  182f5a4 
>   exec/java-exec/src/main/codegen/templates/AbstractFieldWriter.java 1b5dad1 
>   exec/java-exec/src/main/codegen/templates/BaseWriter.java ada410d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 980f9ac 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 0dffa0b 
>   exec/java-exec/src/main/codegen/templates/JsonOutputRecordWriter.java 
> ea643f0 
>   exec/java-exec/src/main/codegen/templates/ListWriters.java ab78603 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java 06a6813 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java ce6a3a7 
>   exec/java-exec/src/mai

Re: Review Request 35739: Patch for DRILL-3333

[Jacques Nadeau]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/#review88842
---



exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java
 (line 28)


please add javadocs



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
 (line 129)


These seems like a lot of duplication from the general partition pruning 
rule.  Can you refactor shared code into a common place?



exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
(line 20)


javadoc



exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 (line 278)


Is creating the metadata converter repeatedly expensive?



exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 (line 319)


short comment.



exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 (line 386)


What about other types, such as timestamp?



exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
 (line 220)


shouldn't swallow exception.


- Jacques Nadeau


On June 22, 2015, 10:22 p.m., Steven Phillips wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35739/
> ---
> 
> (Updated June 22, 2015, 10:22 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-
> https://issues.apache.org/jira/browse/DRILL-
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> DRILL-: Parquet writer auto-partitioning and partition pruning
> 
> Conflicts:
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WriterPrel.java
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>   exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
> 
> 
> Diffs
> -
> 
>   exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java 
> 6b6065f6b6c8469aa548acf194e0621b9f4ffea8 
>   exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java 
> 797f3cb8c83a89821ee46ce0b093f81406fa6067 
>   exec/java-exec/src/main/codegen/templates/NewValueFunctions.java 
> PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/RecordWriter.java 
> c6325fd0a5c7d7cb5f3628df1ecf9c01c264ed52 
>   exec/java-exec/src/main/codegen/templates/StringOutputRecordWriter.java 
> f704cca0e4d62ca1435df84d9eb1b07b32ea8b39 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java
>  5c4ee4da9e0542244b0f71a520cea1c3a2d49a66 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java
>  2d16cd01b94ed8a5463c0e2fb896f019133f7f03 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java
>  d5d64a722ed6d9b5d97158046e6838f07c0d5381 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
>  d9b1354492454dcd2630c72f5dbc1c3badf958c7 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  920b2848d8edb62667b880e81f5aee12b459d63a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
> PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java
>  a43a4a0f21bf11f29b6385e36db4d25003ffa98f 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
>  cf39518b2a8b4564504a3971d1f89c268aee4b30 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
>  621f05c4d50ecf83071a5df414be88e7471f0490 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java
>  31b1fbe9e03282161ee125cb7a4b2f53c8a8da63 
> 
> Diff: https://reviews.apache.org/r/35739/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 35739: Patch for DRILL-3333

[Steven Phillips]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/
---

(Updated June 22, 2015, 10:22 p.m.)


Review request for drill.


Bugs: DRILL-
https://issues.apache.org/jira/browse/DRILL-


Repository: drill-git


Description
---

DRILL-: Parquet writer auto-partitioning and partition pruning

Conflicts:

exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WriterPrel.java

exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java


Diffs (updated)
-

  exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java 
6b6065f6b6c8469aa548acf194e0621b9f4ffea8 
  exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java 
797f3cb8c83a89821ee46ce0b093f81406fa6067 
  exec/java-exec/src/main/codegen/templates/NewValueFunctions.java PRE-CREATION 
  exec/java-exec/src/main/codegen/templates/RecordWriter.java 
c6325fd0a5c7d7cb5f3628df1ecf9c01c264ed52 
  exec/java-exec/src/main/codegen/templates/StringOutputRecordWriter.java 
f704cca0e4d62ca1435df84d9eb1b07b32ea8b39 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java
 5c4ee4da9e0542244b0f71a520cea1c3a2d49a66 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java 
2d16cd01b94ed8a5463c0e2fb896f019133f7f03 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java
 d5d64a722ed6d9b5d97158046e6838f07c0d5381 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java
 PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
 d9b1354492454dcd2630c72f5dbc1c3badf958c7 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
 PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
 920b2848d8edb62667b880e81f5aee12b459d63a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java 
PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java
 a43a4a0f21bf11f29b6385e36db4d25003ffa98f 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 cf39518b2a8b4564504a3971d1f89c268aee4b30 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
 621f05c4d50ecf83071a5df414be88e7471f0490 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java
 31b1fbe9e03282161ee125cb7a4b2f53c8a8da63 

Diff: https://reviews.apache.org/r/35739/diff/


Testing
---


Thanks,

Steven Phillips

[jira] [Created] (DRILL-3339) Error message must be modified when PERCENT_RANK window functions are used without ORDER BY

[Abhishek Girish (JIRA)]

Abhishek Girish created DRILL-3339:
--

 Summary: Error message must be modified when PERCENT_RANK window 
functions are used without ORDER BY 
 Key: DRILL-3339
 URL: https://issues.apache.org/jira/browse/DRILL-3339
 Project: Apache Drill
  Issue Type: Bug
  Components: Query Planning & Optimization
Affects Versions: 1.1.0
Reporter: Abhishek Girish
Assignee: Sean Hsuan-Yi Chu
Priority: Minor


The following message is thrown when a query containing PERCENT_RANK window 
function fails:

{code:sql}
SELECT PERCENT_RANK() OVER (PARTITION BY ss.ss_store_sk) FROM store_sales ss 
LIMIT 20;
Error: PARSE ERROR: From line 1, column 28 to line 1, column 56: RANK or 
DENSE_RANK functions require ORDER BY clause in window specification
{code}

The error message must be modified to include PERCENT_RANK function



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (DRILL-3338) Error message must be modified when PERCENT_RANK window functions are used without ORDER BY

[Abhishek Girish (JIRA)]

Abhishek Girish created DRILL-3338:
--

 Summary: Error message must be modified when PERCENT_RANK window 
functions are used without ORDER BY 
 Key: DRILL-3338
 URL: https://issues.apache.org/jira/browse/DRILL-3338
 Project: Apache Drill
  Issue Type: Bug
  Components: Query Planning & Optimization
Affects Versions: 1.1.0
Reporter: Abhishek Girish
Assignee: Sean Hsuan-Yi Chu
Priority: Minor


The following message is thrown when a query containing PERCENT_RANK window 
function fails:

{code:sql}
SELECT PERCENT_RANK() OVER (PARTITION BY ss.ss_store_sk) FROM store_sales ss 
LIMIT 20;
Error: PARSE ERROR: From line 1, column 28 to line 1, column 56: RANK or 
DENSE_RANK functions require ORDER BY clause in window specification
{code}

The error message must be modified to include PERCENT_RANK function



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (DRILL-3337) Queries with Window Function DENSE_RANK fail with SchemaChangeException

[Abhishek Girish (JIRA)]

Abhishek Girish created DRILL-3337:
--

 Summary: Queries with Window Function DENSE_RANK fail with 
SchemaChangeException
 Key: DRILL-3337
 URL: https://issues.apache.org/jira/browse/DRILL-3337
 Project: Apache Drill
  Issue Type: Bug
  Components: Query Planning & Optimization
Affects Versions: 1.1.0
Reporter: Abhishek Girish
Assignee: Aman Sinha


Example queries which result in exceptions:

DENSE_RANK WF with ORDER BY 1 column and GROUP BY, ORDER BY on the main query
{code:sql}
SELECT DENSE_RANK() OVER (ORDER BY ss.ss_store_sk) FROM store_sales ss GROUP BY 
ss.ss_store_sk, ss.ss_net_paid_inc_tax ORDER BY 1 LIMIT 20;
Error: SYSTEM ERROR: org.apache.drill.exec.exception.SchemaChangeException: 
Failure while materializing expression. 
Error in expression at index 0.  Error: Missing function implementation: 
[dense_rank(INT-REQUIRED)].  Full expression: null.
Fragment 4:10
[Error Id: 4b9187db-e770-4e7f-afe4-0d4dfc045088 on abhi6.qa.lab:31010] 
(state=,code=0)
{code}

DENSE_RANK WF with PARTITION BY 2 columns and ORDER BY 2 column and GROUP BY, 
ORDER BY on the main query
{code:sql}
SELECT DENSE_RANK() OVER (PARTITION BY s.ss_store_sk, s.ss_customer_sk ORDER BY 
s.ss_store_sk, s.ss_customer_sk) FROM store_sales s GROUP BY s.ss_store_sk, 
s.ss_customer_sk, s.ss_quantity ORDER BY 1  LIMIT 20;
Error: SYSTEM ERROR: org.apache.drill.exec.exception.SchemaChangeException: 
Failure while materializing expression. 
Error in expression at index 0.  Error: Missing function implementation: 
[dense_rank(INT-REQUIRED)].  Full expression: null.
Fragment 5:22
[Error Id: 3ac6e4ce-5bb3-4058-b806-3d0becbbd0d1 on abhi6.qa.lab:31010] 
(state=,code=0)
{code}


*The queries execute fine on Postgres*



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[DISCUSS] Allowing the option to use github pull requests in place of reviewboard

[Jason Altekruse]

Hello Drill developers,

I am writing this message today to propose allowing the use of github pull
requests to perform reviews in place of the apache reviewboard instance.

Reviewboard has caused a number of headaches in the past few months, and I
think its time to evaluate the benefits of the apache infrastructure
relative to the actual cost of using it in practice.

For clarity of the discussion, we cannot use the complete github workflow.
Comitters will still need to use patch files, or check out the branch used
in the review request and push to apache master manually. I am not
advocating for using a merging strategy with git, just for using the github
web UI for reviews. I expect anyone generating a chain of commits as
described below to use the rebasing workflow we do today. Additionally devs
should only be breaking up work to make it easier to review, we will not be
reviewing branches that contain a bunch of useless WIP commits.

A few examples of problems I have experienced with reviewboard include:
corruption of patches when they are downloaded, the web interface showing
inconsistent content from the raw diff, and random rejection of patches
that are based directly on the head of apache master.

These are all serious blockers for getting code reviewed and integrated
into the master branch in a timely manner.

In addition to serious bugs in reviewboard, there are a number of
difficulties with the combination of our typical dev workflow and how
reviewboard works with patches. As we are still adding features to Drill,
we often have several weeks of work to submit in response to a JIRA or
series of related JIRAs. Sometimes this work can be broken up into
independent reviewable units, and other times it cannot. When a series of
changes requires a mixture of refactoring and additions, the process is
currently quite painful. Ether reviewers need to look through a giant messy
diff, or the submitters need to do a lot of extra work. This involves not
only organizing their work into a reviewable series of commits, but also
generating redundant squashed versions of the intermediate work to make
reviewboard happy.

For a relatively simple 3 part change, this involves creating 3 reviewboard
pages. The first will contain the first commit by itself. The second will
have the first commits patch as a parent patch with the next change in the
series uploaded as the core change to review. For the third change, a
squashed version of the first two commits must be generated to serve as a
parent patch and then the third changeset uploaded as the reviewable
change. Frequently a change to the first commit requires regenerating all
of these patches and uploading them to the individual review pages.

This gets even worse with larger chains of commits.

It would be great if all of our changes could be small units of work, but
very frequently we want to make sure we are ready to merge a complete
feature before starting the review process. We need to have a better way to
manage these large review units, as I do not see the possibility of
breaking up the work into smaller units as a likely solution. We still have
lots of features and system cleanup to work on.

For anyone unfamiliar, github pull requests are based on a branch you push
to your personal fork. They give space for a general discussion, as well as
allow commenting inline on the diff. They give a clear reference to each
commit in the branch, allowing reviewers to see each piece of work
individually as well as provide a "squashed" view to see the overall
differences.

For the sake of keeping the project history connected to JIRA, we can see
if there is enough automatic github integration or possibly upload patch
files to JIRA each time we update a pull request. As an side note, if we
don't need individual patches for reviewboard we could just put patch files
on JIRA that contain several commits. These are much easier to generate an
apply than a bunch of individual files for each change. This should prevent
JIRAs needing long lists of patches with names like
DRILL-3000-part1-version3.patch

[jira] [Created] (DRILL-3336) to_date(to_timestamp) with group-by in hbase/maprdb table fails with "java.lang.UnsupportedOperationException"

[Hao Zhu (JIRA)]

Hao Zhu created DRILL-3336:
--

 Summary: to_date(to_timestamp) with group-by in hbase/maprdb table 
fails with "java.lang.UnsupportedOperationException"
 Key: DRILL-3336
 URL: https://issues.apache.org/jira/browse/DRILL-3336
 Project: Apache Drill
  Issue Type: Bug
  Components: Execution - Flow, Functions - Drill
Affects Versions: 1.0.0
 Environment: 1.0 GA version
Reporter: Hao Zhu
Assignee: Chris Westin
Priority: Critical


1. Create a hbase/maprdb table in hbase shell:
{code}
create '/tables/esr52','cf'
put '/tables/esr52','1434998909','cf:c','abc'
> scan '/tables/esr52'
ROW  COLUMN+CELL
 1434998909  
column=cf:c, timestamp=1434998994785, value=abc
{code}

2. Below SQLs work fine in Drill:
{code}
>  select * from maprdb.esr52;
+--+---+
|   row_key|  cf   |
+--+---+
| [B@5bafd971  | {"c":"YWJj"}  |
+--+---+
1 row selected (0.095 seconds)

> select to_date(to_timestamp(cast(convert_from(esrtable.row_key,'UTF8') as 
> int))) from maprdb.esr52 esrtable;
+-+
|   EXPR$0|
+-+
| 2015-06-22  |
+-+
1 row selected (0.127 seconds)
{code}

3. However below SQL with group-by fails:
{code}
select to_date(to_timestamp(cast(convert_from(esrtable.row_key,'UTF8') as 
int))),count(*) from maprdb.esr52 esrtable 
group by to_date(to_timestamp(cast(convert_from(esrtable.row_key,'UTF8') as 
int)));

Error: SYSTEM ERROR: java.lang.UnsupportedOperationException: Failure finding 
function that runtime code generation expected.  Signature: 
compare_to_nulls_high( VAR16CHAR:OPTIONAL, VAR16CHAR:OPTIONAL ) returns 
INT:REQUIRED

Fragment 3:0

[Error Id: 26003311-d40e-4a95-9d3c-68793459ad6d on h1.poc.com:31010]

  (java.lang.UnsupportedOperationException) Failure finding function that 
runtime code generation expected.  Signature: compare_to_nulls_high( 
VAR16CHAR:OPTIONAL, VAR16CHAR:OPTIONAL ) returns INT:REQUIRED

org.apache.drill.exec.expr.fn.FunctionGenerationHelper.getFunctionExpression():109

org.apache.drill.exec.expr.fn.FunctionGenerationHelper.getOrderingComparator():62

org.apache.drill.exec.expr.fn.FunctionGenerationHelper.getOrderingComparatorNullsHigh():79

org.apache.drill.exec.physical.impl.common.ChainedHashTable.setupIsKeyMatchInternal():257

org.apache.drill.exec.physical.impl.common.ChainedHashTable.createAndSetupHashTable():206
org.apache.drill.exec.test.generated.HashAggregatorGen1.setup():273

org.apache.drill.exec.physical.impl.aggregate.HashAggBatch.createAggregatorInternal():240

org.apache.drill.exec.physical.impl.aggregate.HashAggBatch.createAggregator():163
org.apache.drill.exec.physical.impl.aggregate.HashAggBatch.buildSchema():110
org.apache.drill.exec.record.AbstractRecordBatch.next():127
org.apache.drill.exec.record.AbstractRecordBatch.next():105
org.apache.drill.exec.record.AbstractRecordBatch.next():95
org.apache.drill.exec.record.AbstractSingleRecordBatch.innerNext():51

org.apache.drill.exec.physical.impl.project.ProjectRecordBatch.innerNext():129
org.apache.drill.exec.record.AbstractRecordBatch.next():146
org.apache.drill.exec.physical.impl.BaseRootExec.next():83

org.apache.drill.exec.physical.impl.SingleSenderCreator$SingleSenderRootExec.innerNext():95
org.apache.drill.exec.physical.impl.BaseRootExec.next():73
org.apache.drill.exec.work.fragment.FragmentExecutor$1.run():259
org.apache.drill.exec.work.fragment.FragmentExecutor$1.run():253
java.security.AccessController.doPrivileged():-2
javax.security.auth.Subject.doAs():422
org.apache.hadoop.security.UserGroupInformation.doAs():1566
org.apache.drill.exec.work.fragment.FragmentExecutor.run():253
org.apache.drill.common.SelfCleaningRunnable.run():38
java.util.concurrent.ThreadPoolExecutor.runWorker():1142
java.util.concurrent.ThreadPoolExecutor$Worker.run():617
java.lang.Thread.run():745 (state=,code=0)
{code}

4. If we remove to_date, and only group-by to_timestamp, it works fine:
{code}
select to_timestamp(cast(convert_from(esrtable.row_key,'UTF8') as int)) from 
maprdb.esr52 esrtable;
++
| EXPR$0 |
++
| 2015-06-22 18:48:29.0  |
++
1 row selected (0.084 seconds)

select to_timestamp(cast(convert_from(esrtable.row_key,'UTF8') as 
int)),count(*) from maprdb.esr52 esrtable
group by to_timestamp(cast(convert_from(esrtable.row_key,'UTF8') as int));
++-+
| EXPR$0 | EXPR$1  |
++-+
| 2015-06-22 18:48:29.0  | 1   |
++-+
1 row selected (0.641 seconds)
{code}

Re: Review Request 34004: DRILL-1942: Improved memory allocator

[Chris Westin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34004/#review88805
---



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 173)


Yes, and this will disappear after all tests work.



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 187)


I didn't use an enum because it is possible to specify more than one flag 
at a time; they are then OR'ed together. I added javadoc for the flag that will 
survive the @Deprecated culling, but not for the one that is currently only 
used in other deprecated things.



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 302)


Not everyone uses the BufferManager yet. Since the constructors are called 
from the allocator, the allocator interface will have to be changed to pass 
that through. Filed DRILL-3331, added TODO.



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 312)


All that will change will be the deletion of the first two options, once 
everyone uses the BufferManager. The call can still fail in the same way if you 
try to ask for more space than is available.



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 322)


That's what DRILL-3331, filed above, will eventually do. But so far only 
the FragmentContext has one. (I didn't make those changes, Jason did that a 
while ago.)



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 399)


No. The comment was a little out-of-date, so I fixed it to indicate that 
the buffer must use the newly provided ledger going forward.

Once you get to the allocator, you'll see that the ledger provides two 
things: (1) a private interface for the buffer to communicate with the 
allocator (there are no public implementations of the BufferLedger interface, 
so it must be provided by the allocator), and (2) there are multiple 
implementations of the ledgers. When a buffer gets shared, it gets a different 
ledger implementation. DrillBuf's only care is to use whatever one the 
allocator supplies gives to it. The allocator chooses which one is required.



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 418)


The unfortunate lack of friend classes in Java makes it necessary to make 
this public. But the use of the BufferLedger interface means that no one else 
can call it, as there aren't any public implementations of that interface.



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 568)


I'll do it before final push.



exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java (line 1022)


At this point, it is only used by the allocator for debugging assertions. 
However, the RPC code seems to depend on isRootBuffer for some reason, so I 
suspect it will want to use this instead.



exec/java-exec/src/main/java/io/netty/buffer/FakeAllocator.java (line 28)


Apparently not. Removed.


- Chris Westin


On June 16, 2015, 6:07 p.m., Chris Westin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34004/
> ---
> 
> (Updated June 16, 2015, 6:07 p.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jason Altekruse.
> 
> 
> Bugs: DRILL-1942
> https://issues.apache.org/jira/browse/DRILL-1942
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> Rewritten direct memory allocator. Simplified interface, and use, along with 
> a means to support additional allocation policies in the future. There are 
> features in the allocator and in DrillBuf that make finding leaks easier, as 
> well as better enforcement of limits. New features include transfer of 
> buffers, and better slicing support.
> 
> This is a preliminary patch to get the review started because it touches a 
> lot of files (readers and record batches were made AutoCloseable in order to 
> cover cleanup). Subsequent reviews can use the differential view to just see 
> additional changes. The new allocator is in BaseAllocator.java (along with 
> derived classes RootAllocator and ChildAllocator); DrillBuf also has 
> significant changes. Most other changes in other files are just to use newer 
> interfaces, or to change cleanup() to close(), or to close subordinate 

Re: Review Request 34004: DRILL-1942: Improved memory allocator

[Chris Westin]

> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > common/src/main/java/org/apache/drill/common/config/NestedConfig.java, line 
> > 33
> > 
> >
> > why wrapped?

I'm assuming you mean "Why commented?" Because it's not referenced, and if left 
uncommented, it produces a warning.


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/codegen/templates/FixedValueVectors.java, line 204
> > 
> >
> > this isn't necessary given the target.clear() directly above.

That's a remnant from when I was trying to make empty buffers behave properly 
-- which is going to have to be a separate major project.


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java, line 
> > 142
> > 
> >
> > I generally think this is an excessive optimization throughout the code 
> > that you've added.

Opinion noted; what action do you want me to take? Do you want me to remove 
them?


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java, line 
> > 324
> > 
> >
> > If we fail to allocate the second vector here, shouldn't we clear the 
> > first (the data vector)

When I rebased, Hanifi had already made that fix.


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java, line 74
> > 
> >
> > With the deprecated items, can you please add javadocs that explain why 
> > deprecated and not removed?

They will be; I kept them for reference. I was just waiting until everything 
worked.


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java, line 
> > 78
> > 
> >
> > Not sure why this would be a system level setting.  It should be a user 
> > level setting.

Neither of the allocation policies we've discussed can be a user-level policy.
(1) Memory is divided equally across all fragments running on a drillbit.
This is the currently implemented policy. This doesn't take users into account. 
Because of that, we can't let one user have this setting, and another choose 
something else (if we had anything else).
(2) On each drillbit, memory is divided equally across all queries running on 
that drillbit (regardless of how many fragments that includes). Note that this 
also doesn't take users into account. And it wouldn't work to combine this with 
(1), unless we also had something higher level that split things across users 
in some way. For example, We could say that IF we were to support splitting 
memory on a drillbit evenly across all users who have anything running on that 
drillbit, then within those arenas, a user could select (1) or (2), but only 
within the user's individual arena.


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java, 
> > line 387
> > 
> >
> > Why is this necessary?  The debug above would print the exception which 
> > would include the cause.

I don't know. All I know was that I was getting an exception on the unprotected 
part. Are you sure that Exception.toString() includes the cause?


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java,
> >  line 59
> > 
> >
> > Yeah, can we fix this.  Wonder how much memory the tests leak this way?

Submitted DRILL-3335, added that to the TODO.


> On June 18, 2015, 9:40 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java,
> >  line 699
> > 
> >
> > Can you discuss the change here?  It seems like we should actually just 
> > fail if the length.value is negative.
> > 
> > Should probably also localize the length.value given multiple 
> > references in perf-sensitive code.

Apparently a negative value was expected some of the time. See the cases down 
below, which explicitly test for it. There are similar cases throughout the 
file. Yeah, it seems wrong to me, but I don't know the calling code. I don't 
know why it's like that, but I do know that the allocator started complaining 
when I added the checked for negative sizes. (Who knows what it did before???

[jira] [Created] (DRILL-3335) PrintingResultsListener's allocator should be a child of DrillClient's

[Chris Westin (JIRA)]

Chris Westin created DRILL-3335:
---

 Summary: PrintingResultsListener's allocator should be a child of 
DrillClient's
 Key: DRILL-3335
 URL: https://issues.apache.org/jira/browse/DRILL-3335
 Project: Apache Drill
  Issue Type: Bug
  Components: Client - CLI
Affects Versions: 1.1.0
Reporter: Chris Westin
Assignee: Daniel Barclay (Drill)


PrintingResultsListener's constructor creates a new top-level allocator. 
Instead, it should create ask DrillClient for a new child allocator that it 
should use. This could cause problems down the line, because we expect there to 
only be one top level allocator per node (or at least per JVM).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Review Request 35739: Patch for DRILL-3333

[Steven Phillips]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35739/
---

Review request for drill.


Bugs: DRILL-
https://issues.apache.org/jira/browse/DRILL-


Repository: drill-git


Description
---

DRILL-: Parquet writer auto-partitioning and partition pruning

Conflicts:

exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/WriterPrel.java

exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java


Diffs
-

  exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java 
6b6065f6b6c8469aa548acf194e0621b9f4ffea8 
  exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java 
797f3cb8c83a89821ee46ce0b093f81406fa6067 
  exec/java-exec/src/main/codegen/templates/NewValueFunctions.java PRE-CREATION 
  exec/java-exec/src/main/codegen/templates/RecordWriter.java 
c6325fd0a5c7d7cb5f3628df1ecf9c01c264ed52 
  exec/java-exec/src/main/codegen/templates/StringOutputRecordWriter.java 
f704cca0e4d62ca1435df84d9eb1b07b32ea8b39 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScan.java
 5c4ee4da9e0542244b0f71a520cea1c3a2d49a66 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/GroupScan.java 
2d16cd01b94ed8a5463c0e2fb896f019133f7f03 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java
 d5d64a722ed6d9b5d97158046e6838f07c0d5381 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/ParquetPartitionDescriptor.java
 PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
 d9b1354492454dcd2630c72f5dbc1c3badf958c7 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/ParquetPruneScanRule.java
 PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
 920b2848d8edb62667b880e81f5aee12b459d63a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/AutoPartitioner.java 
PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/NewValueFunction.java 
PRE-CREATION 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java
 a43a4a0f21bf11f29b6385e36db4d25003ffa98f 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetGroupScan.java
 cf39518b2a8b4564504a3971d1f89c268aee4b30 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
 621f05c4d50ecf83071a5df414be88e7471f0490 
  
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java
 31b1fbe9e03282161ee125cb7a4b2f53c8a8da63 

Diff: https://reviews.apache.org/r/35739/diff/


Testing
---


Thanks,

Steven Phillips

[jira] [Created] (DRILL-3334) "java.lang.IllegalStateException: Failure while reading vector.: raised when using dynamic schema in JSON

[Tugdual Grall (JIRA)]

Tugdual Grall created DRILL-3334:


 Summary:  "java.lang.IllegalStateException: Failure while reading 
vector.: raised when using dynamic schema in JSON
 Key: DRILL-3334
 URL: https://issues.apache.org/jira/browse/DRILL-3334
 Project: Apache Drill
  Issue Type: Bug
  Components: Execution - Data Types
Affects Versions: 1.0.0
 Environment: Single Node running on OSX
and
MapR Hadoop SandBox + Drill
Reporter: Tugdual Grall
Assignee: Daniel Barclay (Drill)


I have a simple data set based on 3 JSON documents:
 - 1 customer
 - 2 orders
(I have attached the document to the JIRA)

when I do the following query that is a join between order and customers I can 
raise some unexpected exception.

A working query:
{code}
SELECT customers.id, orders.total
FROM  dfs.ecommerce.`customers/*.json` customers,
 dfs.ecommerce.`orders/*.json` orders
WHERE customers.id = orders.cust_id
AND customers.country = 'FRANCE'
{code}

It works since orders.total is present in all orders

Now when I execute the following query (tax is not present in all document)
{code}
SELECT customers.id, orders.tax
FROM  dfs.ecommerce.`customers/*.json` customers,
 dfs.ecommerce.`orders/*.json` orders
WHERE customers.id = orders.cust_id
AND customers.country = 'FRANCE'
{code}

Thsi query raise the following exception:
{code}
org.apache.drill.common.exceptions.UserRemoteException: SYSTEM ERROR: 
java.lang.IllegalStateException: Failure while reading vector. Expected vector 
class of org.apache.drill.exec.vector.NullableIntVector but was holding vector 
class org.apache.drill.exec.vector.NullableBigIntVector. Fragment 0:0 [Error 
Id: a7ad300a-4446-41f3-8b1c-4bb7d1dbfb52 on maprdemo:31010]
{code}

If you cannot reproduce with tax, you can try with the field:
 orders.cool

or simply move the tax field from one document to the others.
(the field must be present in 1 document only)

It looks like Drill is losing the list of columns present globally.

Note: if I use a field that does not exist in any document it is working ( 
orders.this_is_crazy )

Note: if I use * instead of a projection this raise another exception:
{code}
org.apache.drill.common.exceptions.UserRemoteException: SYSTEM ERROR: 
org.apache.drill.exec.exception.SchemaChangeException: Hash join does not 
support schema changes Fragment 0:0 [Error Id: 
0b20d580-37a3-491a-9987-4d04fb6f2d43 on maprdemo:31010]
{code}





--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

[Sudheesh Katkam]

> On June 18, 2015, 10:44 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java,
> >  line 157
> > 
> >
> > Request: as part of this patch, since this method is doing more than 
> > logging, can you rename it to something more appropriate?
> 
> Jinfeng Ni wrote:
> Agree. 
> 
> If it's fine for you, I'll open a sepearte JIRA to make the change, since 
> it's a slightly different issue from the one that this JIRA is trying to 
> address.
> 
> Sudheesh Katkam wrote:
> IMO the change is too small for a JIRA, and this patch does refactoring. 
> Anyway, I filed 
> [DRILL-3332](https://issues.apache.org/jira/browse/DRILL-3332) for this.
> 
> Jinfeng Ni wrote:
> IMO, the change is more appropriate in DRILL-3319, which you filed days 
> ago, since they both handled the refactoring of logging issue in the code. 
> Mixed different things together would make it a bit harder for people to see 
> why the code is changed that way, especially months later when they checked 
> the descrpition of JIRA.

I was gonna make the change as part of that JIRA but I did not know the 
appropriate name. I agree that a patch should match the JIRA description. On 
the other hand, I hope we can get to resolve these small issues.


- Sudheesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35629/#review88449
---


On June 20, 2015, 12:49 a.m., Jinfeng Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> ---
> 
> (Updated June 20, 2015, 12:49 a.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query 
> planning component, such that the same SELECT statement will go through the 
> same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, 
> or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -
> 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  2866b8c 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
>  5e685c8 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java
>  5924c7e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java
>  3edcdb2 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
>  0a3393e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java
>  57cfde9 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java
>  473dbcb 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java
>  5e59e8c 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java
>  3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from 
> regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>

[jira] [Created] (DRILL-3333) Add support for auto-partitioning in parquet writer

[Steven Phillips (JIRA)]

Steven Phillips created DRILL-:
--

 Summary: Add support for auto-partitioning in parquet writer
 Key: DRILL-
 URL: https://issues.apache.org/jira/browse/DRILL-
 Project: Apache Drill
  Issue Type: Bug
Reporter: Steven Phillips


When a table is created with a partition by clause, the parquet writer will 
create separate files for the different partition values. The data will first 
be sorted by the partition keys, and the parquet writer will create new file 
when it encounters a new value for the partition columns.

When data is queried against the data that was created this way, partition 
pruning will work if the filter contains a partition column. And unlike 
directory based partitioning, no view is required, nor is it necessary to 
reference the dir* column names.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

[Jinfeng Ni]

> On June 18, 2015, 3:44 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java,
> >  line 157
> > 
> >
> > Request: as part of this patch, since this method is doing more than 
> > logging, can you rename it to something more appropriate?
> 
> Jinfeng Ni wrote:
> Agree. 
> 
> If it's fine for you, I'll open a sepearte JIRA to make the change, since 
> it's a slightly different issue from the one that this JIRA is trying to 
> address.
> 
> Sudheesh Katkam wrote:
> IMO the change is too small for a JIRA, and this patch does refactoring. 
> Anyway, I filed 
> [DRILL-3332](https://issues.apache.org/jira/browse/DRILL-3332) for this.

IMO, the change is more appropriate in DRILL-3319, which you filed days ago, 
since they both handled the refactoring of logging issue in the code. Mixed 
different things together would make it a bit harder for people to see why the 
code is changed that way, especially months later when they checked the 
descrpition of JIRA.


- Jinfeng


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35629/#review88449
---


On June 19, 2015, 5:49 p.m., Jinfeng Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> ---
> 
> (Updated June 19, 2015, 5:49 p.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query 
> planning component, such that the same SELECT statement will go through the 
> same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, 
> or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -
> 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  2866b8c 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
>  5e685c8 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java
>  5924c7e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java
>  3edcdb2 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
>  0a3393e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java
>  57cfde9 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java
>  473dbcb 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java
>  5e59e8c 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java
>  3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from 
> regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>

Review Request 35733: DRILL-3203: Add support for impersonation in Hive storage plugin

[Venki Korukanti]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35733/
---

Review request for drill.


Repository: drill-git


Description
---

Please see https://issues.apache.org/jira/browse/DRILL-3203 for details.


Diffs
-

  contrib/storage-hive/core/pom.xml 546fd7b 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveAuthorizationHelper.java
 PRE-CREATION 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScan.java
 8a2e498 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/DrillHiveTable.java
 d0ea143 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/DrillHiveViewTable.java
 1e02301 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
 83f250b 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/metastore/DrillHiveMetaStoreClient.java
 PRE-CREATION 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/metastore/DrillHiveMetaStoreClientWithUGI.java
 PRE-CREATION 
  
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/metastore/NonCloseableDrillHiveMSClientWithCaching.java
 PRE-CREATION 
  contrib/storage-hive/core/src/main/resources/bootstrap-storage-plugins.json 
5c7174e 
  
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/impersonation/hive/TestHiveImpersonationBase.java
 PRE-CREATION 
  
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/impersonation/hive/TestSqlStdBasedAuthorization.java
 PRE-CREATION 
  
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/impersonation/hive/TestStorageBasedHiveAuthorization.java
 PRE-CREATION 
  
contrib/storage-hive/core/src/test/java/org/apache/drill/exec/store/hive/HiveTestDataGenerator.java
 ea8d90f 
  contrib/storage-hive/core/src/test/resources/student.txt PRE-CREATION 
  contrib/storage-hive/core/src/test/resources/voter.txt PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java 
06f8088 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractSchema.java 
6afce1a 
  exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java a07f621 

Diff: https://reviews.apache.org/r/35733/diff/


Testing
---

Added unittests to cover both storage based and SQL standard based 
authorizations in Hive. More unittests in chained impersonation are coming in 
next patch.


Thanks,

Venki Korukanti

Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

[Sudheesh Katkam]

> On June 18, 2015, 10:44 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java,
> >  line 157
> > 
> >
> > Request: as part of this patch, since this method is doing more than 
> > logging, can you rename it to something more appropriate?
> 
> Jinfeng Ni wrote:
> Agree. 
> 
> If it's fine for you, I'll open a sepearte JIRA to make the change, since 
> it's a slightly different issue from the one that this JIRA is trying to 
> address.

IMO the change is too small for a JIRA, and this patch does refactoring. 
Anyway, I filed [DRILL-3332](https://issues.apache.org/jira/browse/DRILL-3332) 
for this.


- Sudheesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35629/#review88449
---


On June 20, 2015, 12:49 a.m., Jinfeng Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> ---
> 
> (Updated June 20, 2015, 12:49 a.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> ---
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query 
> planning component, such that the same SELECT statement will go through the 
> same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, 
> or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -
> 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java
>  2866b8c 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
>  5e685c8 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java
>  5924c7e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java
>  3edcdb2 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
>  0a3393e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java
>  57cfde9 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java
>  473dbcb 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java
>  5e59e8c 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java
>  3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from 
> regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>

[jira] [Created] (DRILL-3332) Rename DefaultSqlHandler#log(String, Prel)

[Sudheesh Katkam (JIRA)]

Sudheesh Katkam created DRILL-3332:
--

 Summary: Rename DefaultSqlHandler#log(String, Prel)
 Key: DRILL-3332
 URL: https://issues.apache.org/jira/browse/DRILL-3332
 Project: Apache Drill
  Issue Type: Bug
Reporter: Sudheesh Katkam
Assignee: Jinfeng Ni
Priority: Minor
 Fix For: 1.1.0


This method does more than logging; please rename it to something more 
appropriate. From [review board | https://reviews.apache.org/r/35629].



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 35673: DRILL-3326: Query with unsupported windows function containing "AS" blocks correct error message

[Sean Hsuan-Yi Chu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35673/
---

(Updated June 22, 2015, 5:10 p.m.)


Review request for drill and Aman Sinha.


Changes
---

revise comments


Bugs: DRILL-3326
https://issues.apache.org/jira/browse/DRILL-3326


Repository: drill-git


Description
---

DRILL-3326: When inspecting SELECT-LIST, UnsupportedOperatorsVisitor will dig 
into AS clause


Diffs (updated)
-

  
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
 544a838 
  exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java 
1c8b0db 

Diff: https://reviews.apache.org/r/35673/diff/


Testing
---

Unit, others are on the way


Thanks,

Sean Hsuan-Yi Chu

[jira] [Created] (DRILL-3331) Provide the BufferManager at DrillBuf construction time

[Chris Westin (JIRA)]

Chris Westin created DRILL-3331:
---

 Summary: Provide the BufferManager at DrillBuf construction time
 Key: DRILL-3331
 URL: https://issues.apache.org/jira/browse/DRILL-3331
 Project: Apache Drill
  Issue Type: Bug
  Components: Execution - Flow
Affects Versions: 1.1.0
Reporter: Chris Westin
Assignee: Chris Westin
Priority: Minor


DrillBuf.setBufferManager() allows for providing a buffer manager to handle 
reallocation and freeing by operators.

At this time, the BufferManager is not used by everything else yet. 
OperatorContextImpl has not yet been converted to use the BufferManager. Once 
it has been, then a DrillBuf either uses the BufferManager or it doesn't, and 
this argument can be surfaced on its constructors, and passed through 
(optionally null) from the allocator's buffer() call.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)