feat: DH-22441: allow Dictionary encoding for Barrage and Arrow clients by lbooker42 · Pull Request #8187 · deephaven/deephaven-core

lbooker42 · 2026-06-30T20:29:49Z

No description provided.

…ibility.

github-actions · 2026-06-30T20:30:05Z

Deploying docs previews for `6813a27` (available for 14 days)

Copilot

Pull request overview

Enables Apache Arrow Dictionary Encoding support in Deephaven’s Barrage / Arrow Flight wire paths, adding both writer-side emission of DictionaryBatch messages (plus dict-index columns in RecordBatch) and reader-side consumption of DictionaryBatch messages to decode dictionary-encoded columns.

Changes:

Add dictionary-encoded column support end-to-end: schema generation (DictionaryEncoding), writer emission (DictionaryBatch + index buffers), and reader decoding (dictionary registry + index expansion).
Extend Barrage/Arrow readers to process DictionaryBatch IPC messages and wire dictionary registries into chunk readers.
Add coverage across unit / integration tests and update Barrage schema documentation with a dictionary-encoding example and guidance.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
server/jetty/src/test/java/io/deephaven/server/jetty/BarrageChunkFactoryTest.java	Adds Arrow Flight interop test verifying dict-encoded download behavior.
extensions/barrage/src/test/java/io/deephaven/extensions/barrage/util/BarrageUtilTest.java	Adds schema-level tests for dictionary encodings and id assignment.
extensions/barrage/src/test/java/io/deephaven/extensions/barrage/chunk/DictionaryWriterStateTest.java	Unit tests for per-stream dictionary state (delta + stable indices).
extensions/barrage/src/test/java/io/deephaven/extensions/barrage/chunk/DictionaryWriterRegistryTest.java	Tests writer registry reuse and delta tracking.
extensions/barrage/src/test/java/io/deephaven/extensions/barrage/chunk/DictionaryReaderRegistryTest.java	Tests reader registry update semantics (replace/delta/ids/nulls).
extensions/barrage/src/test/java/io/deephaven/extensions/barrage/chunk/DictionaryChunkWriterTest.java	Tests boxing semantics (notably NaN canonicalization) for dictionary keys.
extensions/barrage/src/test/java/io/deephaven/extensions/barrage/chunk/BarrageColumnRoundTripTest.java	Adds dictionary encoding round-trip + overflow tests.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/util/BarrageUtil.java	Adds dict encoding inference from schema + dict-id assignment in schema creation; removes prior hard rejection of dictionary fields.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/util/BarrageMessageReaderImpl.java	Adds `DictionaryBatch` handling and per-stream dictionary registry wiring for decoding.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/util/ArrowToTableConverter.java	Adds `DictionaryBatch` IPC handling for Arrow-to-table conversion.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/ColumnEncoding.java	Expands dictionary encodings to explicit index-width variants + helper predicate.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/DictionaryWriterState.java	Implements per-dictionary-id state tracking (value→index + deltas).
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/DictionaryWriterRegistry.java	Tracks per-stream dictionary states and associated value writers.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/DictionaryReaderRegistry.java	Stores decoded per-id dictionary values for expansion during reads.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/DictionaryChunkWriter.java	Implements dict index serialization and delta-values chunk construction.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/DictionaryChunkReader.java	Implements dict index expansion into Deephaven chunks using registry contents.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/DefaultChunkWriterFactory.java	Creates `DictionaryChunkWriter` when `Field` has `DictionaryEncoding`.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/chunk/DefaultChunkReaderFactory.java	Creates `DictionaryChunkReader` when `Field` has `DictionaryEncoding` and wires dict registries/readers.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/BarrageMessageWriterImpl.java	Emits `DictionaryBatch` messages before each `RecordBatch` and wires writer registry/state.
docs/groovy/how-to-guides/data-import-export/barrage-schema.md	Documents how to apply dictionary encoding via `BARRAGE_SCHEMA_ATTRIBUTE` with cautions.

Addressed co-pilot PR comments.

cpwright · 2026-07-01T19:53:27Z

