advancedxy commented on PR #239: URL: https://github.com/apache/arrow-datafusion-comet/pull/239#issuecomment-2030903762
> No. As empty offset buffer is invalid, we are not going to merge the fix into arrow-rs. It is a hacky fix temporarily only before we have the fix at Java Arrow. I see. Thanks for the clarification. I didn't see that conclusion and thought arrow-rs will allow empty buffer for compatible reason. > We don't use the main branch of arrow-rs directly but a specified branch in forked repo with a temporary fix. The branch is frozen If we don't update it. I know the specified branch is frozen if we don't update it. But it might contain untested/unstable code in the specified branch as it was kind of cut directly from master at some point with a temporary fix. If we are going to go with that approach, do you think it's better to checkout the specified branch from a released tag, such as `50.0.0`? In that way, the specified branch was already tested for release. > But it is also a hacky fix. We need to manually handle these offset-layouted arrays. Of course, it's hacky too. The long term fix should be fixing Java Arrow's C data interface's exporting. Compared to a specified branch of arrow-rs though, this fix is self-contained in the Comet's repo, which might has lower maintenance cost and doesn't depend on the arrow-rs/datafusion, which might be iterated fast to introduce new features/fixes. Anyway, I think current approach is also a good way to iterate fast as long as the Java's Arrow C data API will be fixed soon. -- 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]
