adriangb opened a new pull request, #23496:
URL: https://github.com/apache/datafusion/pull/23496

   ## Which issue does this PR close?
   
   - Part of #23494 (reference / end-state, not intended to merge as-is).
   
   ## Rationale for this change
   
   This **draft** shows the complete end state of #23494 in one place: every 
built-in `ExecutionPlan`, all five `FileSource` scans, all three file sinks, 
and `MemorySourceConfig` moved onto the `try_to_proto` / `try_from_proto` 
hooks, with the central `downcast_ref` chain deleted. It is a reference for the 
incremental PRs — those land one plan/family at a time off #23495; this is not 
meant to be merged directly.
   
   ## What changes are included in this PR?
   
   - The ctx + hooks foundation (== #23495) plus the typed function-codec ctx 
methods and a scalar-subquery-results scope.
   - **~35 plans migrated:** projection, filter, all five joins, unions, sorts 
(incl. `SortPreservingMergeExec`), limits, repartition, both coalesces, unnest, 
explain, analyze, cooperative, buffer, placeholder, empty, coalesce-batches, 
`AggregateExec` (UDAF), both window execs (UDWF), `AsyncFuncExec`, 
`ScalarSubqueryExec`.
   - **DataSource family:** a `try_to_proto` hook on the 
`DataSource`/`FileSource` traits + the `FileScanConfig` serde ported into 
`datafusion-datasource`; csv/json/arrow/avro/parquet sources + 
`MemorySourceConfig`. `DataSourceExec` now serializes purely by delegating 
`DataSource → FileSource` — no downcast in the central encode path.
   - **DataSink family:** the mirror — a `DataSink` hook + `FileSinkConfig` 
serde port + the three file sinks.
   - The 58 now-dead per-plan `PhysicalPlanNodeExt` methods and the 
`try_from_data_source_exec` / `try_from_data_sink_exec` dispatchers are 
deleted; `mod.rs` shrinks by ~1900 lines.
   
   **Left as typed dispatch (by design):** `GenerateSeries`/`LazyMemoryExec` 
(their generator types live above physical-plan — a hook can't name them 
without a crate cycle) and `Extension` (the terminal third-party fallback).
   
   Notable findings baked in: enum conversions are inlined as **by-name** 
matches (proto vs. `datafusion_common` numbering differs); the encode dispatch 
resolves `downcast_delegate()` before the hook (a wrapper-plan bug this 
caught); `Partitioning`/`PhysicalSortExpr`/`Schema`/`ScalarValue` all serialize 
from `datafusion-proto-models`/`-common` with no `datafusion-proto`-only 
helper; no migrated plan needs the raw `PhysicalExtensionCodec`.
   
   ## Are these changes tested?
   
   Yes — the full `proto_integration` roundtrip suite passes (193 tests incl. 
TPC-H, with `avro`/`parquet`), plus new scan/sink roundtrip tests 
(`roundtrip_{csv,json,avro}_scan`) that exercise the new hook paths. `clippy -D 
warnings` + `fmt` clean; the workspace builds.
   
   ## Are there any user-facing changes?
   
   Yes — a documented breaking `api change`: the per-plan `PhysicalPlanNodeExt` 
serialization methods and the `TryFromProto<&protobuf::{Json,Csv,Parquet}Sink>` 
impls are removed (pub-for-proto scaffolding from #21929, released in 54.0.0). 
The load-bearing public API (`AsExecutionPlan`, `PhysicalExtensionCodec`, 
`PhysicalProtoConverterExtension`) is unchanged, and the wire format is 
byte-for-byte identical. An Upgrade Guide note will accompany the incremental 
removals.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to