Restore the From / TryFrom proto conversions dropped since 54.1.0

[adriangb]

Which issue does this PR close?

• Closes Restore the From / TryFrom proto conversions dropped since 54.1.0, and retire FromProto / TryFromProto #24019.

Rationale for this change

datafusion-proto 54.1.0 publishes 39 From / TryFrom impls converting between DataFusion types and their protobuf messages. On main all of them were replaced by the crate-local FromProto / TryFromProto traits introduced in #21929, so code written against the released version stops compiling:

let proto = protobuf::PartitionedFile::try_from(&file)?;   // no longer resolves on main
let frame = WindowFrame::try_from(proto_frame)?;           // no longer resolves on main

That was collateral damage from the orphan-rule workaround, not an intended API change. cargo-semver-checks has no lint for a removed hand-written trait impl, and the breaking_changes_detector workflow baselines against apache/main rather than the last release, so nothing flagged it.

What changes are included in this PR?

Each conversion moves to a crate that owns one side of it, and goes back to being a plain From / TryFrom — the shape 54.1.0 published. Error types are unchanged (FromProtoError decoding, ToProtoError encoding, DataFusionError for the datasource types).

Types	New home
PartitionedFile, FileRange, FileGroup, JsonSink, CsvSink, ParquetSink, FileSinkConfig	already moved by #24006 / #23781 — this PR just deletes the TryFromProto shims that delegated to them
WindowFrame, WindowFrameBound, WindowFrameUnits, MergeIntoClauseKind, NullTreatment	datafusion-expr, behind a new proto feature (optional datafusion-proto-common / datafusion-proto-models deps, mirroring datafusion-datasource)
UnnestOptions, TableReference, StringifiedPlan, JoinType, JoinConstraint, NullEquality, CsvOptions, JsonOptions, and the parquet options types	datafusion-proto-models, on the local proto type — their DataFusion side sits below that crate in the graph, the same arrangement datafusion-proto-common already uses for ScalarValue / Statistics
CsvFormatFactory, JsonFormatFactory, ParquetFormatFactory	datafusion-datasource-{csv,json,parquet}, behind each crate's existing proto feature
Column <-> protobuf::PhysicalColumn	datafusion-physical-expr; Column::try_to_proto / try_from_proto now go through it instead of building the message inline

TryFrom<&[PartitionedFile]> for protobuf::FileGroup needed one extra step. datafusion-datasource cannot host it — &T is #[fundamental] but [T] is not, so &[PartitionedFile] counts as foreign there (error[E0117]: slices are always foreign). But in datafusion-proto-models the self type is local, which is all the orphan rule needs, and staying generic over the element avoids naming PartitionedFile, which sits above that crate in the graph:

impl<T> TryFrom<&[T]> for protobuf::FileGroup
where
    for<'a> &'a T: TryInto<protobuf::PartitionedFile, Error = DataFusionError>,
{ ... }

The bound is satisfied by TryFrom<&PartitionedFile> for protobuf::PartitionedFile in datafusion-datasource, so protobuf::FileGroup::try_from(&files[..]) resolves for callers exactly as it did in 54.1.0 — zero-copy, and no free function.

Two items beyond the issue's checklist, both needed to reach zero implementors:

• the parquet options conversions (ParquetOptions, TableParquetOptions, ParquetColumnOptions, ParquetCdcOptions) — the issue's table undercounts file_formats.rs because they live in a private module, but trait impls are global, so they were public API too. They return as TryFrom; main had already made them fallible, so an exact restore of 54.1.0's infallible From isn't available.
• From<&protobuf::PhysicalColumn> for Column, which the issue's evidence table counts but no work item names.

Not restored, and worth calling out: From<protobuf::dml_node::Type> for WriteOp and its reverse. main replaced them with parse_write_op / serialize_write_op because MergeInto carries a payload a From impl cannot express. That is a separate, deliberate change.

Finally, convert.rs and convert_required_proto! are deleted. #21929 introduced FromProto / TryFromProto so the datafusion-proto-models extraction could land without relocating ~39 conversions at the same time, and flagged them there as "a known workaround, not the end state", with dropping them listed under Future work. With every conversion moved they have no implementors and no callers. Neither trait has ever shipped in a release, so they are removed outright rather than deprecated — there is nothing for downstream users to migrate off, and doing it now keeps the workaround out of the released API entirely.

Are these changes tested?

Yes.