+        // Dictionary encoding is identified by the presence of a DictionaryEncoding on the field, not by typeId.
+        final org.apache.arrow.vector.types.pojo.DictionaryEncoding dictEncoding = field.getDictionary();
+        if (dictEncoding != null) {
+            if (registry == null) {


maybe this is long enough to delegate to a function for just dictionaries.

cpwright · 2026-07-01T19:53:42Z

+            final ChunkReader<? extends WritableChunk<Values>> valuesReader =
+                    newReaderPojo(valuesTypeInfo, options, false, null, null);
+
+            if (dictValuesReadersOut != null) {


should we be checking for a dictValuesReader already; and using that instead?

cpwright · 2026-07-01T19:56:26Z

+     * skips unknown dictionary ids (no reader registered for that id). Callable by subclasses that receive
+     * {@code DictionaryBatch} messages through a different transport path (e.g. gRPC doPut streaming).
+     */
+    protected void applyDictionaryBatch(@NotNull final BarrageProtoUtil.MessageInfo mi) {


If we skip an unknown dictionary ID what happens? Do we end up with bogus values in those columns; or will we properly fail?

Code has changed since review.

On a mismatch, we throw a descriptive error: "IOException: No DictionaryBatch received for dictionary id X before RecordBatch that references it"

cpwright · 2026-07-01T19:58:48Z

        if (subset == null) {
            for (int srcPos = 0; srcPos < logicalSize; ++srcPos) {
-                final Object v = boxValue(source, srcPos);
+                final Object v = options.useDeephavenNulls()


This is doing a dispatch/lookup per position. We cannot have that kind of per-cell virtual call.

cpwright · 2026-07-02T00:35:35Z

+     * true, callers pass the boxed null sentinel (e.g. {@code Integer.valueOf(QueryConstants.NULL_INT)}) as a real
+     * dictionary entry instead.
+     */
+    int indexFor(@NotNull Object value);


I would make this operate on a chunk; and then we might not actually need to expose getDeltaValues (or if we do we could expose it as a chunk to write).

cpwright · 2026-07-02T00:40:20Z

+                encodings.put(field.getName(), encoding);
+            } else if (field.getDictionary() != null) {
+                final org.apache.arrow.vector.types.pojo.DictionaryEncoding dict = field.getDictionary();
+                ColumnEncoding encoding = ColumnEncoding.DICTIONARY_ENCODED_INT32;


Maybe make this final and don't overwrite it sometimes.

cpwright · 2026-07-02T00:43:05Z

-                clientRemovedRows = null; // we'll send full subscriptions the full removed set
-
                if (keyspaceViewport != null) {
+                    // Growing-to-full subscriptions are still materializing rows on the client side. Restrict


Is this a bug we previously had? Should we test/fix it separately so we have the option of cherry-picking it?

Yes, existing bug identified through the new tests. Will make a separate fix for this.

cpwright · 2026-07-02T00:44:44Z

+                    final List<DefensiveDrainable> messages = getInputStream(this, 0, 0, actualBatchSize, metadata,
+                            BarrageMessageWriterImpl.this::appendAddColumns);
+                    for (final DefensiveDrainable msg : messages) {
+                        bytesWritten.add(msg.available());
+                        visitor.accept(msg);
+                    }
+                    final DictionaryWriterRegistry dictRegistry = dictionaryRegistry();
+                    if (dictRegistry != null) {
+                        dictRegistry.resetDeltas();
+                    }


Helper for the duplicated code also around 416?

cpwright · 2026-07-02T00:50:13Z


                final ChunkWriter.DrainableColumn drainableColumn;
-                if (numElements == 0) {
+                if (mcd.chunkListWriter.writer() instanceof DictionaryChunkWriter && dictionaryRegistry != null) {


If you have a DictionaryChunkWriter and no registry that seems like it should be an error to me.

cpwright · 2026-07-02T11:22:09Z

+}
+
+
+final class ObjectDictionaryIndexKernel implements DictionaryIndexKernel {


I do really think this should be replicated; and am not a fan of this pattern where we have the package private classes jammed into one file.

cpwright · 2026-07-02T11:24:02Z

+    }
+
+    private static WritableColumnSource<?> newSource(@NotNull final ChunkType chunkType) {
+        switch (chunkType) {


This seems like a hole in the io.deephaven.engine.table.impl.sources.ArrayBackedColumnSource#getMemoryColumnSource(long, java.lang.Class, java.lang.Class<?>) constructors; and that we should probably add this method there.

lbooker42 added 3 commits June 30, 2026 13:23

Initial commit of phase 1: snapshot support, barrage and arrow compat…

68f1772

…ibility.

Final changes for phase 1

833a9bc

More changes for phase 1

b90781b

lbooker42 self-assigned this Jun 30, 2026

lbooker42 requested a review from Copilot June 30, 2026 20:30

lbooker42 added DocumentationNeeded ReleaseNotesNeeded Release notes are needed labels Jun 30, 2026

Copilot started reviewing on behalf of lbooker42 June 30, 2026 20:30 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

lbooker42 added 5 commits July 1, 2026 10:26

Initial commit of phase 2: subscription support

a0e8bc6

Added explicit tests for R/Go/Cpp run-end and dictionary encoding.

85439e9

Doc update.

f82a031

Doc update.

e951c44

Addressed co-pilot PR comments.

GWT reading and tests.

120534d

cpwright reviewed Jul 2, 2026

View reviewed changes

Some optimizations, changes to cpp client.

c5ed92a

cpwright reviewed Jul 2, 2026

View reviewed changes

Lots more changes. Still messy.

6813a27

		}


		final class ObjectDictionaryIndexKernel implements DictionaryIndexKernel {

Uh oh!

Conversation

lbooker42 commented Jun 30, 2026

Uh oh!

github-actions Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying docs previews for 6813a27 (available for 14 days)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jun 30, 2026 •

edited

Loading

Deploying docs previews for `6813a27` (available for 14 days)