+1 on reverting this if it doesn't break things.

On Wed, Mar 7, 2018 at 11:11 AM, Aman Sinha <amansi...@apache.org> wrote:

> Thanks Sorabh, for the analysis and the pointers to the relevant code.
> Doing the version check check for client and server would be the right
> thing to do but probably too disruptive at this stage of the release.  We
> should do it for the next release.
> For 1.13, could the change of the $values$ be reverted to the actual field
> name of the ValueVector ?  Since the DRILL-6049 was a 'hygiene' change, I
> would think that doing this revert should not break anything...but I am not
> fully sure of this.
> -Aman
> On Tue, Mar 6, 2018 at 8:01 PM, Sorabh Hamirwasia <shamirwa...@mapr.com>
> wrote:
>> Hi All,
>> The root cause for DRILL-6216 is due to a recent change made as part of
>> https://issues.apache.org/jira/browse/DRILL-6049. With this PR a default
>> field name for values ValueVector inside any NullableValueVector was
>> introduced which is $values$ [1]. Before this PR the values ValueVector
>> field name was same as the field name of actual NullableValueVector holding
>> it [2]. In the load method of certain ValueVectors like BigIntVector there
>> is an equality check for the ValueVector field name and metadata.name_part
>> name [3].
>> In setup where Drillbit and DrillClient are running in different version
>> (between 1.12 and 1.13) the equality check in load method will fail. For
>> example: When server is running 1.13 and client is on 1.12, in that case
>> the record batch from server side will come with NullableValueVector (NV1
>> with field name as num_val) but with it's values ValueVector field name as
>> $values$. When on client side corresponding NullableValueVector (NV2) is
>> created it will use the actual field name (num_val) for values ValueVector.
>> After calling load on received NullableValueVector NV2 with metadata
>> information from server that internally alls load on values ValueVector and
>> the check fails since ($values$ != num_val).
>> Since the change is in template of ValueVector, to fix this issue both
>> client and server needs to identify their respective version on which they
>> are running and choose the field name for values ValueVector
>> correspondingly. Given DRILL-6049 touches huge sets of files I am also not
>> sure what are the other impacts with it. It would be great to discuss on
>> how we should proceed with this issue before making any further change.
>> [1]: https://github.com/apache/drill/blob/master/exec/vector/src/
>> main/java/org/apache/drill/exec/vector/ValueVector.java#L92
>> https://github.com/apache/drill/blob/master/exec/vector/src/
>> main/codegen/templates/NullableValueVectors.java#L82
>> [2]:https://github.com/apache/drill/blob/1.12.0/exec/vector/
>> src/main/codegen/templates/NullableValueVectors.java#L70
>> [3]: https://github.com/apache/drill/blob/1.12.0/exec/vector/src/
>> main/codegen/templates/FixedValueVectors.java#L238
>> Thanks,
>> Sorabh

Reply via email to