feat(proto): thread expr encode/decode context into try_encode_expr / try_decode_expr

[adriangb]

Which issue does this PR close?

Closes#22920.

Alternative to #22922 — same goal, but plumbs the per-expr encode/decode context (from the #22418 hook machinery) instead of the raw PhysicalProtoConverterExtension.

Rationale for this change

#21807 introduced the DynamicFilterPhysicalExpr dedup pipeline so identical references on the wire reconstruct to one shared Arc<Inner> via expr_id cache keys, and #22011 hooked it through the SortExec / AggregateExec / HashJoinExec plan codecs.

The remaining gap is the expression-level extension path. In serialize_physical_expr_with_converter / parse_physical_expr_with_converter, the codec's try_encode_expr / try_decode_expr is reached only as a fallback after the built-in try_to_proto path and the ScalarFunctionExpr downcast — i.e. only for downstream-defined custom PhysicalExpr types. When such a codec serializes nested PhysicalExprNode fields inside its own blob, the only helper available today is the free serialize_physical_expr / parse_physical_expr, hardwired to DefaultPhysicalProtoConverter. So those nested exprs get expr_id: None on the wire and reconstruct as distinctInner allocations — heap-max updates from a SortExec never reach the wrapped reference.

What changes are included in this PR?

The built-in expressions migrated under #22418 already receive a PhysicalExprEncodeCtx / PhysicalExprDecodeCtx in their try_to_proto / try_from_proto hooks. Those context objects bundle the dedup-aware converter plus the active schema and task context, and hide PhysicalProtoConverterExtension / PhysicalExtensionCodec from the expression author entirely.

This PR hands the same context to the expr-level codec methods:

fntry_decode_expr(&self,buf:&[u8],inputs:&[Arc<dynPhysicalExpr>],ctx:&PhysicalExprDecodeCtx<'_>,// new) -> Result<Arc<dynPhysicalExpr>>;fntry_encode_expr(&self,node:&Arc<dynPhysicalExpr>,buf:&mutVec<u8>,ctx:&PhysicalExprEncodeCtx<'_>,// new) -> Result<()>;

A codec that embeds nested PhysicalExprNodes now decodes them with ctx.decode(..) and encodes them with ctx.encode_child(..), which:

• route through any active DeduplicatingProtoConverter / DeduplicatingDeserializer, so shared inner expressions cache-hit on expr_id; and
• carry the real schema and task context, so nested UDF / column references resolve against the actual registry.

Why the context, not the raw converter (cf. #22922)

Threading the bare PhysicalProtoConverterExtension still leaves the codec without the schema/registry, forcing it to fabricate a SessionContext::new() and hard-code a schema to call proto_to_physical_expr — an empty-registry footgun for any nested expr that references a UDF or column. Passing the existing Physical{Encode,Decode}Ctx avoids that, keeps the extension escape-hatch consistent with the per-expr proto hooks, and adds no third converter parameter to the codec API (the concern raised on #22922).

Are these changes tested?

Yes — extension_codec_expr_participates_in_deduplication builds a BinaryExpr whose left operand is a bare DynamicFilterPhysicalExpr and whose right operand is a custom WrapperExpr whose codec embeds the same dynamic filter inside its serialized blob (via ctx.encode_child). After a DeduplicatingProtoConverter roundtrip, an update() on the bare-side decoded filter is observed via current() on the wrapped-side filter, proving both refs back the same Inner. The codec needs no fabricated SessionContext — it decodes the nested expr through ctx.decode.

Are there any user-facing changes?

Yes — a breaking change for downstream codecs that override try_encode_expr / try_decode_expr: they must add the new ctx parameter (name it _ctx if the custom expr carries no nested PhysicalExprNodes). Codecs that only override the plan-level try_encode / try_decode are unaffected. Wire format is unchanged.

🤖 Generated with Claude Code

[github-actions]

Thank you for opening this pull request!

Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).

Details

 Cloning apache/main
Building datafusion-proto v54.0.0 (current)
Built [ 64.458s] (current)
Parsing datafusion-proto v54.0.0 (current)
Parsed [ 0.018s] (current)
Building datafusion-proto v54.0.0 (baseline)
Built [ 61.830s] (baseline)
Parsing datafusion-proto v54.0.0 (baseline)
Parsed [ 0.020s] (baseline)
Checking datafusion-proto v54.0.0 -> v54.0.0 (no change; assume patch)
Checked [ 0.298s] 223 checks: 222 pass, 1 fail, 0 warn, 30 skip
--- failure trait_method_parameter_count_changed: pub trait method parameter count changed ---
Description:
A trait method now takes a different number of parameters.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-item-signature
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.48.0/src/lints/trait_method_parameter_count_changed.ron
Failed in:
PhysicalExtensionCodec::try_decode_expr now takes 3 instead of 2 parameters, in file /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:4450
PhysicalExtensionCodec::try_encode_expr now takes 3 instead of 2 parameters, in file /home/runner/work/datafusion/datafusion/datafusion/proto/src/physical_plan/mod.rs:4469
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 128.098s] datafusion-proto

[codecov-commenter]

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.70%. Comparing base (5b65e70) to head (6a5fd41).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/proto/src/physical_plan/mod.rs	0.00%	2 Missing ⚠️
datafusion/proto/src/physical_plan/from_proto.rs	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #23733 +/- ##
========================================
Coverage 80.70% 80.70% ========================================
Files 1089 1089 Lines 368137 368280 +143 Branches 368137 368280 +143 ========================================
+ Hits 297121 297238 +117 - Misses 53311 53324 +13 - Partials 17705 17718 +13 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[zhuqi-lucas]

LGTM, thanks @adriangb !