Mailing-List: contact dev-help@drill.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@drill.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAJrw0OR-yn0tdY--fLqnqUwQ7Zmo+Ms6NfnR0FfhT1tt0+XBOQ@mail.gmail.com>
References: 
 <CAFyDVv+FPdFRFQZumvSNUs+G2m2OCuqdwmttVFJAj4jHQO5Fkw@mail.gmail.com>
	<CAJrw0OR-yn0tdY--fLqnqUwQ7Zmo+Ms6NfnR0FfhT1tt0+XBOQ@mail.gmail.com>
Date: Tue, 25 Aug 2015 08:02:58 -0700
Message-ID: 
 <CAKTYAC-vAMmeKbMzV61TgFDi9VLJhVXMMcnU=BVRchbRcQjigQ@mail.gmail.com>
Subject: Re: zeroVectors() interface for value vectors
From: Abdel Hakim Deneche <adeneche@maprtech.com>
To: "dev@drill.apache.org" <dev@drill.apache.org>
Content-Type: multipart/alternative; boundary=001a1142196e90193c051e2407a0

--001a1142196e90193c051e2407a0
Content-Type: text/plain; charset=UTF-8

Another important point to keep in mind here: ValueVectorWriteExpression
operates under the "undocumented" assumption that the destination vector is
empty, this way it can safely skip writing null values. In the case of
window functions I am using a value vector as an internal buffer to hold
values between batches which voids the assumption.
If this assumption is indeed correct then adding zeroVector to value
vectors is indeed the way to go.

On Mon, Aug 24, 2015 at 8:30 PM, Jacques Nadeau <jacques@dremio.com> wrote:

> In general, let's try to avoid extending the core structures like value
> vector read and write expressions for a single operator. Zerovector is
> trivial to implement so let's resolve that way (trivial since the
> underlying vector already has it and we just need to delegate down).
> On Aug 24, 2015 3:36 PM, "Aman Sinha" <amansinha@apache.org> wrote:
>
> > I am reviewing Hakim's patch for DRILL-3668 (first_value window function
> > incorrect result).  His code uses ValueVectorWriteExpression to set
> values
> > in an internal batch which get re-used across different partitions of the
> > window function.  Ideally, we just want to zero out the vector rather
> than
> > calling clear() since clear() will release the buffer.
> >
> > However, currently zeroVectors() is only supported by FixedWidthVector,
> not
> > VariableWidthVector.  * Should there be such an interface for variable
> > width ? * The implementation could zero out just the offset vector.
> >
> > In the absence of such an interface, Hakim has added a boolean flag
> > witeNulls to ValueVectorWriteExpression (see
> >
> >
> https://github.com/adeneche/incubator-drill/commit/cab73cd1a50163dd25fe0f9c55c264780ea3616d
> > )
> >  and is conditionally doing the null-ing out in the generated code.  It
> > won't affect the normal code path, it would get used for specific window
> > functions.
> >
> > I am thinking of committing his patch and tracking the zeroVectors()
> > enhancement separately (if people agree that it would be useful).  Let me
> > know...
> >
> > Aman
> >
>


-- 

Abdelhakim Deneche

Software Engineer

  <http://www.mapr.com/>


Now Available - Free Hadoop On-Demand Training
<http://www.mapr.com/training?utm_source=Email&utm_medium=Signature&utm_campaign=Free%20available>

--001a1142196e90193c051e2407a0--