Skip to content

feat: add ComposedLogicalExtensionCodec - #2752

Merged
milenkovicm merged 8 commits into
apache:mainfrom
NoahKusaba:feat/composed-logical-codec
Nov 3, 2026
Merged

milenkovicm merged 8 commits into
apache:mainfrom
NoahKusaba:feat/composed-logical-codec

Conversation

@NoahKusaba

@NoahKusaba NoahKusaba commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

No issue yet. Follows the discussion on #2638 about how several table format codecs (Iceberg, Hudi, Delta, ...) can share one Ballista deployment.

Rationale for this change

DataFusion has ComposedPhysicalExtensionCodec but no logical counterpart. Without one, each format crate has to wrap the codec it's given and invent its own framing, and protobuf has no type identity, so a codec that "tries to decode" bytes another codec wrote can decode them into the wrong thing.

ComposedLogicalExtensionCodec is the logical counterpart, and routes the same way: every payload records the position of the codec that encoded it, and only that codec decodes it.

What changes are included in this PR?

  • ballista_core::serde::ComposedLogicalExtensionCodec, matching DataFusion's ComposedPhysicalExtensionCodec:
    • payloads use the same DataEncoderTuple { encoder_position, blob } layout;
    • on encode, codecs are tried in order and the first that succeeds is recorded;
    • on decode, only the codec at the recorded position is used (an empty payload is position 0).
  • Differences only in error reporting, not in the bytes written:
    • when no codec can encode a node, every codec's error is reported, not just the last;
    • a malformed payload or an out-of-range position is a configuration error, explaining that every node must compose the same codecs in the same order.
  • Docs: a "Combining Codecs" section in extending-components.md, pairing it with DataFusion's ComposedPhysicalExtensionCodec. It notes the shared limitation that only the first codec can encode a function (the trait's default sends functions by name), and that the physical codec doesn't route custom physical expressions, window functions or higher-order functions. Table scans, such as Iceberg's, need neither.

The module is self-contained, with a TODO to move it into datafusion-proto beside ComposedPhysicalExtensionCodec.

Are there any user-facing changes?

Yes, a new public ComposedLogicalExtensionCodec and docs. Nothing existing changes; no .proto files are touched.

The logical counterpart of DataFusion's ComposedPhysicalExtensionCodec:
tries each codec in turn, records the position of the one that encoded a
payload, and decodes it only with the codec at that position, so a codec
is never offered another codec's bytes.

Unlike the physical one, a codec that succeeds without writing anything
doesn't claim the node, so a later codec can still encode a function,
and a function no codec writes is still sent by name.
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Nov 3, 2026
@NoahKusaba
NoahKusaba marked this pull request as ready for review November 3, 2026 04:47
@NoahKusaba

Copy link
Copy Markdown
Contributor Author

cc @milenkovicm

@milenkovicm
milenkovicm self-requested a review November 3, 2026 09:23

@milenkovicm milenkovicm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks @NoahKusaba

@milenkovicm
milenkovicm added this pull request to the merge queue Nov 3, 2026
Merged via the queue into apache:main with commit a976c4c Nov 3, 2026
24 of 25 checks passed
@NoahKusaba
NoahKusaba deleted the feat/composed-logical-codec branch November 3, 2026 13:14
NoahKusaba added a commit to NoahKusaba/datafusion-ballista that referenced this pull request Nov 6, 2026
Depend on the Ballista crates at a commit that includes apache#2752 instead of
the crates beside it, and stop running the workflow on changes under
ballista/, so a dependency bump in Ballista can't break this build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants