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.

Sign up for GitHub

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

Jump to bottom

ARROW-15382: [Java] fix SplitAndTransfer throws for (0,0) if vector empty #12250

Closed
wants to merge 1 commit into from
Closed

ARROW-15382: [Java] fix SplitAndTransfer throws for (0,0) if vector empty #12250

wants to merge 1 commit into from

Conversation

wzx140
Copy link

@wzx140 wzx140 commented Jan 25, 2022

No description provided.

@github-actions
Copy link

https://issues.apache.org/jira/browse/ARROW-15382

@wzx140
Copy link
Author

wzx140 commented May 18, 2022

@lidavidm Could you please review it? Really thanks!

@lidavidm
Copy link
Member

@davisusanibar would you like to take a look here first?

lidavidm
lidavidm approved these changes May 19, 2022
View reviewed changes
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think it would be good to have a test with a non-empty vector but transferring 0 length as well (also, trying a non-zero startIndex but with 0 length).

@pitrou
Copy link
Member

pitrou commented Jun 7, 2022

@wzx140 Would you like to add another unit test as suggested by @lidavidm ?

@lidavidm
Copy link
Member

lidavidm commented Nov 7, 2022

@davisusanibar or @lwhite1 would you like to perhaps carry this over the finish line?

@lwhite1
Copy link
Contributor

lwhite1 commented Nov 7, 2022

@lidavidm yes, I will add the suggested tests.

@lwhite1
Copy link
Contributor

lwhite1 commented Nov 16, 2022

Thanks. I think it would be good to have a test with (1) a non-empty vector but transferring 0 length as well (also, (2) trying a non-zero startIndex but with 0 length).

I created a test with a non-empty vector and transferring length 0, but it throws an exception when the allocator is closed java.lang.IllegalStateException: Allocator[ROOT] closed with outstanding buffers allocated (1).

The same test transferring length 1 passes.

lwhite1
lwhite1 suggested changes Nov 17, 2022
View reviewed changes
Copy link
Contributor

@lwhite1 lwhite1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in earlier comment, this PR does not address situations where the source vector is non-empty, and the transfer length is 0.

@amol-
Copy link
Member

amol- commented Mar 30, 2023

Closing because it has been untouched for a while, in case it's still relevant feel free to reopen and move it forward 👍

@amol- amol- closed this Mar 30, 2023
@FiV0
Copy link
Contributor

FiV0 commented Apr 2, 2024

Hey, could this be reopened. This is still an issue in Arrow 15.0.2 and I run into it multiple times. Also happy to look into it if further work needs to be done.

