Mailing-List: contact dev-help@drill.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@drill.apache.org
From: paul-rogers <git@git.apache.org>
To: dev@drill.apache.org
Reply-To: dev@drill.apache.org
References: <git-pr-840-drill@git.apache.org>
In-Reply-To: <git-pr-840-drill@git.apache.org>
Subject: [GitHub] drill pull request #840: DRILL-5517: Size-aware set methods in value vectors
Content-Type: text/plain
Message-Id: <20170630015912.1C5C1E943C@git1-us-west.apache.org>
Date: Fri, 30 Jun 2017 01:59:12 +0000 (UTC)
archived-at: Fri, 30 Jun 2017 01:59:16 -0000

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---