Uh oh!
There was an error while loading. Please reload this page.
ARROW-7284: [Java] ensure java implementation meets clarified dictionary spec - #5945
ARROW-7284: [Java] ensure java implementation meets clarified dictionary spec#5945tianchen92 wants to merge 2 commits into
Conversation
tianchen92
commented
Dec 3, 2019
cc @emkornfield |
emkornfield
commented
Dec 12, 2019
@tianchen92 sorry, I've been backlogged will try to review your open PRs over the next week or so (unless others get to them sooner CC @praveenbingo@pravindra@BryanCutler) |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
tianchen92
commented
Jan 10, 2020
Thanks @emkornfield, PR updated according to your comments. |
| serializeDictionaryBatch(out, dictionary1, false, closeableList); | ||
| // write recordBatch2 | ||
| serializeRecordBatch(out, Arrays.asList(encodedVector1, encodedVector2), closeableList); |
There was a problem hiding this comment.
is this necessary for this test?
There was a problem hiding this comment.
I guess yes, if we only sent dictionaries without batches, seems dictionaries won't be read since we made this change in https://issues.apache.org/jira/browse/ARROW-6040
… batch Related to [ARROW-7546](https://issues.apache.org/jira/browse/ARROW-7546). Per discussion #5945 (comment). In ARROW-7284, we write a simple method to concat vectors. However, ARROW-7073 is about to concat vector values efficiently, after this PR merged, we should use this new implementation in ArrowReader. Closes#6431 from tianchen92/ARROW-7546 and squashes the following commits: 9bced46 <tianchen> ARROW-7546: Use new implementation to concat vectors values in batch Authored-by: tianchen <niki.lj@alibaba-inc.com> Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
… batch Related to [ARROW-7546](https://issues.apache.org/jira/browse/ARROW-7546). Per discussion apache/arrow#5945 (comment). In ARROW-7284, we write a simple method to concat vectors. However, ARROW-7073 is about to concat vector values efficiently, after this PR merged, we should use this new implementation in ArrowReader. Closes #6431 from tianchen92/ARROW-7546 and squashes the following commits: 9bced461c <tianchen> ARROW-7546: Use new implementation to concat vectors values in batch Authored-by: tianchen <niki.lj@alibaba-inc.com> Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
…ary spec Related to [ARROW-7284](https://issues.apache.org/jira/browse/ARROW-7284). As discussed on [[link](https://lists.apache.org/thread.html/d0f137e9db0abfcfde2ef879ca517a710f620e5be4dd749923d22c37@%3Cdev.arrow.apache.org%3E)]. This is the java side implementation. 1. It is not required that all dictionary batches occur at the beginning of the IPC stream format (if a the first record batch has an all null dictionary encoded column, the null column's dictionary might not be sent until later in the stream). 2. A second dictionary batch for the same ID that is not a "delta batch" in an IPC stream indicates the dictionary should be replaced. 3. Clarifies that the file format, can only contain 1 "NON-delta" dictionary batch and multiple "delta" dictionary batches. 4. Add an enum to dictionary metadata for possible future changes in what format dictionary batches can be sent. (the most likely would be an array Map<Int, Value>). An enum is needed as a place holder to allow for forward compatibility past the release 1.0.0. Closesapache#5945 from tianchen92/ARROW-7284 and squashes the following commits: 16a609d <tianchen> resolve some comments 1231756 <tianchen> ARROW-7284: ensure java implementation meets clarified dictionary spec Authored-by: tianchen <niki.lj@alibaba-inc.com> Signed-off-by: Micah Kornfield <emkornfield@gmail.com>
Related to ARROW-7284.
As discussed on [link]. This is the java side implementation.
It is not required that all dictionary batches occur at the beginning
of the IPC stream format (if a the first record batch has an all null
dictionary encoded column, the null column's dictionary might not be sent
until later in the stream).
A second dictionary batch for the same ID that is not a "delta batch"
in an IPC stream indicates the dictionary should be replaced.
Clarifies that the file format, can only contain 1 "NON-delta"
dictionary batch and multiple "delta" dictionary batches.
Add an enum to dictionary metadata for possible future changes in what
format dictionary batches can be sent. (the most likely would be an array
Map<Int, Value>). An enum is needed as a place holder to allow for forward
compatibility past the release 1.0.0.