Github user jaltekruse commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-144846430
Overall, the changes look solid.
I am thinking it may be worth trying to remove the bodies of the existing
getBufferSize() methods and replace them with calls into these new methods,
passing in the current valueCount that is expected to be set with a call to
setValueCount(int) before getBufferSize() is used. It seems weird to have two
different ways to compute the same values, although I see the reason for it,
previously we were using the writerIndex() for all of the fixed length
primitives, which is also set in setValueCount().
I don't consider it a must fix to check in, I just wanted to know if you
had thought about this and ruled it out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