@FiV0 FiV0 mentioned this pull request Apr 8, 2024
Merged
@asfimport asfimport mentioned this pull request Nov 16, 2022
Closed
lidavidm pushed a commit that referenced this pull request Apr 9, 2024
@FiV0
GH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vector empty (
5dc5086 
#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of #12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: #30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
verma-kartik pushed a commit to verma-kartik/arrow that referenced this pull request Apr 11, 2024
@FiV0 @verma-kartik
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
327b9fc 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request Apr 15, 2024
@FiV0 @vibhatha
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
97f9443 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
tolleybot pushed a commit to tmct/arrow that referenced this pull request May 2, 2024
@FiV0 @tolleybot
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
a3f9574 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
tolleybot pushed a commit to tmct/arrow that referenced this pull request May 4, 2024
@FiV0 @tolleybot
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
02462c5 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
rok pushed a commit to tmct/arrow that referenced this pull request May 8, 2024
@FiV0 @rok
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
7c147e0 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
rok pushed a commit to tmct/arrow that referenced this pull request May 8, 2024
@FiV0 @rok
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
f3f337f 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
FiV0 added a commit to xtdb/arrow that referenced this pull request May 24, 2024
@FiV0
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
480ee97 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
@FiV0 @vibhatha
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
461a89c 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
jarohen pushed a commit to xtdb/arrow that referenced this pull request Jul 17, 2024
@FiV0 @jarohen
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
37300b1 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
lriggs pushed a commit to lriggs/arrow that referenced this pull request Sep 3, 2024
@FiV0 @lriggs
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
6d8505a 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
@lriggs lriggs mentioned this pull request Sep 3, 2024
Merged
lriggs added a commit to dremio/arrow that referenced this pull request Sep 4, 2024
@lriggs @FiV0
@kou @raulcd
Cherry pick fix for empty vectors and some additional build process f…
0924124 
…ixes. (#81)

* apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vector empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>

* apacheGH-43463: [C++][Gandiva] Always use gdv_function_stubs.h in context_helper.cc (apache#43464)

### Rationale for this change

`gdv_function_stubs.h` has declarations of functions in `context_helper.cc`.

If we don't include `gdv_function_stubs.h`, it causes attribution mismatch error with unity build.

### What changes are included in this PR?

Always include `gdv_function_stubs.h` in `context_helper.cc`.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#43463

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>

* apacheGH-43119: [CI][Packaging] Update manylinux 2014 CentOS repos that have been deprecated (apache#43121)

### Rationale for this change

Jobs are failing to find mirrorlist.centos.org

### What changes are included in this PR?

Updating repos based on solution from: apache#43119 (comment)

### Are these changes tested?

Via archery

### Are there any user-facing changes?
No
* GitHub Issue: apache#43119

Lead-authored-by: Raúl Cumplido <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>

* Update macos deployment target to 12 to match build machine.

* apacheGH-43400: [C++] Ensure using bundled GoogleTest when we use bundled GoogleTest (apache#43465)

### Rationale for this change

If we use bundled GoogleTest and system other dependencies such as Boost, our include path options may be:

* `-isystem /opt/homebrew/include` (for Boost)
* `-isystem build_dir/_deps/googletest-src/googletest` (for bundled GoogleTest)
* `-isystem build_dir/_deps/googletest-src/googlemock` (for bundled GoogleTest)

With this order, GoogleTest headers in `/opt/homebrew/include/` are used with bundled GoogleTest. It may cause link errors.

### What changes are included in this PR?

This change introduces a new CMake target
`arrow::GTest::gtest_headers` that has include paths for bundled GoogleTest. And it's always used as the first link library of all test program. With this change, our include path options are:

* `-isystem build_dir/_deps/googletest-src/googletest` (for bundled GoogleTest)
* `-isystem build_dir/_deps/googletest-src/googlemock` (for bundled GoogleTest)
* `-isystem /opt/homebrew/include` (for Boost)

With this order, we can always use our bundled GoogleTest.

`arrow::GTest::gtest_headers` is defined only when we use bundled GoogleTest. So this doesn't change the system GoogleTest case.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* GitHub Issue: apache#43400

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>

---------

Signed-off-by: David Li <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Co-authored-by: Finn Völkel <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Raúl Cumplido <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
lriggs pushed a commit to lriggs/arrow that referenced this pull request Sep 5, 2024
@FiV0 @lriggs
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
ef9b9b5 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
lriggs pushed a commit to lriggs/arrow that referenced this pull request Sep 6, 2024
@FiV0 @lriggs
apacheGH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vecto…
b7db21e 
…r empty (apache#41066)

This is addresses https://issues.apache.org/jira/browse/ARROW-15382 and is reopening of apache#12250 (which I asked to be reopened).

I tried to address all the comments from the previous discussion, added some more tests and fixed an issue in the old commit.
* GitHub Issue: apache#30866

Authored-by: Finn Völkel <[email protected]>
Signed-off-by: David Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lwhite1 lwhite1 lwhite1 requested changes

@lidavidm lidavidm lidavidm approved these changes

Assignees
No one assigned
Labels
Component: Java
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@wzx140 @lidavidm @pitrou @lwhite1 @amol- @FiV0