ARROW-18014: [Java] Implement copy functions for vectors and Table #14389

lwhite1 · 2022-10-12T18:18:26Z

Copy methods for the table and for individual vectors are provided with tests

lwhite1 · 2022-10-12T18:24:03Z

@davisusanibar If you wouldn't mind providing a review it would be appreciated.

github-actions · 2022-10-12T20:07:53Z

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

github-actions · 2022-10-12T20:07:54Z

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

lwhite1 · 2022-10-13T12:30:46Z

@lidavidm Are you familiar with the error shown in the Windows Flight tests:

Warning:  Tests run: 4, Failures: 0, Errors: 0, Skipped: 4, Time elapsed: 0 s - in org.apache.arrow.flight.TestBackPressure
[2069](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2070)
[INFO] Running org.apache.arrow.flight.TestBasicOperation
[2070](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2071)
[INFO] Running org.apache.arrow.algorithm.sort.TestVariableWidthSorting
[2071](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2072)
[INFO] Tests run: 49, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.97 s - in org.apache.arrow.algorithm.sort.TestVariableWidthSorting
[2072](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2073)
63 6F 6F 6C 20 74 68 69 6E 67 
[2073](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2074)
get
[2074](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2075)
put
[2075](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2076)
hello
[2076](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2077)
Schema<>
[2077](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2078)
Error:  Tests run: 19, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 5.808 s <<< FAILURE! - in org.apache.arrow.flight.TestBasicOperation
[2078](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2079)
Error:  org.apache.arrow.flight.TestBasicOperation.getStreamLargeBatch  Time elapsed: 2.814 s  <<< ERROR!
[2079](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2080)
java.lang.IllegalStateException: 
[2080](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2081)
Memory was leaked by query. Memory leaked: (134217728)
[2081](https://github.com/apache/arrow/actions/runs/3236942456/jobs/5303398561#step:5:2082)

lidavidm · 2022-10-13T12:32:41Z

This has been known for a while, but I've never been able to reproduce it.

lidavidm · 2022-10-13T12:36:06Z

The resolution would probably be to figure out how to enable allocator logging in CI so that we can properly debug this

lwhite1 · 2022-10-13T12:40:01Z

The resolution would probably be to figure out how to enable allocator logging in CI so that we can properly debug this

OK. thanks

lidavidm · 2022-10-13T12:56:29Z

See ARROW-18034 / ARROW-18035

lidavidm

LGTM, just one typo spotted in a doc comment

lidavidm · 2022-10-24T14:28:40Z

java/vector/src/main/java/org/apache/arrow/vector/table/BaseTable.java

+   *
+   * @param columnName The name of the vector to copy
+   * @return A copy of the Vector with the given name
+   * @throws IllegalArgumentException if the name is not the name of a vector in the table.


nit: the exception class is different from what's actually thrown

I will fix the exception class issue. Thanks

lidavidm · 2022-10-24T14:35:11Z

java/vector/src/test/java/org/apache/arrow/vector/table/BaseTableTest.java

+        assertEquals(2, copy.getValueCount());
+        assertEquals(0, copy.getNullCount());
+        for (int i = 0; i < t.getRowCount(); i++) {
+          assertEquals(original.getObject(i), copy.getObject(i));
+        }


not necessarily an issue for this PR, but there is a VectorValueComparator and perhaps it would be good to integrate it as FieldVector#equals if it isn't already

Good to know. Thank you

lwhite1 · 2022-10-24T16:15:28Z

I modified the documentation to reflect the correct Exception (It should be IllegalArgumentException.) This was mentioned incorrectly in many places in the Row class so the documentation was corrected there as well. Only documentation was touched in the latest commit

lidavidm

Thank you!

lidavidm · 2022-10-24T20:04:05Z

Seems tests need to be realigned too?

Error:    BaseTableTest.getVectorCopyByIndex:199 Unexpected exception type thrown, expected: <java.lang.IllegalStateException> but was: <java.lang.IllegalArgumentException>
Error:    BaseTableTest.getVectorCopyByName:219 Unexpected exception type thrown, expected: <java.lang.IllegalStateException> but was: <java.lang.IllegalArgumentException>
Error:    BaseTableTest.testGetVector:178 Unexpected exception type thrown, expected: <java.lang.IllegalStateException> but was: <java.lang.IllegalArgumentException>
Error:    RowTest.testNameNotFound:172 Unexpected exception type thrown, expected: <java.lang.IllegalStateException> but was: <java.lang.IllegalArgumentException>

davisusanibar · 2022-10-24T20:53:07Z

java/vector/src/main/java/org/apache/arrow/vector/table/BaseTable.java

+   * @return A copy of the Vector with the given name
+   * @throws IllegalArgumentException if the name is not the name of a vector in the table.
+   */
+  public FieldVector getVectorCopy(String columnName) {


I have a doubt, Is there some base/abstract object that Table and VectorShemaRoot share?

For example, this method could be helpful in both sides Tables and VectorSchemaRoot

If by design this was created independently please let me know to consider that at the moment to read *.table package

I have a doubt, Is there some base/abstract object that Table and VectorShemaRoot share?

For example, this method could be helpful in both sides Tables and VectorSchemaRoot

There's no abstract class as VSR doesn't inherit from anything. I have considered adding an interface at some point that they both could implement to make it easier to swap one for the other.

If by design this was created independently please let me know to consider that at the moment to read *.table package

I'm not sure I understand this part of the comment.

There's no abstract class as VSR doesn't inherit from anything. I have considered adding an interface at some point that they both could implement to make it easier to swap one for the other.

Ok, thank you.

I'm not sure I understand this part of the comment.

This is related with the 1st question that initially VSR and Table are growing independently and at some point they both could implement the same contract.

Got it. Thanks.
Given the possibility of someday having a shared interface, I have tried to write similar methods using the same method signature as much as possible.

ursabot · 2022-10-28T01:22:55Z

Benchmark runs are scheduled for baseline = ea32825 and contender = 0e94da9. 0e94da9 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed] ec2-t3-xlarge-us-east-2
[Failed] test-mac-arm
[Finished ⬇️1.36% ⬆️0.0%] ursa-i9-9960x
[Failed] ursa-thinkcentre-m75q
Buildkite builds:
[Failed] 0e94da90 ec2-t3-xlarge-us-east-2
[Failed] 0e94da90 test-mac-arm
[Finished] 0e94da90 ursa-i9-9960x
[Failed] 0e94da90 ursa-thinkcentre-m75q
[Failed] ea32825e ec2-t3-xlarge-us-east-2
[Failed] ea32825e test-mac-arm
[Finished] ea32825e ursa-i9-9960x
[Failed] ea32825e ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

This PR bumps Apache Arrow version from 10.0.0 to 11.0.0. Main changes related to PyAmber: ## Java/Scala side: - Distribute Apple M1 compatible JNI libraries via mavencentral ([#14472](apache/arrow#14472)). - Improve performance by short-circuiting null checks when comparing non null field types ([#15106](apache/arrow#15106)). - Extend Table copy functionality, and support returning copies of individual vectors ([#14389](apache/arrow#14389)). - Several enhancements to dictionary encoding ([#14891](apache/arrow#14891), ([#14902](apache/arrow#14902), ([#14874](apache/arrow#14874)). - Extend Table to support additional vector types ([#14573](apache/arrow#14573)). - Enhance and simplify handling of allocation management by integrating C Data into allocator hierarchy ([#14506](apache/arrow#14506)). ## Python side: - PyArrow now requires pandas >= 1.0 ([ARROW-18173](https://issues.apache.org/jira/browse/ARROW-18173)). - Added support for the [DataFrame Interchange Protocol](https://data-apis.org/dataframe-protocol/latest/purpose_and_scope.html) for pyarrow.Table ([GH-33346](apache/arrow#33346)). - Support for custom metadata of record batches in the IPC read and write APIs ([ARROW-16430](https://issues.apache.org/jira/browse/ARROW-16430)). - The Time32Scalar, Time64Scalar, Date32Scalar and Date64Scalar classes got a .value attribute to access the underlying integer value, similar to the other date-time related scalars ([ARROW-18264](https://issues.apache.org/jira/browse/ARROW-18264)). - Casting to string is now supported for duration ([ARROW-15822](https://issues.apache.org/jira/browse/ARROW-15822)) and decimal ([ARROW-17458](https://issues.apache.org/jira/browse/ARROW-17458)) types, which also means those can now be written to CSV. ## Issues fixed: - Now Do_action (from Python server back to Java Client) is returning a stream of results properly, and it alerts when the results are not fully consumed by the client. Such results will be used to send the flow control credits back from the Python side. We limit the results to be exact 1 for now, although it can be a stream. - Fix a bug in the Python proxy server, when unregistered action is invoked, it should not parse and return the results.

…pache#14389) Copy methods for the table and for individual vectors are provided with tests Authored-by: Larry White <[email protected]> Signed-off-by: David Li <[email protected]>

lwhite1 added 2 commits October 12, 2022 10:43

partial implementation and testing of copy functions

9eb898b

Add the copy functions and their associated tests

2ffe868

github-actions bot added the Component: Java label Oct 12, 2022

lidavidm approved these changes Oct 24, 2022

View reviewed changes

Fixed documentation issue: Wrong Exception named in doc

8e18a09

lidavidm approved these changes Oct 24, 2022

View reviewed changes

fix broken tests

6ae8b83

davisusanibar reviewed Oct 24, 2022

View reviewed changes

davisusanibar approved these changes Oct 24, 2022

View reviewed changes

lidavidm merged commit 0e94da9 into apache:master Oct 25, 2022

asfimport mentioned this pull request Oct 28, 2022

[Java] Add copy functions to Table #33217

Closed

Yicong-Huang mentioned this pull request May 1, 2023

Bump Apache Arrow to 11.0.0 apache/texera#1900

Merged

ARROW-18014: [Java] Implement copy functions for vectors and Table #14389

ARROW-18014: [Java] Implement copy functions for vectors and Table #14389

Uh oh!

Conversation

lwhite1 commented Oct 12, 2022

Uh oh!

lwhite1 commented Oct 12, 2022

Uh oh!

github-actions bot commented Oct 12, 2022

Uh oh!

github-actions bot commented Oct 12, 2022

Uh oh!

lwhite1 commented Oct 13, 2022

Uh oh!

lidavidm commented Oct 13, 2022

Uh oh!

lidavidm commented Oct 13, 2022

Uh oh!

lwhite1 commented Oct 13, 2022

Uh oh!

lidavidm commented Oct 13, 2022

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lwhite1 commented Oct 24, 2022

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

lidavidm commented Oct 24, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ursabot commented Oct 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants