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/03/20 23:46:10 UTC
Review Request 32335: DRILL-2366: accurately materialize schema path
for complex types + test cases
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32335/
-----------------------------------------------------------
Review request for drill and Mehant Baid.
Repository: drill-git
Description
-------
The changes cover
- Fix schema path materialization for complex types.
- Add unit tests
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java 3bc7d11b9d7bf7f3159041ac2235333519d17931
exec/java-exec/src/test/java/org/apache/drill/exec/expr/TestSchemaPathMaterialization.java PRE-CREATION
exec/java-exec/src/test/resources/complex/json/repeated_list.json PRE-CREATION
exec/java-exec/src/test/resources/complex/json/repeated_list_map.json PRE-CREATION
Diff: https://reviews.apache.org/r/32335/diff/
Testing
-------
unit + regression
Thanks,
Hanifi Gunes
Re: Review Request 32335: DRILL-2366: accurately materialize schema
path for complex types + test cases
Posted by Hanifi Gunes <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32335/
-----------------------------------------------------------
(Updated March 24, 2015, 11:18 p.m.)
Review request for drill and Mehant Baid.
Changes
-------
Change list:
- Refactored the patch so that parts relevant to RepeatedMap lives under RepeatedMap.
- Fixed an issue in VectorWrapper implementations regarding child vector look-up to handle repeated map as well.
Testing: unit + regression.
Repository: drill-git
Description
-------
The changes cover
- Fix schema path materialization for complex types.
- Add unit tests
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/record/HyperVectorWrapper.java 9dbb583cc85d2ba20a99fa82a5484e314b981f62
exec/java-exec/src/main/java/org/apache/drill/exec/record/SimpleVectorWrapper.java 5bd3e41d8c51bec55e109e6245c385ba5089cf26
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java 3bc7d11b9d7bf7f3159041ac2235333519d17931
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java b99546253ffd8fe0eb56dc95ec76f5b4bb16a1dc
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java c7e7cbab12f56c0ff571733e61263c380891d5b5
exec/java-exec/src/test/java/org/apache/drill/exec/expr/TestSchemaPathMaterialization.java PRE-CREATION
exec/java-exec/src/test/resources/complex/json/repeated_list.json PRE-CREATION
exec/java-exec/src/test/resources/complex/json/repeated_list_map.json PRE-CREATION
Diff: https://reviews.apache.org/r/32335/diff/
Testing
-------
unit + regression
Thanks,
Hanifi Gunes
Re: Review Request 32335: DRILL-2366: accurately materialize schema
path for complex types + test cases
Posted by Hanifi Gunes <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32335/#review77598
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java
<https://reviews.apache.org/r/32335/#comment125727>
This code block handles repeated map(RM) alone. One cannot do multi-level numbered access to an RM. As an example RM[0][0] would return null. Let me refactor this & decorate with comments for clarity.
- Hanifi Gunes
On March 20, 2015, 10:46 p.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32335/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 10:46 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> The changes cover
> - Fix schema path materialization for complex types.
> - Add unit tests
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java 3bc7d11b9d7bf7f3159041ac2235333519d17931
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/TestSchemaPathMaterialization.java PRE-CREATION
> exec/java-exec/src/test/resources/complex/json/repeated_list.json PRE-CREATION
> exec/java-exec/src/test/resources/complex/json/repeated_list_map.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32335/diff/
>
>
> Testing
> -------
>
> unit + regression
>
>
> Thanks,
>
> Hanifi Gunes
>
>
Re: Review Request 32335: DRILL-2366: accurately materialize schema
path for complex types + test cases
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32335/#review77515
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java
<https://reviews.apache.org/r/32335/#comment125608>
the two if conditions are the same? we should combine them.
Could you also provide little more detail as to what is happening here and why we need this?
- Mehant Baid
On March 20, 2015, 10:46 p.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32335/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 10:46 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> The changes cover
> - Fix schema path materialization for complex types.
> - Add unit tests
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java 3bc7d11b9d7bf7f3159041ac2235333519d17931
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/TestSchemaPathMaterialization.java PRE-CREATION
> exec/java-exec/src/test/resources/complex/json/repeated_list.json PRE-CREATION
> exec/java-exec/src/test/resources/complex/json/repeated_list_map.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32335/diff/
>
>
> Testing
> -------
>
> unit + regression
>
>
> Thanks,
>
> Hanifi Gunes
>
>
Re: Review Request 32335: DRILL-2366: accurately materialize schema
path for complex types + test cases
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32335/#review77594
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java
<https://reviews.apache.org/r/32335/#comment125719>
I see that the seg is being reassigned in the first if condition.
- Mehant Baid
On March 20, 2015, 10:46 p.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32335/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 10:46 p.m.)
>
>
> Review request for drill and Mehant Baid.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> The changes cover
> - Fix schema path materialization for complex types.
> - Add unit tests
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractContainerVector.java 3bc7d11b9d7bf7f3159041ac2235333519d17931
> exec/java-exec/src/test/java/org/apache/drill/exec/expr/TestSchemaPathMaterialization.java PRE-CREATION
> exec/java-exec/src/test/resources/complex/json/repeated_list.json PRE-CREATION
> exec/java-exec/src/test/resources/complex/json/repeated_list_map.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32335/diff/
>
>
> Testing
> -------
>
> unit + regression
>
>
> Thanks,
>
> Hanifi Gunes
>
>