Paul , thanks for confirming that it is safe to revert.
On Wed, Mar 7, 2018 at 11:52 AM, Paul Rogers <par0...@yahoo.com.invalid> wrote: > Hi Sorabh, > Thanks for tracking this one down. Our unit tests did not uncover this > issue when I did the original PR, unfortunately. The name change was done > to be consistent with other places where we use special names and, as I > recall, help with certain tasks. > Clearly, however, if the client depends on the name, we cannot change the > name without breaking the client. It is safe to simply revert the change > and rerun the tests. > (Having an API version number would have been handy here: we could detect > the older client. But Drill, like most Hadoop components, has no API > versioning mechanism, alas.) > To prevent future issues, let's also change that bit of client code: there > is no reason to assert the name; the structural relationship is all that > should matter. > Thanks, > - Paul > > > > On Tuesday, March 6, 2018, 8:02:05 PM PST, 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 > >