• New datafusion/proto/tests/cases/public_conversions.rs coerces all 45 proto conversions in the touched crates to fn pointers (the 39 from 54.1.0 plus the ones added on main). This is the regression guard the issue asks for: it fails to compile when an impl is removed, and stays quiet when one merely moves between crates, which is exactly the case cargo-semver-checks cannot see.
• New round-trip tests next to the moved impls in datafusion-expr and datafusion-proto-models (window frames, table references, join enums, unnest options, stringified plans).
• The PartitionedFile tests move from datafusion-proto to datafusion-datasource, alongside the logic they cover; two that duplicated existing coverage there are dropped.
• Existing round-trip suites (roundtrip_logical_plan, roundtrip_physical_plan) pass unchanged, which is the real wire-format check.
• Every moved impl body was diffed against main: 22 are byte-identical modulo the trait rename, and the other 9 differ only by Self:: shorthand, error-type aliasing, and rustfmt reflow. No serialization logic changed.
• ./dev/rust_lint.sh, cargo machete, and the extended test suite all pass at HEAD. Also checked: datafusion-proto without parquet, datafusion-expr with proto off and --no-default-features, the format crates without proto, and json on both proto crates.

Are there any user-facing changes?

Yes, and they restore rather than break the released API.

• The 39 conversions removed since 54.1.0 compile again. Trait impls are global, so X::try_from(&proto) / proto.try_into() resolve regardless of which crate now hosts the impl — no import changes needed, and no upgrade-guide entry for the moves.
• One genuine delta remains: the parquet options conversions are TryFrom rather than 54.1.0's infallible From. That predates this PR — main had already made them fallible — but it is a real 54.1.0 -> 55.0.0 break and was undocumented, so it is now in the 55.0.0 upgrade guide with a migration snippet.
• FromProto / TryFromProto and convert_required_proto! are gone. Not a breaking change: they exist only on main and appear nowhere in 54.0.0 or 54.1.0.
• datafusion-expr gains an off-by-default proto feature. Additive.
• datafusion-proto-models gains a direct datafusion-common dependency (already present transitively) and two new public modules.

Keeping the api change label for the parquet options fallibility.

[Copilot]

Pull request overview

Restores standard protobuf From/TryFrom conversions by relocating implementations to crates that satisfy Rust’s orphan rules.

Changes:

• Moves expression, common-type, datasource, format-factory, and physical-column conversions to owning crates.
• Replaces the unsupported slice conversion with partitioned_files_to_proto.
• Adds migration documentation and compile-time API regression tests.

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
docs/source/library-user-guide/upgrading/55.0.0.md	Documents the slice-conversion migration.
datafusion/proto/tests/cases/roundtrip_logical_plan.rs	Uses restored standard conversions.
datafusion/proto/tests/cases/public_conversions.rs	Adds compile-time API guards, but misses six restored impls.
datafusion/proto/tests/cases/mod.rs	Registers the new tests.
datafusion/proto/src/physical_plan/to_proto.rs	Removes encoding shims and re-exports the slice helper.
datafusion/proto/src/physical_plan/from_proto.rs	Removes decoding shims and relocated tests.
datafusion/proto/src/logical_plan/to_proto.rs	Uses relocated standard conversions.
datafusion/proto/src/logical_plan/mod.rs	Updates logical-plan conversion calls.
datafusion/proto/src/logical_plan/from_proto.rs	Removes local decoding implementations.
datafusion/proto/src/logical_plan/file_formats.rs	Delegates format-option conversions to owning crates.
datafusion/proto/src/convert.rs	Documents the now-unused custom traits.
datafusion/proto/src/common.rs	Marks the custom conversion macro unused.
datafusion/proto/Cargo.toml	Enables expression protobuf support.
datafusion/proto-models/src/to_proto.rs	Adds common-type protobuf encoders.
datafusion/proto-models/src/lib.rs	Exposes conversion modules.
datafusion/proto-models/src/from_proto.rs	Adds common-type protobuf decoders.
datafusion/proto-models/Cargo.toml	Adds the common crate dependency.
datafusion/physical-expr/src/expressions/column.rs	Restores physical-column conversions.
datafusion/expr/src/proto.rs	Adds expression protobuf conversions and tests.
datafusion/expr/src/lib.rs	Registers the feature-gated conversion module.
datafusion/expr/Cargo.toml	Adds the optional proto feature.
datafusion/datasource/src/proto.rs	Adds the slice conversion function and tests.
datafusion/datasource/src/mod.rs	Makes the protobuf module public.
datafusion/datasource-parquet/src/file_format.rs	Moves Parquet factory encoding.
datafusion/datasource-json/src/file_format.rs	Moves JSON factory encoding.
datafusion/datasource-csv/src/file_format.rs	Moves CSV factory encoding.
Cargo.lock	Records dependency changes.

Suppressed comments (1)

datafusion/proto/tests/cases/public_conversions.rs:117

