fix: preserve Parquet sort pushdown across proto roundtrips

[jayshrivastava]

Which issue does this PR close?

• Informs Destructure proto hooks for the Parquet source and sink #24620.

Rationale for this change

reverse_row_groups and sort_order_for_reorder are not preserved across proto round trips, so those optimizations are lost.

What changes are included in this PR?

In try_to_proto and try_from_proto in ParquetSource, serialize those fields.

What is the testing strategy for this PR?

New unit test roundtrip_parquet_exec_with_sort_pushdown

[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-parquet v55.0.0 (current)
       Built [  54.678s] (current)
     Parsing datafusion-datasource-parquet v55.0.0 (current)
      Parsed [   0.035s] (current)
    Building datafusion-datasource-parquet v55.0.0 (baseline)
       Built [  49.151s] (baseline)
     Parsing datafusion-datasource-parquet v55.0.0 (baseline)
      Parsed [   0.036s] (baseline)
    Checking datafusion-datasource-parquet v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.170s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [ 106.030s] datafusion-datasource-parquet
    Building datafusion-proto v55.0.0 (current)
       Built [  55.618s] (current)
     Parsing datafusion-proto v55.0.0 (current)
      Parsed [   0.018s] (current)
    Building datafusion-proto v55.0.0 (baseline)
       Built [  54.516s] (baseline)
     Parsing datafusion-proto v55.0.0 (baseline)
      Parsed [   0.019s] (baseline)
    Checking datafusion-proto v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.119s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [ 112.262s] datafusion-proto
    Building datafusion-proto-models v55.0.0 (current)
       Built [  25.497s] (current)
     Parsing datafusion-proto-models v55.0.0 (current)
      Parsed [   0.130s] (current)
    Building datafusion-proto-models v55.0.0 (baseline)
       Built [  25.441s] (baseline)
     Parsing datafusion-proto-models v55.0.0 (baseline)
      Parsed [   0.139s] (baseline)
    Checking datafusion-proto-models v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   1.632s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip

--- failure constructible_struct_adds_field: struct exhaustively constructible through public API adds field ---

Description:
A pub struct that could be exhaustively constructed with a literal using only public API has a new pub field, breaking existing exhaustive literals.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field ParquetScanExecNode.sort_order_for_reorder in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1974
  field ParquetScanExecNode.reverse_row_groups in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1976
  field ParquetScanExecNode.sort_order_for_reorder in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1974
  field ParquetScanExecNode.reverse_row_groups in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1976

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  54.530s] datafusion-proto-models

[adriangb]

@jayshrivastava fyi ci is red

[jayshrivastava]

Taking a look

[adriangb]

let's look at coverage and make sure these lines are covered by the query (since it's quite indirect, although I do like / prefer testing it from SQL)

[codecov-commenter]

Codecov Report

❌ Patch coverage is 42.00000% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.63%. Comparing base (20d1c56) to head (da45907).

Files with missing lines	Patch %	Lines
datafusion/proto-models/src/generated/pbjson.rs	0.00%	26 Missing ⚠️
datafusion/datasource-parquet/src/source.rs	87.50%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24930      +/-   ##
==========================================
- Coverage   81.64%   81.63%   -0.01%     
==========================================
  Files        1123     1123              
  Lines      410248   410298      +50     
  Branches   410248   410298      +50     
==========================================
+ Hits       334940   334958      +18     
- Misses      55617    55646      +29     
- Partials    19691    19694       +3     

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

[jayshrivastava]

87.5% codecov seems good. I assume we can ignore the files in /generated. It's complaining about coverage for if encode_expr or sort_exprs_try_to_proto fail. We handle both cases with ? right now.

[adriangb]

Thanks!