Re: Review Request 34839: DRILL-3155: Part 2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34839/#review86325 --- Ship it! Ship It! - Hanifi Gunes On June 2, 2015, 9:49 p.m., Mehant Baid wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34839/ --- (Updated June 2, 2015, 9:49 p.m.) Review request for drill and Hanifi Gunes. Repository: drill-git Description --- While allocating memory for composite vectors if one of the allocation fails we need to release all the allocated memory upto that point. Diffs - exec/java-exec/src/main/codegen/templates/NullableValueVectors.java 90ec6be exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java 7b2b78d exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b3389e2 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java 3c01939 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java a97847b Diff: https://reviews.apache.org/r/34839/diff/ Testing --- Thanks, Mehant Baid
Re: Review Request 34839: DRILL-3155: Part 2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34839/ --- (Updated June 2, 2015, 9:49 p.m.) Review request for drill and Hanifi Gunes. Changes --- Addressed review comments Repository: drill-git Description --- While allocating memory for composite vectors if one of the allocation fails we need to release all the allocated memory upto that point. Diffs (updated) - exec/java-exec/src/main/codegen/templates/NullableValueVectors.java 90ec6be exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java 7b2b78d exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b3389e2 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java 3c01939 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java a97847b Diff: https://reviews.apache.org/r/34839/diff/ Testing --- Thanks, Mehant Baid
Re: Review Request 34839: DRILL-3155: Part 2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34839/ --- (Updated June 1, 2015, 8:54 p.m.) Review request for drill and Hanifi Gunes. Repository: drill-git Description --- While allocating memory for composite vectors if one of the allocation fails we need to release all the allocated memory upto that point. Diffs (updated) - exec/java-exec/src/main/codegen/templates/NullableValueVectors.java 90ec6be exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java 7b2b78d exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b3389e2 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java 3c01939 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java a97847b Diff: https://reviews.apache.org/r/34839/diff/ Testing --- Thanks, Mehant Baid
Re: Review Request 34839: DRILL-3155: Part 2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34839/#review86083 --- Almost there. exec/java-exec/src/main/codegen/templates/NullableValueVectors.java https://reviews.apache.org/r/34839/#comment137998 This applies to all vectors: we should be consistent across allocateNew/Safe in when to trigger clear. I would recommend to clear iff allocation fails. All other cases should be handled by consumers. exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java https://reviews.apache.org/r/34839/#comment137978 We should execute this after both allocations are completed. exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java https://reviews.apache.org/r/34839/#comment138001 We should move this allocation out of try-finally. exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java https://reviews.apache.org/r/34839/#comment137982 Minor cosmetic issue: perhaps we should consider returning after finally since we declare the variable before try. - Hanifi Gunes On June 1, 2015, 8:54 p.m., Mehant Baid wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34839/ --- (Updated June 1, 2015, 8:54 p.m.) Review request for drill and Hanifi Gunes. Repository: drill-git Description --- While allocating memory for composite vectors if one of the allocation fails we need to release all the allocated memory upto that point. Diffs - exec/java-exec/src/main/codegen/templates/NullableValueVectors.java 90ec6be exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java 7b2b78d exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b3389e2 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java 3c01939 exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java a97847b Diff: https://reviews.apache.org/r/34839/diff/ Testing --- Thanks, Mehant Baid
