Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace JNA usage in ArrowTransferDefinitions with JavaCPP #499

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

EmergentOrder
Copy link
Collaborator

@EmergentOrder EmergentOrder commented Feb 16, 2022

This needs to be reconciled with the addition of NullableScalarVec before merging (transfer-definitions.cpp and parts of transfer-definitions.hpp can be removed when this is done).

Was passing all tests before the latest merge (with additions to NullableScalarVec) , currently compiles but fails tests.

build.sbt Outdated Show resolved Hide resolved
@EmergentOrder EmergentOrder marked this pull request as draft February 16, 2022 23:23
@EmergentOrder EmergentOrder marked this pull request as ready for review February 16, 2022 23:24
Can't exclude any because they don't have defaults
@EmergentOrder EmergentOrder marked this pull request as draft February 17, 2022 02:04
@EmergentOrder EmergentOrder marked this pull request as ready for review February 17, 2022 02:05
@EmergentOrder EmergentOrder marked this pull request as draft February 20, 2022 19:57
@EmergentOrder EmergentOrder marked this pull request as ready for review February 20, 2022 19:58
@EmergentOrder EmergentOrder marked this pull request as draft February 20, 2022 20:11
@EmergentOrder EmergentOrder marked this pull request as ready for review February 20, 2022 20:12
@wgip
Copy link
Contributor

wgip commented Feb 20, 2022

Shows merge conflicts at the moment

image

@EmergentOrder
Copy link
Collaborator Author

Working on the merge now.

@EmergentOrder
Copy link
Collaborator Author

FYI there remain a few usages of JNA which need to be replaced, in CRunner.scala and CArrowNativeInterface.scala

@EmergentOrder EmergentOrder marked this pull request as draft February 20, 2022 21:27
@EmergentOrder EmergentOrder marked this pull request as ready for review February 20, 2022 21:28
@wgip
Copy link
Contributor

wgip commented Feb 20, 2022

Unit test JVM looks good

com.nec.spark.cgescape.CodegenEscapeSpec fails but unrelated to this PR I believe

@wgip
Copy link
Contributor

wgip commented Feb 20, 2022

CMake scope

image

If possible, might be appropriate load the lib programmatically rather than via SBT, then it should be more consistent across different scopes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants