Skip to content

Conversation

@pitrou
Copy link
Member

@pitrou pitrou commented Apr 14, 2020

Implement integration tests for LargeList, LargeBinary and LargeString types.
Enable them for C++ (only).

Also add tests for recursive nested types.

@pitrou pitrou requested review from bkietz and wesm and removed request for bkietz April 14, 2020 16:27
@github-actions
Copy link

Implement integration tests for LargeList, LargeBinary and LargeString types.
Enable them for C++ (only).
@pitrou pitrou force-pushed the ARROW-8450-integ-large-offset-types branch from 8d52a87 to 5240b14 Compare April 14, 2020 17:14
@pitrou
Copy link
Member Author

pitrou commented Apr 14, 2020

@pitrou pitrou requested a review from bkietz April 15, 2020 08:02
@pitrou
Copy link
Member Author

pitrou commented Apr 15, 2020

Hmm, I notice the 64-bit offsets are serialized as JSON integers. @nealrichardson should they be serialized as strings instead?

@bkietz
Copy link
Member

bkietz commented Apr 15, 2020

@pitrou I believe they should, looking for the relevant JIRA

@pitrou
Copy link
Member Author

pitrou commented Apr 15, 2020

Hmm, it seems that even the "DATA" of int64 columns is output as JSON integers currently...

@bkietz
Copy link
Member

bkietz commented Apr 15, 2020

Hmmm, that should have been changed in #5267

@pitrou
Copy link
Member Author

pitrou commented Apr 15, 2020

That seems to have been lost when it was migrated to Archery.

@bkietz bkietz force-pushed the ARROW-8450-integ-large-offset-types branch from 8763f37 to 5240b14 Compare April 15, 2020 15:09
Copy link
Member

@bkietz bkietz left a comment

Choose a reason for hiding this comment

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

It was surprising to me that GetIntArray and WriteIntField were not being used to handle the data buffers of integer arrays (which would have fixed https://issues.apache.org/jira/browse/ARROW-8471 ). Some deduplication should be possible in a follow up

@pitrou
Copy link
Member Author

pitrou commented Apr 15, 2020

Yes, it seems so.

@bkietz
Copy link
Member

bkietz commented Apr 15, 2020

merging

@bkietz bkietz closed this in 3e3712a Apr 15, 2020
@pitrou pitrou deleted the ARROW-8450-integ-large-offset-types branch April 15, 2020 16:48
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.

2 participants