andishgar commented on issue #50249: URL: https://github.com/apache/arrow/issues/50249#issuecomment-4850755748
@pitrou, several notes: 1. Regarding this: > If we want to add one, we should do so for all builder classes, not just BinaryView. I'm okay with that, and I'm aware that `SparseUnion`, `DenseUnion`, and `RunEndEncoded` builders have the bitmap buffer in different positions, so I should use a different strategy to check whether the data is null. I'll send a follow-up PR to address this by adding `IsNull` support for all builders. 2. I found that the following code doesn't appear to be used anywhere, and its logic also seems incorrect. Shouldn't we remove it? https://github.com/apache/arrow/blob/bd8c6cc86749af8e70905ca3e20f5b81bfd8ab1a/cpp/src/arrow/array/builder_base.cc#L70-L86 3. Regarding this: > It could make sense to have some kind of BinaryViewDataBuilder class that builds the data buffers (excluding the null bitmap). I'll open a separate issue to discuss this. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
