[GitHub] vrozov commented on issue #1244: DRILL-6373: Refactor Result Set Loader for Union, List support

2018-07-21 Thread GitBox
vrozov commented on issue #1244: DRILL-6373: Refactor Result Set Loader for 
Union, List support
URL: https://github.com/apache/drill/pull/1244#issuecomment-406811356
 
 
   @paul-rogers The change to `NullableVarCharVector` that was part of your PR 
exposed a bug that is part of the existing code. Please see 
`AbstractMapVector.java:48`. The problem is that a newly created map vector 
references a newly created deep copy(clone) of the passed `MaterializedField` 
while newly created child vectors of the map vector references children of 
**another** instance of the `MaterializeField`. If that **other** instance is 
completely immutable it would not cause any problem (except that now there is 
no reason to create deep copy), but with your changes there was an attempt to 
mutate from different threads the **other** instance as that **other** instance 
is used to create multiple outgoing vectors. I hope that this explains what I 
mean when referring to **existing** bug.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] vrozov commented on issue #1244: DRILL-6373: Refactor Result Set Loader for Union, List support

2018-07-20 Thread GitBox
vrozov commented on issue #1244: DRILL-6373: Refactor Result Set Loader for 
Union, List support
URL: https://github.com/apache/drill/pull/1244#issuecomment-406668760
 
 
   @paul-rogers I disagree that the bug was introduced in the prior changes to 
this PR. The bug is already part of `AbstractMapVector` that clones 
MaterializedField but modifies children of the original `MaterializedField` and 
not the clone. In combination with changes to `NullableVarCharVector` it 
triggered functional test failures.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] vrozov commented on issue #1244: DRILL-6373: Refactor Result Set Loader for Union, List support

2018-07-19 Thread GitBox
vrozov commented on issue #1244: DRILL-6373: Refactor Result Set Loader for 
Union, List support
URL: https://github.com/apache/drill/pull/1244#issuecomment-406364011
 
 
   @paul-rogers Did you try to pull changes from PR #1383?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services