You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Jason Altekruse <al...@gmail.com> on 2015/06/26 22:35:42 UTC
Review Request 35942: DRILL-1673 (reopened) : issue with flatten when
used with nested lists
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35942/
-----------------------------------------------------------
Review request for drill and Venki Korukanti.
Bugs: DRILL-1673
https://issues.apache.org/jira/browse/DRILL-1673
Repository: drill-git
Description
-------
There were two small issues in ValueVectors that caused this to fail. This patches fixes those issues and a few new flatten tests. To validate the flatten results a small reference implementation was added to generate baselines.
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java 8c61a60
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java f538399
exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java d4e7ed6
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlatten.java 6f5a303
exec/java-exec/src/test/resources/flatten/complex_transaction_example_data.json PRE-CREATION
exec/java-exec/src/test/resources/store/json/1673.json PRE-CREATION
Diff: https://reviews.apache.org/r/35942/diff/
Testing
-------
Unit tests run, regression in progress
Thanks,
Jason Altekruse
Re: Review Request 35942: DRILL-1673 (reopened) : issue with flatten
when used with nested lists
Posted by Jason Altekruse <al...@gmail.com>.
> On June 26, 2015, 9:29 p.m., Hanifi Gunes wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java, line 46
> > <https://reviews.apache.org/r/35942/diff/1/?file=993534#file993534line46>
> >
> > This seems to fix the symptom. The only case I can think of that would require this ternary statement here is when offsets vector has zero elements, which breaks the invariant of this class. What is causing offsets vector to come empty?
Just tried to remove this code and my tests I added still passed. Looks like some refactoring of the vectors removed the need for this workaround. I had written a partial fix for this a little while back but I didn't have thorough tests for it so I didn't post my patch. I will remove the ternary statment before running tests and merging.
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35942/#review89567
-----------------------------------------------------------
On June 26, 2015, 8:35 p.m., Jason Altekruse wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35942/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 8:35 p.m.)
>
>
> Review request for drill and Venki Korukanti.
>
>
> Bugs: DRILL-1673
> https://issues.apache.org/jira/browse/DRILL-1673
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> There were two small issues in ValueVectors that caused this to fail. This patches fixes those issues and a few new flatten tests. To validate the flatten results a small reference implementation was added to generate baselines.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java 8c61a60
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java f538399
> exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java d4e7ed6
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlatten.java 6f5a303
> exec/java-exec/src/test/resources/flatten/complex_transaction_example_data.json PRE-CREATION
> exec/java-exec/src/test/resources/store/json/1673.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35942/diff/
>
>
> Testing
> -------
>
> Unit tests run, regression in progress
>
>
> Thanks,
>
> Jason Altekruse
>
>
Re: Review Request 35942: DRILL-1673 (reopened) : issue with flatten
when used with nested lists
Posted by Hanifi Gunes <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35942/#review89567
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java (line 46)
<https://reviews.apache.org/r/35942/#comment142191>
This seems to fix the symptom. The only case I can think of that would require this ternary statement here is when offsets vector has zero elements, which breaks the invariant of this class. What is causing offsets vector to come empty?
- Hanifi Gunes
On June 26, 2015, 8:35 p.m., Jason Altekruse wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35942/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 8:35 p.m.)
>
>
> Review request for drill and Venki Korukanti.
>
>
> Bugs: DRILL-1673
> https://issues.apache.org/jira/browse/DRILL-1673
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> There were two small issues in ValueVectors that caused this to fail. This patches fixes those issues and a few new flatten tests. To validate the flatten results a small reference implementation was added to generate baselines.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java 8c61a60
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java f538399
> exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java d4e7ed6
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlatten.java 6f5a303
> exec/java-exec/src/test/resources/flatten/complex_transaction_example_data.json PRE-CREATION
> exec/java-exec/src/test/resources/store/json/1673.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35942/diff/
>
>
> Testing
> -------
>
> Unit tests run, regression in progress
>
>
> Thanks,
>
> Jason Altekruse
>
>
Re: Review Request 35942: DRILL-1673 (reopened) : issue with flatten
when used with nested lists
Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35942/#review89566
-----------------------------------------------------------
Ship it!
LGTM
- Venki Korukanti
On June 26, 2015, 1:35 p.m., Jason Altekruse wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35942/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 1:35 p.m.)
>
>
> Review request for drill and Venki Korukanti.
>
>
> Bugs: DRILL-1673
> https://issues.apache.org/jira/browse/DRILL-1673
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> There were two small issues in ValueVectors that caused this to fail. This patches fixes those issues and a few new flatten tests. To validate the flatten results a small reference implementation was added to generate baselines.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/EmptyValuePopulator.java 8c61a60
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java f538399
> exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java d4e7ed6
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/flatten/TestFlatten.java 6f5a303
> exec/java-exec/src/test/resources/flatten/complex_transaction_example_data.json PRE-CREATION
> exec/java-exec/src/test/resources/store/json/1673.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/35942/diff/
>
>
> Testing
> -------
>
> Unit tests run, regression in progress
>
>
> Thanks,
>
> Jason Altekruse
>
>