+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 >> >> >
