Skip to content

feat: Decode dictionary delta batches #955

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paleolimbot merged 3 commits into from
Oct 16, 2026
Merged

feat: Decode dictionary delta batches #955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
827fb28
feat(ipc): decode dictionary delta batches
rustyconover Oct 4, 2026
f0544fa
Merge remote-tracking branch 'origin/main' into fix/pr955-review
rustyconover Oct 13, 2026
544b501
fix(ipc): reject overflowing dictionary deltas
rustyconover Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 75 additions & 7 deletions src/nanoarrow/ipc/decoder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,16 +337,82 @@ static ArrowErrorCode ArrowIpcDictionaryReplace(struct ArrowIpcDictionary* dicti
return NANOARROW_OK;
}

// Point dictionaries of dst (and its children) at those of src, which reflect
// the most recent value of any nested dictionaries.
static ArrowErrorCode ArrowIpcArraySetDictionaries(struct ArrowArray* dst,
const struct ArrowArray* src) {
if (src->dictionary != NULL) {
NANOARROW_DCHECK(dst->dictionary != NULL);
if (dst->dictionary->release != NULL) {
ArrowArrayRelease(dst->dictionary);
}
NANOARROW_RETURN_NOT_OK(ArrowArrayCloneShared(src->dictionary, dst->dictionary));
}

for (int64_t i = 0; i < src->n_children; i++) {
NANOARROW_RETURN_NOT_OK(
ArrowIpcArraySetDictionaries(dst->children[i], src->children[i]));
}

return NANOARROW_OK;
}

// Append the values of current and then value into combined, which must have been
// initialized from array_view. array_view is used as scratch space to view current
// and value.
static ArrowErrorCode ArrowIpcDictionaryConcatenate(struct ArrowArray* combined,
const struct ArrowArray* current,
const struct ArrowArray* value,
struct ArrowArrayView* array_view,
struct ArrowError* error) {
if (current->length > INT64_MAX - value->length) {
ArrowErrorSet(error,
"Expected dictionary length plus delta length to fit in int64 "
"but found %" PRId64 " and %" PRId64,
current->length, value->length);
return EOVERFLOW;
}

NANOARROW_RETURN_NOT_OK_WITH_ERROR(ArrowArrayStartAppending(combined), error);
NANOARROW_RETURN_NOT_OK_WITH_ERROR(
ArrowArrayReserve(combined, current->length + value->length), error);
Comment on lines +377 to +378

@paleolimbot paleolimbot Oct 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Codex kindly reminded me here that the two lengths here could overflow, so somebody who has run out of all of the normal ways to make software crash could provide huge null-typed dictionaries and make this unpredictable/crash.

I think we have plenty of places already where we assume two lengths won't overflow an int64; however, if this is easy to fix it would be good to do it here.

@rustyconover rustyconover Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 544b501. ArrowIpcDictionaryConcatenate() now checks current->length > INT64_MAX - value->length before adding the lengths, returning EOVERFLOW with both lengths in the error message.

The regression test uses null dictionaries to cover INT64_MAX + 1 and 1 + INT64_MAX through both dictionary decoding APIs. It also checks that rejection preserves the current dictionary and that subsequent replacement and delta decoding still work. With the guard removed, the test reproduces the signed overflow under UBSan; with the fix, it passes.

Also merged the latest main (ea215b0) as requested, triggering fresh CI.

Built and tested on aarch64 Linux EC2, Debug with ZSTD/LZ4:

  • Arrow C++ 24.0.0: 857 tests passed, 13 skipped, no failures.
  • Without Arrow C++, with UBSan: all 387 tests passed (serial CTest).
  • Decoder suite under Valgrind: 301 passed, 10 skipped; zero errors and no leaks.

Formatting checked with clang-format 16.0.6.


NANOARROW_RETURN_NOT_OK(ArrowArrayViewSetArray(array_view, current, error));
NANOARROW_RETURN_NOT_OK(
ArrowArrayAppendStorageFromArrayView(combined, array_view, error));
NANOARROW_RETURN_NOT_OK(ArrowArrayViewSetArray(array_view, value, error));
NANOARROW_RETURN_NOT_OK(
ArrowArrayAppendStorageFromArrayView(combined, array_view, error));

NANOARROW_RETURN_NOT_OK_WITH_ERROR(ArrowIpcArraySetDictionaries(combined, value),
error);
return ArrowArrayFinishBuildingDefault(combined, error);
}

// On success, value is released and the dictionary's current value is replaced
// with the concatenation of its previous value and value. Previously exported
// clones of the dictionary keep referencing the previous value.
static ArrowErrorCode ArrowIpcDictionaryAppend(struct ArrowIpcDictionary* dictionary,
struct ArrowArray* value,
struct ArrowArrayView* array_view,
struct ArrowError* error) {
if (dictionary->current_value.release != NULL &&
dictionary->current_value.length != 0) {
ArrowErrorSet(error, "Dictionary concatenation is not yet supported");
return ENOTSUP;
if (dictionary->current_value.release == NULL ||
dictionary->current_value.length == 0) {
return ArrowIpcDictionaryReplace(dictionary, value, error);
}

NANOARROW_RETURN_NOT_OK(ArrowIpcDictionaryReplace(dictionary, value, error));
struct ArrowArray combined;
NANOARROW_RETURN_NOT_OK(ArrowArrayInitFromArrayView(&combined, array_view, error));
int result = ArrowIpcDictionaryConcatenate(&combined, &dictionary->current_value, value,
array_view, error);
if (result != NANOARROW_OK) {
ArrowArrayRelease(&combined);
return result;
}

// On failure, ArrowIpcDictionaryReplace() releases combined
NANOARROW_RETURN_NOT_OK(ArrowIpcDictionaryReplace(dictionary, &combined, error));
ArrowArrayRelease(value);
return NANOARROW_OK;
}

Expand Down Expand Up @@ -2824,13 +2890,15 @@ static ArrowErrorCode ArrowIpcDecoderDecodeDictionaryInternal(
}

if (decoder->dictionary->is_delta) {
result = ArrowIpcDictionaryAppend(dictionary, &tmp, error);
result = ArrowIpcDictionaryAppend(dictionary, &tmp, array_view, error);
} else {
result = ArrowIpcDictionaryReplace(dictionary, &tmp, error);
}

if (result != NANOARROW_OK) {
ArrowArrayRelease(&tmp);
if (tmp.release != NULL) {
ArrowArrayRelease(&tmp);
}
return result;
}

Expand Down
Loading
Loading