• This guard does not cover four restored file-format impls: encoding each of CsvFormatFactory, JsonFormatFactory, and ParquetFormatFactory, plus decoding ParquetCdcOptions. Their removal would therefore go undetected despite this test's stated goal of pinning every restored conversion. Please add assertions for all four.

fn file_format_option_conversions_are_std_traits() {

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 73.09469% with 233 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.07%. Comparing base (fc846dd) to head (783d5ad).
⚠️ Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/proto-models/src/from_proto.rs	70.64%	86 Missing and 10 partials ⚠️
datafusion/datasource-parquet/src/file_format.rs	55.33%	37 Missing and 9 partials ⚠️
datafusion/proto-models/src/to_proto.rs	75.93%	44 Missing and 1 partial ⚠️
datafusion/expr/src/proto.rs	82.17%	12 Missing and 11 partials ⚠️
datafusion/datasource-csv/src/file_format.rs	71.42%	1 Missing and 9 partials ⚠️
datafusion/datasource/src/proto.rs	82.50%	0 Missing and 7 partials ⚠️
datafusion/datasource-json/src/file_format.rs	87.50%	1 Missing ⚠️
datafusion/proto/src/logical_plan/from_proto.rs	83.33%	0 Missing and 1 partial ⚠️
datafusion/proto/src/logical_plan/mod.rs	80.00%	1 Missing ⚠️
datafusion/proto/src/logical_plan/to_proto.rs	83.33%	0 Missing and 1 partial ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24205      +/-   ##
==========================================
+ Coverage   81.05%   81.07%   +0.01%     
==========================================
  Files        1107     1109       +2     
  Lines      381574   381940     +366     
  Branches   381574   381940     +366     
==========================================
+ Hits       309281   309647     +366     
+ Misses      54034    53999      -35     
- Partials    18259    18294      +35     

☔ 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.

[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-datasource v54.1.0 (current)
       Built [  47.649s] (current)
     Parsing datafusion-datasource v54.1.0 (current)
      Parsed [   0.030s] (current)
    Building datafusion-datasource v54.1.0 (baseline)
       Built [  42.157s] (baseline)
     Parsing datafusion-datasource v54.1.0 (baseline)
      Parsed [   0.032s] (baseline)
    Checking datafusion-datasource v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.238s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  91.567s] datafusion-datasource
    Building datafusion-datasource-csv v54.1.0 (current)
       Built [  42.730s] (current)
     Parsing datafusion-datasource-csv v54.1.0 (current)
      Parsed [   0.011s] (current)
    Building datafusion-datasource-csv v54.1.0 (baseline)
       Built [  42.928s] (baseline)
     Parsing datafusion-datasource-csv v54.1.0 (baseline)
      Parsed [   0.012s] (baseline)
    Checking datafusion-datasource-csv v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.093s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  87.399s] datafusion-datasource-csv
    Building datafusion-datasource-json v54.1.0 (current)
       Built [  43.162s] (current)
     Parsing datafusion-datasource-json v54.1.0 (current)
      Parsed [   0.012s] (current)
    Building datafusion-datasource-json v54.1.0 (baseline)
       Built [  42.671s] (baseline)
     Parsing datafusion-datasource-json v54.1.0 (baseline)
      Parsed [   0.014s] (baseline)
    Checking datafusion-datasource-json v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.091s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  86.926s] datafusion-datasource-json
    Building datafusion-datasource-parquet v54.1.0 (current)
       Built [  48.324s] (current)
     Parsing datafusion-datasource-parquet v54.1.0 (current)
      Parsed [   0.030s] (current)
    Building datafusion-datasource-parquet v54.1.0 (baseline)
       Built [  47.775s] (baseline)
     Parsing datafusion-datasource-parquet v54.1.0 (baseline)
      Parsed [   0.032s] (baseline)
    Checking datafusion-datasource-parquet v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.146s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  98.360s] datafusion-datasource-parquet
    Building datafusion-expr v54.1.0 (current)
       Built [  28.455s] (current)
     Parsing datafusion-expr v54.1.0 (current)
      Parsed [   0.076s] (current)
    Building datafusion-expr v54.1.0 (baseline)
       Built [  26.868s] (baseline)
     Parsing datafusion-expr v54.1.0 (baseline)
      Parsed [   0.076s] (baseline)
    Checking datafusion-expr v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   1.248s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  57.795s] datafusion-expr
    Building datafusion-physical-expr v54.1.0 (current)
       Built [  29.019s] (current)
     Parsing datafusion-physical-expr v54.1.0 (current)
      Parsed [   0.048s] (current)
    Building datafusion-physical-expr v54.1.0 (baseline)
       Built [  28.455s] (baseline)
     Parsing datafusion-physical-expr v54.1.0 (baseline)
      Parsed [   0.048s] (baseline)
    Checking datafusion-physical-expr v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.332s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  59.032s] datafusion-physical-expr
    Building datafusion-physical-expr-common v54.1.0 (current)
       Built [  24.314s] (current)
     Parsing datafusion-physical-expr-common v54.1.0 (current)
      Parsed [   0.021s] (current)
    Building datafusion-physical-expr-common v54.1.0 (baseline)
       Built [  24.272s] (baseline)
     Parsing datafusion-physical-expr-common v54.1.0 (baseline)
      Parsed [   0.020s] (baseline)
    Checking datafusion-physical-expr-common v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.199s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  49.644s] datafusion-physical-expr-common
    Building datafusion-proto v54.1.0 (current)
       Built [  60.437s] (current)
     Parsing datafusion-proto v54.1.0 (current)
      Parsed [   0.018s] (current)
    Building datafusion-proto v54.1.0 (baseline)
       Built [  60.076s] (baseline)
     Parsing datafusion-proto v54.1.0 (baseline)
      Parsed [   0.019s] (baseline)
    Checking datafusion-proto v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.233s] 223 checks: 220 pass, 3 fail, 0 warn, 30 skip

