-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-8674: [JS] Implement IPC RecordBatch body buffer compression #13076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f18c1d3
fc7a082
e8d9aef
08f1b7b
3438858
2017793
cbb3d2f
483a614
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Licensed to the Apache Software Foundation (ASF) under one | ||
| // or more contributor license agreements. See the NOTICE file | ||
| // distributed with this work for additional information | ||
| // regarding copyright ownership. The ASF licenses this file | ||
| // to you under the Apache License, Version 2.0 (the | ||
| // "License"); you may not use this file except in compliance | ||
| // with the License. You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, | ||
| // software distributed under the License is distributed on an | ||
| // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| // KIND, either express or implied. See the License for the | ||
| // specific language governing permissions and limitations | ||
| // under the License. | ||
|
|
||
| import { CompressionType } from '../fb/compression-type.js'; | ||
|
|
||
| export interface Codec { | ||
| encode?(data: Uint8Array): Uint8Array; | ||
| decode?(data: Uint8Array): Uint8Array; | ||
| } | ||
|
|
||
| class _CompressionRegistry { | ||
| protected declare registry: { [key in CompressionType]?: Codec }; | ||
|
|
||
| constructor() { | ||
| this.registry = {}; | ||
| } | ||
|
|
||
| set(compression: CompressionType, codec: Codec) { | ||
| this.registry[compression] = codec; | ||
| } | ||
|
|
||
| get(compression?: CompressionType): Codec | null { | ||
| if (compression !== undefined) { | ||
| return this.registry?.[compression] || null; | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| export const compressionRegistry = new _CompressionRegistry(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ import { | |
| isFileHandle, isFetchResponse, | ||
| isReadableDOMStream, isReadableNodeStream | ||
| } from '../util/compat.js'; | ||
| import { compressionRegistry } from './compression.js'; | ||
|
|
||
| /** @ignore */ export type FromArg0 = ArrowJSONLike; | ||
| /** @ignore */ export type FromArg1 = PromiseLike<ArrowJSONLike>; | ||
|
|
@@ -352,12 +353,24 @@ abstract class RecordBatchReaderImpl<T extends TypeMap = any> implements RecordB | |
| return this; | ||
| } | ||
|
|
||
| protected _loadRecordBatch(header: metadata.RecordBatch, body: any) { | ||
| protected _loadRecordBatch(header: metadata.RecordBatch, body: Uint8Array) { | ||
| if (header.compression) { | ||
| const codec = compressionRegistry.get(header.compression); | ||
| if (codec?.decode) { | ||
| // TODO: does this need to be offset by 8 bytes? Since the uncompressed length is | ||
| // written in the first 8 bytes: | ||
| // https://github.com/apache/arrow/blob/1fc251f18d5b48f0c9fe8af8168237e7e6d05a45/format/Message.fbs#L59-L65 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confusing that here the uint8array is the "body" but it might correspond to the "buffer" in that spec. I'm not sure off hand which is actually here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started digging into this although being unfamiliar to arrow and zstd both it has been a bit of a struggle. The first 8 bytes are not part of the compressed section. You can see it corrected for in other places like here: https://github.com/apache/arrow/pull/9137/files#diff-38c3da9d6c7d521df83f59ee7d8cfced00951e8dcd38f39c7c36d26b6feee3d6R75 If you encode something with ZSTD you'll see it starts with This within the body here is: Now there's another issue where which is there appears to be padding at the end and between the fields which I'm very uncertain on how to detect besides brute forcing in reverse between the LZ4 header and the end.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for doing some digging! This PR isn't currently at the top of my list of things to work on, but would love for this support to be added to arrow JS! |
||
| body = codec.decode(body); | ||
| } else { | ||
| throw new Error('Record batch is compressed but codec not found'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this error include the codec ID? |
||
| } | ||
| } | ||
|
|
||
| const children = this._loadVectors(header, body, this.schema.fields); | ||
| const data = makeData({ type: new Struct(this.schema.fields), length: header.length, children }); | ||
| return new RecordBatch(this.schema, data); | ||
| } | ||
| protected _loadDictionaryBatch(header: metadata.DictionaryBatch, body: any) { | ||
| protected _loadDictionaryBatch(header: metadata.DictionaryBatch, body: Uint8Array) { | ||
| const { id, isDelta } = header; | ||
| const { dictionaries, schema } = this; | ||
| const dictionary = dictionaries.get(id); | ||
|
|
@@ -370,7 +383,7 @@ abstract class RecordBatchReaderImpl<T extends TypeMap = any> implements RecordB | |
| } | ||
| return dictionary.memoize(); | ||
| } | ||
| protected _loadVectors(header: metadata.RecordBatch, body: any, types: (Field | DataType)[]) { | ||
| protected _loadVectors(header: metadata.RecordBatch, body: Uint8Array, types: (Field | DataType)[]) { | ||
| return new VectorLoader(body, header.nodes, header.buffers, this.dictionaries).visitMany(types); | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intended to only be registered in tests, and users need to declare the dependency and register in their own applications if they want?
We should include code snippets and instructions for how to do that for each codec needed to support the full Arrow spec. The error message could even link to the docs on exactly how to enable a given compression codec.