[
https://issues.apache.org/jira/browse/DRILL-6585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16537732#comment-16537732
]
ASF GitHub Bot commented on DRILL-6585:
---------------------------------------
paul-rogers commented on a change in pull request #1367: DRILL-6585:
PartitionSender clones vectors, but shares field metdata
URL: https://github.com/apache/drill/pull/1367#discussion_r201170628
##########
File path: exec/vector/src/main/codegen/templates/BasicTypeHelper.java
##########
@@ -233,10 +227,54 @@ public static ValueVector getNewVector(String name,
BufferAllocator allocator, M
throw new UnsupportedOperationException(buildErrorMessage("get holder
reader implementation", type, mode));
}
- public static ValueVector getNewVector(MaterializedField field,
BufferAllocator allocator){
+ public static ValueVector getNewVector(String name, BufferAllocator
allocator, MajorType type, CallBack callBack) {
+ MaterializedField field = MaterializedField.create(name, type);
+ return getNewVector(field, allocator, callBack);
+ }
+
+ public static ValueVector getNewVector(MaterializedField field,
BufferAllocator allocator) {
return getNewVector(field, allocator, null);
}
- public static ValueVector getNewVector(MaterializedField field,
BufferAllocator allocator, CallBack callBack){
+
+ /**
+ * Create a new vector by cloning the metadata from an existing vector. Use
this
+ * variation when the metadata is already in use and so should <i>not</i> be
+ * attached to the new vector.
+ *
+ * @param field field metadata to be cloned for the new vector
+ * @param allocator allocator to use to allocate memory for the vector
+ * @param callBack callBack when the schema of the vector changes (not
typically
+ * used in modern Drill)
+ * @return the new vector with no memory allocated
+ * @see #getNewVector(MaterializedField, BufferAllocator, CallBack) for when
+ * the metadata need not be cloned
+ */
+
+ public static ValueVector getClonedVector(MaterializedField field,
BufferAllocator allocator, CallBack callBack) {
+ MaterializedField newField = MaterializedField.create(field.getName(),
field.getType());
Review comment:
The cloning is creating a new `MaterializedField`. Though you do point out a
good point: the `MajorType` includes subtypes for a union vector; those need to
be dropped here. I'll add code that rebuilds the `MajorType` from the
`MinorType` and `Mode`.
----------------------------------------------------------------
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
> PartitionSender clones vectors, but shares field metdata
> --------------------------------------------------------
>
> Key: DRILL-6585
> URL: https://issues.apache.org/jira/browse/DRILL-6585
> Project: Apache Drill
> Issue Type: Bug
> Affects Versions: 1.13.0
> Reporter: Paul Rogers
> Assignee: Paul Rogers
> Priority: Major
>
> See the discussion for [PR #1244 for
> DRILL-6373|https://github.com/apache/drill/pull/1244].
> The PartitionSender clones vectors. But, it does so by reusing the
> {{MaterializedField}} from the original vector. Though the original authors
> of {{MaterializedField}} apparently meant it to be immutable, later changes
> for maps and unions ended up changing it to add members.
> When cloning a map, we get the original map materialized field, then start
> doctoring it up as we add the cloned map members. This screws up the original
> map vector's metadata.
> The solution is to clone an empty version of the materialized field when
> creating a new vector.
> But, since much code creates vectors by giving a perfectly valid, unique
> materialized field, we want to add a new method for use by the ill-behaved
> uses, such as PartitionSender, that ask to create a new vector without
> cloning the materialized field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
