Review Request 32248: DRILL-2139: Star is not expanded correctly in "select distinct" query

2015-03-19 Thread Sean Hsuan-Yi Chu

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

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/HashAggBatch.java
 c29fbf2 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
 33d2c7a 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
 a23780e 
  exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java d2d97f8 
  
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 

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


Testing
---

Unit test passed.

QA tests are running.


Thanks,

Sean Hsuan-Yi Chu



Re: Review Request 32248: DRILL-2139: Star is not expanded correctly in "select distinct" query

2015-03-19 Thread Sean Hsuan-Yi Chu

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

(Updated March 19, 2015, 6:31 p.m.)


Review request for drill, Aman Sinha and Jinfeng Ni.


Changes
---

QA tests passed


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/HashAggBatch.java
 c29fbf2 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
 33d2c7a 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
 a23780e 
  exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java d2d97f8 
  
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 

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


Testing (updated)
---

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

2015-03-19 Thread Jinfeng Ni

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


A couple of comments after a quick look at the code. 

Will look more in detail later.


exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java


Can you show example where you will expect to see "*" in the aggregate 
expression? For distinct * over schemaless table, I assume the * will appear in 
the GroupBy keys, not in the aggregated expressions. For count(*) as the agg 
expression, planner will replace * with a constant. So, I could not see why it 
is necessary to check if there is star in the aggregated expression.



exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java


Have you consider the case where * comes from join?

select distinct * from dept, emp where ...

Seems the code you added will not work properly for such case.


- Jinfeng Ni


On March 19, 2015, 11:31 a.m., Sean Hsuan-Yi Chu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32248/
> ---
> 
> (Updated March 19, 2015, 11:31 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/HashAggBatch.java
>  c29fbf2 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
>  33d2c7a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
>  a23780e 
>   exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 
> d2d97f8 
>   
> 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 
> 
> 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

2015-03-19 Thread Sean Hsuan-Yi Chu


> On March 19, 2015, 6:35 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java, line 
> > 747
> > 
> >
> > Have you consider the case where * comes from join?
> > 
> > select distinct * from dept, emp where ...
> > 
> > Seems the code you added will not work properly for such case.

Hmmm~ I am not sure why that case could fail. 

As soon as Aggr sees '*' in the plan, it knows it should group-by respect to 
all the columns. This logic seems to hold for JOIN as well to me.

Besides, I tried some simple casesm such as:

String queryDistinct = String.format("select distinct * " +
  "from dfs.`%s` t1, dfs.`%s` t2 " +
  "where (t1.a1 = t2.a1);", root, root);
(t1 and t2 are simply .json)

DRILL gave the same result as Postgres.


> On March 19, 2015, 6:35 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java,
> >  line 72
> > 
> >
> > Can you show example where you will expect to see "*" in the aggregate 
> > expression? For distinct * over schemaless table, I assume the * will 
> > appear in the GroupBy keys, not in the aggregated expressions. For count(*) 
> > as the agg expression, planner will replace * with a constant. So, I could 
> > not see why it is necessary to check if there is star in the aggregated 
> > expression.

Oh... I was not aware of the planner's smart substitution of * in that case.

As of now, I agree with you. But let me think more about it. Thanks.


- Sean Hsuan-Yi


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


On March 19, 2015, 6:31 p.m., Sean Hsuan-Yi Chu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32248/
> ---
> 
> (Updated March 19, 2015, 6:31 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/HashAggBatch.java
>  c29fbf2 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
>  33d2c7a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
>  a23780e 
>   exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 
> d2d97f8 
>   
> 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 
> 
> 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

2015-03-23 Thread Sean Hsuan-Yi Chu

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

(Updated March 23, 2015, 6:43 p.m.)


Review request for drill, Aman Sinha and Jinfeng Ni.


Changes
---

new patch


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/HashAggBatch.java
 c29fbf2 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java
 33d2c7a 
  
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
 a23780e 
  exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java d2d97f8 
  
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

2015-05-26 Thread Sean Hsuan-Yi Chu

---
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

2015-05-27 Thread Jinfeng Ni

---
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


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


Add "final" where necessary in the code.



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java


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


Same comment as line 52.



exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java


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

2015-05-28 Thread Sean Hsuan-Yi Chu

---
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



Re: Review Request 32248: DRILL-2139: Star is not expanded correctly in "select distinct" query

2015-05-28 Thread Sean Hsuan-Yi Chu


> 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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

2015-05-28 Thread Sean Hsuan-Yi Chu


> On March 19, 2015, 6:35 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java,
> >  line 72
> > 
> >
> > Can you show example where you will expect to see "*" in the aggregate 
> > expression? For distinct * over schemaless table, I assume the * will 
> > appear in the GroupBy keys, not in the aggregated expressions. For count(*) 
> > as the agg expression, planner will replace * with a constant. So, I could 
> > not see why it is necessary to check if there is star in the aggregated 
> > expression.
> 
> Sean Hsuan-Yi Chu wrote:
> Oh... I was not aware of the planner's smart substitution of * in that 
> case.
> 
> As of now, I agree with you. But let me think more about it. Thanks.

This issue was outdated and got resolved in the latest patch


> On March 19, 2015, 6:35 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java, line 
> > 747
> > 
> >
> > Have you consider the case where * comes from join?
> > 
> > select distinct * from dept, emp where ...
> > 
> > Seems the code you added will not work properly for such case.
> 
> Sean Hsuan-Yi Chu wrote:
> Hmmm~ I am not sure why that case could fail. 
> 
> As soon as Aggr sees '*' in the plan, it knows it should group-by respect 
> to all the columns. This logic seems to hold for JOIN as well to me.
> 
> Besides, I tried some simple casesm such as:
> 
> String queryDistinct = String.format("select distinct * " +
>   "from dfs.`%s` t1, dfs.`%s` t2 " +
>   "where (t1.a1 = t2.a1);", root, root);
> (t1 and t2 are simply .json)
> 
> DRILL gave the same result as Postgres.

A test case was added


- Sean Hsuan-Yi


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


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

2015-07-24 Thread Sean Hsuan-Yi Chu

---
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

2015-09-30 Thread Sean Hsuan-Yi Chu

---
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

2015-09-30 Thread Jacques Nadeau

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



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java
 (line 32)


Can you add a Javadoc here?



exec/java-exec/src/test/java/org/apache/drill/TestDistinctStar.java (line 25)


Can you add a test for something that has a known schema select * distinct?


- Jacques Nadeau


On Oct. 1, 2015, 1:50 a.m., Sean Hsuan-Yi Chu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32248/
> ---
> 
> (Updated Oct. 1, 2015, 1:50 a.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, 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
>  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

2015-10-01 Thread Sean Hsuan-Yi Chu

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

(Updated Oct. 1, 2015, 3:18 p.m.)


Review request for drill, Aman Sinha, Jacques Nadeau, and Jinfeng Ni.


Changes
---

addressed 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
 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

2015-10-01 Thread Sean Hsuan-Yi Chu


> On Oct. 1, 2015, 2:11 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggregateUtils.java,
> >  line 32
> > 
> >
> > Can you add a Javadoc here?

Done


> On Oct. 1, 2015, 2:11 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/test/java/org/apache/drill/TestDistinctStar.java, line 25
> > 
> >
> > Can you add a test for something that has a known schema select * 
> > distinct?

add two; one for streamagg; the other for hash agg;


- Sean Hsuan-Yi


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


On Oct. 1, 2015, 3:18 p.m., Sean Hsuan-Yi Chu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32248/
> ---
> 
> (Updated Oct. 1, 2015, 3:18 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, 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
>  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
> 
>