ARROW-3256,3304: [JS] fix file footer inconsistency, yield all messages from the stream reader#2616
Closed
trxcllnt wants to merge 3 commits intoapache:masterfrom
Closed
ARROW-3256,3304: [JS] fix file footer inconsistency, yield all messages from the stream reader#2616trxcllnt wants to merge 3 commits intoapache:masterfrom
trxcllnt wants to merge 3 commits intoapache:masterfrom
Conversation
…tables from the same source stream
wesm
reviewed
Sep 26, 2018
Member
wesm
left a comment
There was a problem hiding this comment.
The metadata changes look good to me
| constructor(version: MetadataVersion, length: Long | number, nodes: FieldMetadata[], buffers: BufferMetadata[], bodyLength?: Long | number) { | ||
| if (bodyLength === void(0)) { | ||
| bodyLength = buffers.reduce((s, b) => align(s + b.length + (b.offset - s), 8), 0); | ||
| bodyLength = buffers.reduce((bodyLength, buffer) => bodyLength + buffer.length, 0); |
Member
There was a problem hiding this comment.
Are your buffer lengths here guaranteed to a multiple of 8? Otherwise should round up
Contributor
Author
There was a problem hiding this comment.
Yep, this size is rounded up in the writer here
Member
|
Cool, @TheNeuralBit does this look good? |
Member
|
👍 LGTM |
trxcllnt
added a commit
that referenced
this pull request
Sep 27, 2018
Fixes https://issues.apache.org/jira/browse/ARROW-3336. Realized too late I branched from the #2616, so that's why there's some extra commits at the front. Check out the relevant tests here: https://github.com/trxcllnt/arrow/blob/860f61046fca0081dbe0ce986a97408ed5934e22/js/test/unit/writer-tests.ts#L26. This test covers the primitive, nested, and dictionary vectors, but it'd be worth adding more (for example, Unions). Author: ptaylor <paul.e.taylor@me.com> Closes #2638 from trxcllnt/js-fix-writing-sliced-arrays and squashes the following commits: bcd58ca <ptaylor> fix writer-tests.ts import fa4d8f5 <ptaylor> only use vector offset to serialize bitmaps and data buffers of variable-width vectors, as all other buffers have already been adjusted as part of the original call to Vector#slice() 5ece080 <ptaylor> reset internal children Array in case the childData for each has been sliced e093f7c <ptaylor> add test validating writer serializes sliced arrays correctly f9dd91b <ptaylor> move vector comparison extension into separate module 380d655 <ptaylor> fix table constructor args
kou
pushed a commit
to apache/arrow-js
that referenced
this pull request
May 14, 2025
Fixes https://issues.apache.org/jira/browse/ARROW-3336. Realized too late I branched from the apache/arrow#2616, so that's why there's some extra commits at the front. Check out the relevant tests here: https://github.com/trxcllnt/arrow/blob/860f61046fca0081dbe0ce986a97408ed5934e22/js/test/unit/writer-tests.ts#L26. This test covers the primitive, nested, and dictionary vectors, but it'd be worth adding more (for example, Unions). Author: ptaylor <paul.e.taylor@me.com> Closes #2638 from trxcllnt/js-fix-writing-sliced-arrays and squashes the following commits: bcd58caa <ptaylor> fix writer-tests.ts import fa4d8f54 <ptaylor> only use vector offset to serialize bitmaps and data buffers of variable-width vectors, as all other buffers have already been adjusted as part of the original call to Vector#slice() 5ece0806 <ptaylor> reset internal children Array in case the childData for each has been sliced e093f7c0 <ptaylor> add test validating writer serializes sliced arrays correctly f9dd91b0 <ptaylor> move vector comparison extension into separate module 380d6555 <ptaylor> fix table constructor args
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
3 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.
Yield all messages from the stream reader to support reading multiple tables from the same source stream. This is something a stream-reader refactor should support out of the box, but this PR adds this ability to master until that refactor is done. I added an integration test that shows what such a reader would look like today.