You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2018/07/12 00:54:54 UTC

[GitHub] vrozov commented on issue #1367: DRILL-6585: PartitionSender clones vectors, but shares field metdata

vrozov commented on issue #1367: DRILL-6585: PartitionSender clones vectors, but shares field metdata
URL: https://github.com/apache/drill/pull/1367#issuecomment-404355666
 
 
   @paul-rogers I am somewhat OK with the code changes granted that they fix the issue. My concerns are (in random order)
   
   - Usage of the `clone()` where copy constructor should be used (see [Copy Constructor versus Cloning](https://www.artima.com/intv/bloch13.html))
   - Improper implementation of `clone()` by  the `MaterializedField`
   - Unclear semantics when `MaterializedField` needs to be cloned
   - Should not by the time `InitializedBatch` is called, `MaterializedField` for incoming vectors be fully initialized? If they are, what causes changes in outgoing vectors `MaterializedField` and if they are not, how changes to `MaterializedField` in incoming vectors will be propagated to outgoing vectors `MaterializedField` once they refer to a deep copy? 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services