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 <[email protected]> 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" <[email protected]> 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>
