jacques-n commented on pull request #6156: URL: https://github.com/apache/arrow/pull/6156#issuecomment-657095875
> @tianchen92 rereading, after rereading all the comments. I think we should > > 1. Remove setReaderWriterIndeces in getFieldBuffers > 2. Deprecate getBuffers > 3. Introduce a new getIpcBuffers which is unambiguously used for writing record batches (i.e. in VectorUnloader). > 4. Update documentation where it makes sense based on all this conversation. > > @jacques-n or someone else from dremio can maybe provide additional insight into how getBuffers is used and whether we really need to keep it I agree with item 1 on your list. I think 2-4 need more conversation about what we want to expose. I'd definitely avoid introducing a new method (3 on your list) until we figure out what sets of functionality we want. getBuffers() may actually be exactly what we want (with a changed order as necessary). I think it would be valuable to rationalize getFieldBuffers() in this context. Remember that FieldVector and getFieldBuffers() were introduced when we had separate non-nullable and nullable vectors but wanted to treat the non-nullable ones as internal (and thus they didn't expose the FieldVector interface). It seems like we have several operational needs: getFieldBuffers: get the list of buffers cheaply with no modifications to do some low level buffer operations (e.g. hand written addition logic) getBuffers: export the list of buffers for writing (with or without removing them-- why both...)? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
