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/05/26 20:28:35 UTC
Re: Review Request 32248: DRILL-2139: Star is not expanded correctly
in "select distinct" query
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32248/
-----------------------------------------------------------
(Updated May 26, 2015, 6:28 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
new PAtch to address * with prefix (i.e., Tx || *)
Bugs: DRILL-2139
https://issues.apache.org/jira/browse/DRILL-2139
Repository: drill-git
Description
-------
Expand * at the run time
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java e1b5909
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java b252971
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 8871a5f
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 75bbc13
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java 3786bfd
exec/java-exec/src/test/resources/store/text/data/repeatedRows.json PRE-CREATION
exec/java-exec/src/test/resources/testframework/testExampleQueries/testSelectDistinctByStreamAgg.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testExampleQueries/testSelectDistinctOverJoin.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testHashAggr/testSelectDistinctByHashAgg.tsv PRE-CREATION
Diff: https://reviews.apache.org/r/32248/diff/
Testing
-------
Unit and all QA tests passed.
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 32248: DRILL-2139: Star is not expanded correctly
in "select distinct" query
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
> On May 27, 2015, 11:16 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java, line 40
> > <https://reviews.apache.org/r/32248/diff/3/?file=971867#file971867line40>
> >
> > Is it always true that getExpr will return SchemaPath in all cases? What if I have group by some expression?
Yes, it will always be a SchemaPath. In agg, we are not supposed to do that kind of calculation (which should have been done in the project)
For example, a query select distinct *, a + 3 ...
The expression in the distinct (i.e., a + 3) will be processed at a Project before reaching the agg operator.
Also, I added one more test case for this concern.
> On May 27, 2015, 11:16 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java, line 48
> > <https://reviews.apache.org/r/32248/diff/3/?file=971867#file971867line48>
> >
> > Add "final" where necessary in the code.
Done!
> On May 27, 2015, 11:16 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java, line 52
> > <https://reviews.apache.org/r/32248/diff/3/?file=971867#file971867line52>
> >
> > If you put "incomingSchema.getColumn(indexCol).getPath()" into a variable, you do not have to use this long expression mutiple times.
Used the claimed variable
> On May 27, 2015, 11:16 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java, line 61
> > <https://reviews.apache.org/r/32248/diff/3/?file=971867#file971867line61>
> >
> > Same comment as line 52.
Used the claimed variable
> On May 27, 2015, 11:16 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java, line 1005
> > <https://reviews.apache.org/r/32248/diff/3/?file=971871#file971871line1005>
> >
> > Will it work for select distinct *, colA, colB, etc?
> >
> > I'm wondering if we need the similar logic as the one in ProjectRecordBatch to handle such cases.
On top of Scan, there is a project which gives '*' a proper prefix and appends a postfix number to ensure the uniqueness of the column names.
Thus, we do not need to duplicate that logic at agg again.
Also, test cases were added for this concern
- Sean Hsuan-Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32248/#review85455
-----------------------------------------------------------
On May 28, 2015, 10:38 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32248/
> -----------------------------------------------------------
>
> (Updated May 28, 2015, 10:38 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-2139
> https://issues.apache.org/jira/browse/DRILL-2139
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Expand * at the run time
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java e1b5909
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java b252971
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 8871a5f
> exec/java-exec/src/test/java/org/apache/drill/TestDistinctStar.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java d80e752
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java 3786bfd
> exec/java-exec/src/test/resources/store/text/data/repeatedRows.json PRE-CREATION
> exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinct.tsv PRE-CREATION
> exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctExpression.tsv PRE-CREATION
> exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctOverJoin.tsv PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32248/diff/
>
>
> Testing
> -------
>
> Unit and all QA tests passed.
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32248: DRILL-2139: Star is not expanded correctly
in "select distinct" query
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32248/#review85455
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java
<https://reviews.apache.org/r/32248/#comment137027>
Is it always true that getExpr will return SchemaPath in all cases? What if I have group by some expression?
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java
<https://reviews.apache.org/r/32248/#comment137035>
Add "final" where necessary in the code.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java
<https://reviews.apache.org/r/32248/#comment137034>
If you put "incomingSchema.getColumn(indexCol).getPath()" into a variable, you do not have to use this long expression mutiple times.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java
<https://reviews.apache.org/r/32248/#comment137036>
Same comment as line 52.
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java
<https://reviews.apache.org/r/32248/#comment137037>
Will it work for select distinct *, colA, colB, etc?
I'm wondering if we need the similar logic as the one in ProjectRecordBatch to handle such cases.
- Jinfeng Ni
On May 26, 2015, 11:28 a.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32248/
> -----------------------------------------------------------
>
> (Updated May 26, 2015, 11:28 a.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-2139
> https://issues.apache.org/jira/browse/DRILL-2139
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Expand * at the run time
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java e1b5909
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java b252971
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 8871a5f
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 75bbc13
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java 3786bfd
> exec/java-exec/src/test/resources/store/text/data/repeatedRows.json PRE-CREATION
> exec/java-exec/src/test/resources/testframework/testExampleQueries/testSelectDistinctByStreamAgg.tsv PRE-CREATION
> exec/java-exec/src/test/resources/testframework/testExampleQueries/testSelectDistinctOverJoin.tsv PRE-CREATION
> exec/java-exec/src/test/resources/testframework/testHashAggr/testSelectDistinctByHashAgg.tsv PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32248/diff/
>
>
> Testing
> -------
>
> Unit and all QA tests passed.
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 32248: DRILL-2139: Star is not expanded correctly
in "select distinct" query
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32248/
-----------------------------------------------------------
(Updated Oct. 1, 2015, 1:49 a.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
rebase
Bugs: DRILL-2139
https://issues.apache.org/jira/browse/DRILL-2139
Repository: drill-git
Description
-------
Expand * at the run time
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java a033a8e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java 2ab1e66
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 49a64cf
exec/java-exec/src/test/java/org/apache/drill/TestDistinctStar.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java 3786bfd
exec/java-exec/src/test/resources/store/text/data/repeatedRows.json PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinct.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctExpression.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctOverJoin.tsv PRE-CREATION
Diff: https://reviews.apache.org/r/32248/diff/
Testing
-------
Unit and all QA tests passed.
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 32248: DRILL-2139: Star is not expanded correctly
in "select distinct" query
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32248/
-----------------------------------------------------------
(Updated July 24, 2015, 3:56 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
rebase
Bugs: DRILL-2139
https://issues.apache.org/jira/browse/DRILL-2139
Repository: drill-git
Description
-------
Expand * at the run time
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java a033a8e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java 5a26134
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 4bb1572
exec/java-exec/src/test/java/org/apache/drill/TestDistinctStar.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java 3786bfd
exec/java-exec/src/test/resources/store/text/data/repeatedRows.json PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinct.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctExpression.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctOverJoin.tsv PRE-CREATION
Diff: https://reviews.apache.org/r/32248/diff/
Testing
-------
Unit and all QA tests passed.
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 32248: DRILL-2139: Star is not expanded correctly
in "select distinct" query
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32248/
-----------------------------------------------------------
(Updated May 28, 2015, 10:38 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
new patch to address the comments
Bugs: DRILL-2139
https://issues.apache.org/jira/browse/DRILL-2139
Repository: drill-git
Description
-------
Expand * at the run time
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java e1b5909
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java b252971
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 8871a5f
exec/java-exec/src/test/java/org/apache/drill/TestDistinctStar.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java d80e752
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestHashAggr.java 3786bfd
exec/java-exec/src/test/resources/store/text/data/repeatedRows.json PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinct.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctExpression.tsv PRE-CREATION
exec/java-exec/src/test/resources/testframework/testDistinctStar/testSelectDistinctOverJoin.tsv PRE-CREATION
Diff: https://reviews.apache.org/r/32248/diff/
Testing
-------
Unit and all QA tests passed.
Thanks,
Sean Hsuan-Yi Chu