chore: Revise array import to more follow C Data Interface semantics#905
Merged
viirya merged 8 commits intoapache:mainfrom Sep 6, 2024
Merged
chore: Revise array import to more follow C Data Interface semantics#905viirya merged 8 commits intoapache:mainfrom
viirya merged 8 commits intoapache:mainfrom
Conversation
00ac1e4 to
8a94f10
Compare
Member
|
@Kontinuation fyi |
feb1fea to
f6def47
Compare
Kontinuation
reviewed
Sep 4, 2024
Kontinuation
reviewed
Sep 4, 2024
f6def47 to
f748ba3
Compare
viirya
commented
Sep 4, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #905 +/- ##
=============================================
+ Coverage 34.03% 54.85% +20.81%
+ Complexity 883 853 -30
=============================================
Files 113 109 -4
Lines 43170 10667 -32503
Branches 9516 2044 -7472
=============================================
- Hits 14693 5851 -8842
+ Misses 25471 3787 -21684
+ Partials 3006 1029 -1977 ☔ View full report in Codecov by Sentry. |
Member
Author
|
Okay. The latest commit fixes the SIGSEGV errors. |
andygrove
reviewed
Sep 5, 2024
2f862f2 to
ad61ee9
Compare
kazuyukitanimura
approved these changes
Sep 6, 2024
|
|
||
| /// Prepares arrow arrays for output. | ||
| fn prepare_output( | ||
| unsafe fn prepare_output( |
Contributor
There was a problem hiding this comment.
Is it possible to narrow down the area of unsafe?
Member
Author
|
I will go to merge this today if no more comments. Thank you. |
Member
Author
|
Merged. Thanks @andygrove @Kontinuation @kazuyukitanimura |
Member
Author
|
Ah, I found I forgot to commit the patch addressing latest reviews #905 (comment) and #905 (comment). Opend a followup at #920 for that. |
This was referenced Sep 6, 2024
Merged
coderfender
pushed a commit
to coderfender/datafusion-comet
that referenced
this pull request
Dec 13, 2025
…pache#905) * chore: Revise array import to more follow C Data Interface semantics * more * fix * For review * Try * check alignment * Try * Add comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #885.
Rationale for this change
This is another part of #885. In #893, we already revised the way we export arrays to native side. We now allocate array/schema structures in native side when exporting arrays from JVM to native.
Similarly, this PR revises the way we import array from native side by allocating array/schema structures in JVM when importing arrays from native to JVM.
What changes are included in this PR?
How are these changes tested?