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/22 01:36:03 UTC
Review Request 34575: DRILL-3019: Extra column in Schema of
Recordbatch from scanning Values
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34575/
-----------------------------------------------------------
Review request for drill, Aman Sinha and Jinfeng Ni.
Bugs: DRILL-3019
https://issues.apache.org/jira/browse/DRILL-3019
Repository: drill-git
Description
-------
In JsonReader, update atLeastOneWrite to true if writeListDataIfTyped and writeMapDataIfTyped write a value
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java 095d8c6
exec/java-exec/src/test/java/org/apache/drill/TestInListConstructedByUnionAll.java PRE-CREATION
Diff: https://reviews.apache.org/r/34575/diff/
Testing
-------
Unit, TPCH done
waiting for functional...
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 34575: DRILL-3019: Extra column in Schema of
Recordbatch from scanning Values
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34575/#review85484
-----------------------------------------------------------
Ship it!
Minor comment below; otherwise looks good.
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java
<https://reviews.apache.org/r/34575/#comment137062>
The 'values' in the name is misleading since large IN lists are list of values; you test is specific to expressions, so you could call this testExprsInInList.
- Aman Sinha
On May 22, 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/34575/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 5:41 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3019
> https://issues.apache.org/jira/browse/DRILL-3019
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In JsonReader, update atLeastOneWrite to true if writeListDataIfTyped and writeMapDataIfTyped write a value
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java 095d8c6
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java 281a946
>
> Diff: https://reviews.apache.org/r/34575/diff/
>
>
> Testing
> -------
>
> Unit, TPCH done
>
> waiting for functional...
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 34575: DRILL-3019: Extra column in Schema of
Recordbatch from scanning Values
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34575/
-----------------------------------------------------------
(Updated May 28, 2015, 2:10 a.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Changes
-------
Addressed the comments
Bugs: DRILL-3019
https://issues.apache.org/jira/browse/DRILL-3019
Repository: drill-git
Description
-------
In JsonReader, update atLeastOneWrite to true if writeListDataIfTyped and writeMapDataIfTyped write a value
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java 095d8c6
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java 281a946
Diff: https://reviews.apache.org/r/34575/diff/
Testing
-------
Unit, TPCH done
waiting for functional...
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 34575: DRILL-3019: Extra column in Schema of
Recordbatch from scanning Values
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34575/
-----------------------------------------------------------
(Updated May 22, 2015, 5:41 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Bugs: DRILL-3019
https://issues.apache.org/jira/browse/DRILL-3019
Repository: drill-git
Description
-------
In JsonReader, update atLeastOneWrite to true if writeListDataIfTyped and writeMapDataIfTyped write a value
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java 095d8c6
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java 281a946
Diff: https://reviews.apache.org/r/34575/diff/
Testing
-------
Unit, TPCH done
waiting for functional...
Thanks,
Sean Hsuan-Yi Chu
Re: Review Request 34575: DRILL-3019: Extra column in Schema of
Recordbatch from scanning Values
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
> On May 22, 2015, 12:41 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java, line 436
> > <https://reviews.apache.org/r/34575/diff/1/?file=968424#file968424line436>
> >
> > The JIRA talks about performance degradation - is that as a result of this fix or regardless of this fix ?
The description in DIRA is out of date. I just updated the message there as follows:
"
To clarify, the degradation is a separate issue. Please refer to DRILL-3117.
This issue is meant to eliminate the extra column.
"
> On May 22, 2015, 12:41 a.m., Aman Sinha wrote:
> > exec/java-exec/src/test/java/org/apache/drill/TestInListConstructedByUnionAll.java, line 24
> > <https://reviews.apache.org/r/34575/diff/1/?file=968425#file968425line24>
> >
> > Do you need this test in a separate file ? why not add it to the existing TestLargeInClause ?
Make sense. Will update a patch
- Sean Hsuan-Yi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34575/#review84843
-----------------------------------------------------------
On May 21, 2015, 11:36 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34575/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:36 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3019
> https://issues.apache.org/jira/browse/DRILL-3019
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In JsonReader, update atLeastOneWrite to true if writeListDataIfTyped and writeMapDataIfTyped write a value
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java 095d8c6
> exec/java-exec/src/test/java/org/apache/drill/TestInListConstructedByUnionAll.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34575/diff/
>
>
> Testing
> -------
>
> Unit, TPCH done
>
> waiting for functional...
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 34575: DRILL-3019: Extra column in Schema of
Recordbatch from scanning Values
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34575/#review84843
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java
<https://reviews.apache.org/r/34575/#comment136253>
The JIRA talks about performance degradation - is that as a result of this fix or regardless of this fix ?
exec/java-exec/src/test/java/org/apache/drill/TestInListConstructedByUnionAll.java
<https://reviews.apache.org/r/34575/#comment136254>
Do you need this test in a separate file ? why not add it to the existing TestLargeInClause ?
- Aman Sinha
On May 21, 2015, 11:36 p.m., Sean Hsuan-Yi Chu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34575/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:36 p.m.)
>
>
> Review request for drill, Aman Sinha and Jinfeng Ni.
>
>
> Bugs: DRILL-3019
> https://issues.apache.org/jira/browse/DRILL-3019
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> In JsonReader, update atLeastOneWrite to true if writeListDataIfTyped and writeMapDataIfTyped write a value
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java 095d8c6
> exec/java-exec/src/test/java/org/apache/drill/TestInListConstructedByUnionAll.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34575/diff/
>
>
> Testing
> -------
>
> Unit, TPCH done
>
> waiting for functional...
>
>
> Thanks,
>
> Sean Hsuan-Yi Chu
>
>
Re: Review Request 34575: DRILL-3019: Extra column in Schema of
Recordbatch from scanning Values
Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34575/
-----------------------------------------------------------
(Updated May 21, 2015, 11:36 p.m.)
Review request for drill, Aman Sinha and Jinfeng Ni.
Bugs: DRILL-3019
https://issues.apache.org/jira/browse/DRILL-3019
Repository: drill-git
Description
-------
In JsonReader, update atLeastOneWrite to true if writeListDataIfTyped and writeMapDataIfTyped write a value
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java 095d8c6
exec/java-exec/src/test/java/org/apache/drill/TestInListConstructedByUnionAll.java PRE-CREATION
Diff: https://reviews.apache.org/r/34575/diff/
Testing (updated)
-------
Unit, TPCH done
waiting for functional...
Thanks,
Sean Hsuan-Yi Chu