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_r201199782
##########
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) {
Review comment:
I perhaps misunderstood the original comment.
There are two ways that `getNewVector` is used:
* The caller builds up a `MaterializedField` and passes it to the new
vector. No sharing occurs. This is the normal case used by, as far as I can
tell, all but one place in the code.
* The `PartitionSenderTemplate` creates a new vector using the type of a
previous vector. Here, the `MaterializedField` is already in use and must be
cloned.
There seems no good reason to clone every `MaterializedField` since most
uses are correct. In fact, the new result set loader code rather counts on the
fact that the passed-in `MaterializeField` is the one that the vector uses.
We could possibly insert code into the `getNewVector()` that triggers an
assert if the `MaterializedField` already has children (or its minor type has
subtypes). That will catch field sharing if it occurs again.
Doing so might uncover other improper usages, so is a bit out of the scope
of this PR. Good exercise for someone who has access to the full pre-commit
test suite.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