--- failure declarative_macro_missing: macro_rules declaration removed or renamed ---

Description:
A `macro_rules!` declarative macro cannot be invoked by its prior name. The macro may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/reference/macros-by-example.html#path-based-scope
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.49.0/src/lints/declarative_macro_missing.ron

Failed in:
  macro convert_required_proto, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/5a771c5ef589a0448dc9180a70436770ab8dc71b/datafusion/proto/src/common.rs:35

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.49.0/src/lints/module_missing.ron

Failed in:
  mod datafusion_proto::convert, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/5a771c5ef589a0448dc9180a70436770ab8dc71b/datafusion/proto/src/convert.rs:18

--- failure trait_missing: pub trait removed or renamed ---

Description:
A publicly-visible trait cannot be imported by its prior path. A `pub use` may have been removed, or the trait itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.49.0/src/lints/trait_missing.ron

Failed in:
  trait datafusion_proto::convert::TryFromProto, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/5a771c5ef589a0448dc9180a70436770ab8dc71b/datafusion/proto/src/convert.rs:41
  trait datafusion_proto::TryFromProto, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/5a771c5ef589a0448dc9180a70436770ab8dc71b/datafusion/proto/src/convert.rs:41
  trait datafusion_proto::convert::FromProto, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/5a771c5ef589a0448dc9180a70436770ab8dc71b/datafusion/proto/src/convert.rs:35
  trait datafusion_proto::FromProto, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/5a771c5ef589a0448dc9180a70436770ab8dc71b/datafusion/proto/src/convert.rs:35

     Summary semver requires new major version: 3 major and 0 minor checks failed
    Finished [ 122.155s] datafusion-proto
    Building datafusion-proto-models v54.1.0 (current)
       Built [  24.863s] (current)
     Parsing datafusion-proto-models v54.1.0 (current)
      Parsed [   0.130s] (current)
    Building datafusion-proto-models v54.1.0 (baseline)
       Built [  24.930s] (baseline)
     Parsing datafusion-proto-models v54.1.0 (baseline)
      Parsed [   0.130s] (baseline)
    Checking datafusion-proto-models v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   1.691s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  52.823s] datafusion-proto-models

[Copilot]

Pull request overview

Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (2)

datafusion/proto/tests/cases/public_conversions.rs:90

• This compile-time regression guard omits both restored MergeIntoClauseKind conversions. Removing either impl would therefore still leave this test suite compiling, despite this file's stated purpose of pinning every restored standard-trait conversion. Add both directions to the guard.

fn window_frame_conversions_are_std_traits() {

datafusion/proto/tests/cases/public_conversions.rs:124

• This guard misses four relocated public impls: encoding each CSV/JSON/Parquet format factory, and decoding ParquetCdcOptions. Those APIs can regress without failing the test. Please assert their exact signatures alongside the other file-format conversions.

fn file_format_option_conversions_are_std_traits() {

[Copilot]

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (2)

datafusion/datasource-parquet/src/file_format.rs:703

• This newly added struct literal is not rustfmt-formatted: its fields are aligned with the literal rather than nested, and several expressions below remain unreflowed. The required cargo fmt --all check will rewrite this whole block; please run it before merging.

        protobuf::TableParquetOptions {
        global: Some(protobuf::ParquetOptions {

datafusion/proto-models/src/from_proto.rs:128

• This new line is not rustfmt-formatted, so the repository's required cargo fmt --all check will rewrite it. Please format the file before merging.

                        analyzer_name:analyzer_name.clone()