Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/840#discussion_r124938564
--- Diff: exec/vector/src/main/codegen/templates/NullableValueVectors.java
---
@@ -674,6 +764,14 @@ public void reset(){
setCount = 0;
<#if type.major = "VarLen">lastSet = -1;</#if>
}
+
+ @Override
+ public void exchange(ValueVector.Mutator other) {
--- End diff --
Yes, indeed I am trusting the caller, in the same way we trust the caller
to free a vector's buffer at the right time, to call `setSafe()` instead of
`set()`, to write to the vector only once, and so on. In short, if someone does
try to use `exchange()` with a vector owned by another allocator, the allocator
is likely to complain about memory leaks.
An alternative is to always use transfer pairs, though that adds quite a
bit of complexity for this one use case.
This method has a very specific usage: an operator writes to a vector,
discovers that the vector overflows, and must swap buffers between two sets of
vectors. This swapping is necessary because downstream operators depend on a
vector instance, as does the writer, so it is necessary to swap buffers into
and out of these fixed set of value vectors. Rather confusing, but the most
performant solution given how vectors work today.
For this particular method, confusion is understandable. This is an
`exchange()` on the `Mutator` class, swapping the state that nullable vector
mutators maintain. It is called from a vector `exchange()` method earlier in
this file. Added a comment to help future readers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
