You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Hanifi Gunes <ha...@gmail.com> on 2015/05/20 23:39:23 UTC
Re: Review Request 34499: DRILL-3032: repeated vectors should handle
late type && instantiate its children upon construction
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34499/
-----------------------------------------------------------
(Updated May 20, 2015, 9:39 p.m.)
Review request for drill and Mehant Baid.
Changes
-------
added dependency
Repository: drill-git
Description
-------
DRILL-3032: repeated vectors should handle late type && instantiate its children upon construction
MaterializedField.java
- remove unused imports
BaseRepeatedValueVector.java
- repeated types should not attempt to create a child vector of late type
RepeatedListVector.java
- pass entire field rather than type to recursively instantiate a list vector
The rest
- minor code refactoring
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java 64ba8611b36377084d3912f997ea428715ed2cf8
exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java e5f4be1e960462f27f5c9477a3225fb7767cfde0
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java bcf0793751443ffed5879d36e09dc97ac4f2591f
exec/java-exec/src/main/java/org/apache/drill/exec/vector/VectorDescriptor.java 9a29848cf88bcba9e8d9ef57eec08f28a0ba9b4f
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java b5de8b1e2081e13c36fe563002dad00341367b6e
Diff: https://reviews.apache.org/r/34499/diff/
Testing
-------
all
Thanks,
Hanifi Gunes
Re: Review Request 34499: DRILL-3032: repeated vectors should handle
late type && instantiate its children upon construction
Posted by Hanifi Gunes <ha...@gmail.com>.
> On May 21, 2015, 10:42 p.m., Mehant Baid wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java, line 145
> > <https://reviews.apache.org/r/34499/diff/1/?file=965701#file965701line145>
> >
> > In which case where we creating vector with Late type. Shouldn't we have materialized the field earlier?
in case one batch happens to be [] serialized field still carries along a late inner type.
- Hanifi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34499/#review84807
-----------------------------------------------------------
On May 22, 2015, 9:19 p.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34499/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 9:19 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-3032: repeated vectors should handle late type && instantiate its children upon construction
>
> MaterializedField.java
> - remove unused imports
>
> BaseRepeatedValueVector.java
> - repeated types should not attempt to create a child vector of late type
>
> RepeatedListVector.java
> - pass entire field rather than type to recursively instantiate a list vector
>
> The rest
> - minor code refactoring
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java 64ba8611b36377084d3912f997ea428715ed2cf8
> exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java e5f4be1e960462f27f5c9477a3225fb7767cfde0
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java bcf0793751443ffed5879d36e09dc97ac4f2591f
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/VectorDescriptor.java 9a29848cf88bcba9e8d9ef57eec08f28a0ba9b4f
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java b5de8b1e2081e13c36fe563002dad00341367b6e
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 75bbc13eb47b3b1dfe95b3a24f2ecfd4b72f845b
> exec/java-exec/src/test/resources/join/join-left-drill-3032.json PRE-CREATION
> exec/java-exec/src/test/resources/join/join-right-drill-3032.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34499/diff/
>
>
> Testing
> -------
>
> all
>
>
> Thanks,
>
> Hanifi Gunes
>
>
Re: Review Request 34499: DRILL-3032: repeated vectors should handle
late type && instantiate its children upon construction
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34499/#review84807
-----------------------------------------------------------
Can you add a unit test like the one mentioned in the JIRA report for this bug.
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java
<https://reviews.apache.org/r/34499/#comment136192>
In which case where we creating vector with Late type. Shouldn't we have materialized the field earlier?
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java
<https://reviews.apache.org/r/34499/#comment136195>
store children's size in a local variable instead of making multiple calls.
- Mehant Baid
On May 20, 2015, 9:39 p.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34499/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 9:39 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-3032: repeated vectors should handle late type && instantiate its children upon construction
>
> MaterializedField.java
> - remove unused imports
>
> BaseRepeatedValueVector.java
> - repeated types should not attempt to create a child vector of late type
>
> RepeatedListVector.java
> - pass entire field rather than type to recursively instantiate a list vector
>
> The rest
> - minor code refactoring
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java 64ba8611b36377084d3912f997ea428715ed2cf8
> exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java e5f4be1e960462f27f5c9477a3225fb7767cfde0
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java bcf0793751443ffed5879d36e09dc97ac4f2591f
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/VectorDescriptor.java 9a29848cf88bcba9e8d9ef57eec08f28a0ba9b4f
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java b5de8b1e2081e13c36fe563002dad00341367b6e
>
> Diff: https://reviews.apache.org/r/34499/diff/
>
>
> Testing
> -------
>
> all
>
>
> Thanks,
>
> Hanifi Gunes
>
>
Re: Review Request 34499: DRILL-3032: repeated vectors should handle
late type && instantiate its children upon construction
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34499/#review85012
-----------------------------------------------------------
Ship it!
Ship It!
- Mehant Baid
On May 22, 2015, 9:19 p.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34499/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 9:19 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-3032: repeated vectors should handle late type && instantiate its children upon construction
>
> MaterializedField.java
> - remove unused imports
>
> BaseRepeatedValueVector.java
> - repeated types should not attempt to create a child vector of late type
>
> RepeatedListVector.java
> - pass entire field rather than type to recursively instantiate a list vector
>
> The rest
> - minor code refactoring
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java 64ba8611b36377084d3912f997ea428715ed2cf8
> exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java e5f4be1e960462f27f5c9477a3225fb7767cfde0
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java bcf0793751443ffed5879d36e09dc97ac4f2591f
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/VectorDescriptor.java 9a29848cf88bcba9e8d9ef57eec08f28a0ba9b4f
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java b5de8b1e2081e13c36fe563002dad00341367b6e
> exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 75bbc13eb47b3b1dfe95b3a24f2ecfd4b72f845b
> exec/java-exec/src/test/resources/join/join-left-drill-3032.json PRE-CREATION
> exec/java-exec/src/test/resources/join/join-right-drill-3032.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34499/diff/
>
>
> Testing
> -------
>
> all
>
>
> Thanks,
>
> Hanifi Gunes
>
>
Re: Review Request 34499: DRILL-3032: repeated vectors should handle
late type && instantiate its children upon construction
Posted by Hanifi Gunes <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34499/
-----------------------------------------------------------
(Updated May 22, 2015, 9:19 p.m.)
Review request for drill and Mehant Baid.
Changes
-------
- added a test case
- stylistic changes by IDE
Repository: drill-git
Description
-------
DRILL-3032: repeated vectors should handle late type && instantiate its children upon construction
MaterializedField.java
- remove unused imports
BaseRepeatedValueVector.java
- repeated types should not attempt to create a child vector of late type
RepeatedListVector.java
- pass entire field rather than type to recursively instantiate a list vector
The rest
- minor code refactoring
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java 64ba8611b36377084d3912f997ea428715ed2cf8
exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java e5f4be1e960462f27f5c9477a3225fb7767cfde0
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseRepeatedValueVector.java bcf0793751443ffed5879d36e09dc97ac4f2591f
exec/java-exec/src/main/java/org/apache/drill/exec/vector/VectorDescriptor.java 9a29848cf88bcba9e8d9ef57eec08f28a0ba9b4f
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java b5de8b1e2081e13c36fe563002dad00341367b6e
exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java 75bbc13eb47b3b1dfe95b3a24f2ecfd4b72f845b
exec/java-exec/src/test/resources/join/join-left-drill-3032.json PRE-CREATION
exec/java-exec/src/test/resources/join/join-right-drill-3032.json PRE-CREATION
Diff: https://reviews.apache.org/r/34499/diff/
Testing
-------
all
Thanks,
Hanifi Gunes