Skip to content

fix(proto): check integer conversions in common options and constraints - #24726

Open
buraksenn wants to merge 1 commit into
apache:mainfrom
buraksenn:24170-fallible-conversions
Open

fix(proto): check integer conversions in common options and constraints#24726
buraksenn wants to merge 1 commit into
apache:mainfrom
buraksenn:24170-fallible-conversions

Conversation

@buraksenn

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

#24483 added checked integer conversions across logical and physical plan serialization. Common protobuf conversions for file-format options and constraints still used unchecked casts, allowing oversized values to silently truncate on 32-bit targets.

What changes are included in this PR?

The protobuf wire format is unchanged.

Are these changes tested?

Yes. Existing option round-trip tests pass, and tests cover the fallible public conversion contracts and malformed constraints.

Are there any user-facing changes?

Oversized protobuf values now return an error instead of silently truncating.

Some public From conversions are now TryFrom, so downstream callers must handle the resulting error. This migration is documented in the 56.0.0 upgrade guide.

@github-actions github-actions Bot added documentation Improvements or additions to documentation proto Related to proto crate datasource Changes to the datasource crate labels Aug 27, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.62500% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.44%. Comparing base (2a7a1e3) to head (ed0facc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/proto-common/src/from_proto/mod.rs 69.23% 7 Missing and 21 partials ⚠️
datafusion/proto-models/src/from_proto.rs 63.63% 4 Missing and 20 partials ⚠️
datafusion/proto/src/logical_plan/file_formats.rs 0.00% 0 Missing and 2 partials ⚠️
datafusion/proto/src/logical_plan/mod.rs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24726      +/-   ##
==========================================
- Coverage   81.44%   81.44%   -0.01%     
==========================================
  Files        1120     1120              
  Lines      401605   401871     +266     
  Branches   401605   401871     +266     
==========================================
+ Hits       327098   327287     +189     
- Misses      55348    55366      +18     
- Partials    19159    19218      +59     

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate documentation Improvements or additions to documentation proto Related to proto crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proto decode silently truncates limits and sizes to usize on 32-bit targets

2 participants