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

FiV0 · 2024-04-08T09:19:30Z

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: [Java] SplitAndTransfer throws for (0,0) if vector empty #30866

github-actions · 2024-04-08T09:19:53Z

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

github-actions · 2024-04-08T09:21:45Z

⚠️ GitHub issue #30866 has been automatically assigned in GitHub to PR creator.

lidavidm

Thanks, just one nit

java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java

vibhatha · 2024-04-09T00:42:36Z

@github-actions crossbow submit -g java

github-actions · 2024-04-09T00:46:03Z

Revision: 937e93d

Submitted crossbow builds: ursacomputing/crossbow @ actions-b4f4444c91

Task	Status
java-jars
verify-rc-source-java-linux-almalinux-8-amd64
verify-rc-source-java-linux-conda-latest-amd64
verify-rc-source-java-linux-ubuntu-20.04-amd64
verify-rc-source-java-linux-ubuntu-22.04-amd64
verify-rc-source-java-macos-amd64

…mpty

conbench-apache-arrow · 2024-04-09T13:04:17Z

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 5dc5086.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

…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]>

…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]>

…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]>

) ### Rationale for this change Empty MapVector.splitAndTransfer throws `java.lang.IndexOutOfBoundsException`. Details in #44626 ### What changes are included in this PR? Fixed for MapVector as for other vector types in #41066 ### Are these changes tested? Added unit test mimicking the scenario we've observed where MapVector's offset buffer capacity is 0. * GitHub Issue: #44626 Authored-by: Maksim Yegorov <[email protected]> Signed-off-by: David Li <[email protected]>

FiV0 requested a review from lidavidm as a code owner April 8, 2024 09:19

github-actions bot added Component: Java awaiting review Awaiting review labels Apr 8, 2024

FiV0 changed the title ~~ARROW-15382: [Java] fix SplitAndTransfer throws for (0,0) if vector empty~~ GH-30866: [Java] fix SplitAndTransfer throws for (0,0) if vector empty Apr 8, 2024

FiV0 force-pushed the ARROW-15382 branch from 13af659 to 937e93d Compare April 8, 2024 14:08

lidavidm approved these changes Apr 9, 2024

View reviewed changes

java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java Outdated Show resolved Hide resolved

github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Apr 9, 2024

FiV0 force-pushed the ARROW-15382 branch from 937e93d to f059b1d Compare April 9, 2024 07:36

ARROW-15382: [Java] fix SplitAndTransfer throws for (0,0) if vector e…

f059b1d

…mpty

lidavidm approved these changes Apr 9, 2024

View reviewed changes

lidavidm merged commit 5dc5086 into apache:main Apr 9, 2024
16 checks passed

lidavidm removed the awaiting merge Awaiting merge label Apr 9, 2024

lidavidm mentioned this pull request Apr 9, 2024

[Java] SplitAndTransfer throws for (0,0) if vector empty #30866

Closed

lriggs mentioned this pull request Sep 3, 2024

Cherry pick fix for empty vectors and some additional build process fixes. dremio/arrow#81

Merged

maksimyego-db mentioned this pull request Oct 9, 2024

[Java] VectorSchemaRoot.slice() throws for NullVector #44344

Closed

maksimyego-db mentioned this pull request Nov 3, 2024

[Java] MapVector.splitAndTransfer throws for empty vector #44626

Closed

myegorov mentioned this pull request Nov 3, 2024

GH-44626: [Java] fix SplitAndTransfer throws for empty MapVector #44627

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

FiV0 commented Apr 8, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Apr 8, 2024

github-actions bot commented Apr 8, 2024

lidavidm left a comment

vibhatha commented Apr 9, 2024

github-actions bot commented Apr 9, 2024

conbench-apache-arrow bot commented Apr 9, 2024

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

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

Conversation

FiV0 commented Apr 8, 2024 • edited by github-actions bot Loading

github-actions bot commented Apr 8, 2024

github-actions bot commented Apr 8, 2024

lidavidm left a comment

Choose a reason for hiding this comment

vibhatha commented Apr 9, 2024

github-actions bot commented Apr 9, 2024

conbench-apache-arrow bot commented Apr 9, 2024

FiV0 commented Apr 8, 2024 •

edited by github-actions bot

Loading