You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Sean Hsuan-Yi Chu <hs...@usc.edu> on 2015/03/15 22:48:05 UTC
Review Request 32089: DRILL-2311: Create table with same columns of
different case results in a java.lang.IllegalStateException
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/
-----------------------------------------------------------
Review request for drill and Aman Sinha.
Bugs: DRILL-2311
https://issues.apache.org/jira/browse/DRILL-2311
Repository: drill-git
Description
-------
In ProjectRecordBatch, ensure the output columns are unique.
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
Diff: https://reviews.apache.org/r/32089/diff/
Testing
-------
Unit tests
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
> On March 16, 2015, 7:58 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java, line 44
> > <https://reviews.apache.org/r/32089/diff/2/?file=896260#file896260line44>
> >
> > I understand these testcase should use "unOrdered()", in stead of "ordered(). But why do you change in this patch? Is it because you run into testcase failure with the original "ordered()", once you change ProjectRecordBatch? If so, then, we need understand why the testcase previously is successful with "ordered", and now fails with "ordered" with your code change in ProjectRecordBatch.
Oh...oh...
Actually, either worked just fine. I made the change simply because it is not theoretically correct.
- Sean Hsuan-Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76631
-----------------------------------------------------------
On March 16, 2015, 7:45 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 16, 2015, 7:45 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Aman Sinha <as...@maprtech.com>.
> On March 16, 2015, 7:58 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java, line 44
> > <https://reviews.apache.org/r/32089/diff/2/?file=896260#file896260line44>
> >
> > I understand these testcase should use "unOrdered()", in stead of "ordered(). But why do you change in this patch? Is it because you run into testcase failure with the original "ordered()", once you change ProjectRecordBatch? If so, then, we need understand why the testcase previously is successful with "ordered", and now fails with "ordered" with your code change in ProjectRecordBatch.
>
> Sean Hsuan-Yi Chu wrote:
> Oh...oh...
> Actually, either worked just fine. I made the change simply because it is not theoretically correct.
Agree with Jinfeng that changing the ordering as part of this patch could be misleading, so maybe leave the original tests as is and for the new ones you are using unOrdered anyways.
- Aman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76631
-----------------------------------------------------------
On March 16, 2015, 7:45 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 16, 2015, 7:45 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76631
-----------------------------------------------------------
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
<https://reviews.apache.org/r/32089/#comment124203>
I understand these testcase should use "unOrdered()", in stead of "ordered(). But why do you change in this patch? Is it because you run into testcase failure with the original "ordered()", once you change ProjectRecordBatch? If so, then, we need understand why the testcase previously is successful with "ordered", and now fails with "ordered" with your code change in ProjectRecordBatch.
- Jinfeng Ni
On March 16, 2015, 12:45 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 16, 2015, 12:45 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
> On March 16, 2015, 9:54 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java, line 369
> > <https://reviews.apache.org/r/32089/diff/2/?file=896259#file896259line369>
> >
> > Can you add some comments here to indicate which columns would be considered here and about uniquely naming the columns.
Updated!!!
- Sean Hsuan-Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76653
-----------------------------------------------------------
On March 16, 2015, 7:45 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 16, 2015, 7:45 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76653
-----------------------------------------------------------
Ship it!
Couple of minor comments below.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
<https://reviews.apache.org/r/32089/#comment124253>
Can you add some comments here to indicate which columns would be considered here and about uniquely naming the columns.
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
<https://reviews.apache.org/r/32089/#comment124259>
maybe shorten the name :)
- Aman Sinha
On March 16, 2015, 7:45 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 16, 2015, 7:45 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76751
-----------------------------------------------------------
Ship it!
Ship It!
- Aman Sinha
On March 17, 2015, 5:41 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 17, 2015, 5:41 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/
-----------------------------------------------------------
(Updated March 17, 2015, 5:41 p.m.)
Review request for drill and Aman Sinha.
Changes
-------
diff updated
Bugs: DRILL-2311
https://issues.apache.org/jira/browse/DRILL-2311
Repository: drill-git
Description
-------
In ProjectRecordBatch, ensure the output columns are unique.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
Diff: https://reviews.apache.org/r/32089/diff/
Testing
-------
Unit tests, all QA tests
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
> On March 17, 2015, 4:34 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java, line 369
> > <https://reviews.apache.org/r/32089/diff/3/?file=897157#file897157line369>
> >
> > comment looks good..but could you move it below the else statement ... that's the general convention.
Done!!! Please see the updated diff.
Will upload one to Jira after this review passes through.
- Sean Hsuan-Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76709
-----------------------------------------------------------
On March 17, 2015, 1:57 a.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 17, 2015, 1:57 a.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/#review76709
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
<https://reviews.apache.org/r/32089/#comment124340>
comment looks good..but could you move it below the else statement ... that's the general convention.
- Aman Sinha
On March 17, 2015, 1:57 a.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32089/
> -----------------------------------------------------------
>
> (Updated March 17, 2015, 1:57 a.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Bugs: DRILL-2311
> https://issues.apache.org/jira/browse/DRILL-2311
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In ProjectRecordBatch, ensure the output columns are unique.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
>
> Diff: https://reviews.apache.org/r/32089/diff/
>
>
> Testing
> -------
>
> Unit tests, all QA tests
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/
-----------------------------------------------------------
(Updated March 17, 2015, 1:57 a.m.)
Review request for drill and Aman Sinha.
Changes
-------
Comments were added !
Bugs: DRILL-2311
https://issues.apache.org/jira/browse/DRILL-2311
Repository: drill-git
Description
-------
In ProjectRecordBatch, ensure the output columns are unique.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
Diff: https://reviews.apache.org/r/32089/diff/
Testing
-------
Unit tests, all QA tests
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 32089: DRILL-2311: Create table with same columns
of different case results in a java.lang.IllegalStateException
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32089/
-----------------------------------------------------------
(Updated March 16, 2015, 7:45 p.m.)
Review request for drill and Aman Sinha.
Bugs: DRILL-2311
https://issues.apache.org/jira/browse/DRILL-2311
Repository: drill-git
Description
-------
In ProjectRecordBatch, ensure the output columns are unique.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 8f7812f
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 8ae0ae4
Diff: https://reviews.apache.org/r/32089/diff/
Testing (updated)
-------
Unit tests, all QA tests
Thanks,
Sean Hsuan-Yi Chu