fix: validate parquet.coerce_int96 when it is set

[akashjainn]

Which issue does this PR close?

• Part of The config values aren't checked #17498. This covers datafusion.execution.parquet.coerce_int96 only.

Rationale for this change

The option is stored as a string, so a bad value is accepted and only fails later:

-- Succeeds, and SHOW reports 'nanos'.
SET datafusion.execution.parquet.coerce_int96 = 'nanos';

-- Fails here, on the first table that touches Parquet:
-- Invalid or Unsupported Configuration: Unknown or unsupported parquet coerce_int96: nanos. Valid values are: ns, us, ms, and s.
CREATE EXTERNAL TABLE t STORED AS PARQUET LOCATION 'data.parquet';

The same thing happens through CREATE EXTERNAL TABLE ... OPTIONS ('format.coerce_int96' 'nanos'). If the table has an explicit schema the string is parsed again in ParquetSource with unwrap(), so that path panics instead.

#20253 fixed this the same way and was reviewed, but went stale and was closed. This picks it up with the review applied.

What changes are included in this PR?

• DFTimeUnit in parquet_config.rs: ns, us, ms, s, parsed case-insensitively and displayed as those strings, so SHOW, information_schema and the proto encoding are unchanged. The ConfigField impls follow DFParquetStatistics, so a rejected value leaves the previous one in place and an unset option stays unset.
• ParquetOptions.coerce_int96 becomes Option<DFTimeUnit>, and ParquetFormat::coerce_int96() / with_coerce_int96() use the typed value.
• parse_coerce_int96_string in datasource-parquet is removed, along with the unwrap() that called it. From<DFTimeUnit> for arrow::datatypes::TimeUnit replaces it.
• Proto conversion parses on the way in, so a bad value in a serialized plan is an error rather than a panic.
• The option's docstring said "If true, ..." as if it were a boolean. It now says what the value is and lists the valid values. configs.md and information_schema.slt carry that text.

I kept the Option. #20253 looked at dropping it, but None means "do not coerce" and coerce_int96_tz only applies when coerce_int96 is set, so None and Some(ns) are not the same.

What is the testing strategy for this PR?

• set_variable.slt: set, show, one rejected value with its full error text, and that the setting is unchanged afterwards.
• test_parquet_coerce_int96_validation in config.rs: valid values, a rejected value keeps the previous one, an unset value stays unset, nested keys are rejected.
• time_unit_parses_and_displays in parquet_config.rs.
• test_invalid_parquet_coerce_int96_rejected_from_proto in proto-common, and the two existing round-trip tests now use the typed value.
• I also checked CREATE EXTERNAL TABLE ... OPTIONS ('format.coerce_int96' 'nanos') fails at CREATE time with the new message. The existing coerce_int96 data tests in parquet.slt and the core Parquet tests pass unchanged.

Are there any user-facing changes?

Yes. An invalid datafusion.execution.parquet.coerce_int96 is rejected by SET and by table options with a message, instead of being accepted and failing (or panicking) at scan time.

This changes the public field ParquetOptions::coerce_int96 from Option<String> to Option<DFTimeUnit>, and the two ParquetFormat accessors with it, so it needs the api change label. SQL and ConfigOptions::set users are not affected.

I searched GitHub for code that assigns the field directly. These will need a one-line change on upgrade, from Some("us".to_string()) to Some(DFTimeUnit::Microsecond) or Some("us".parse()?):

• apache/datafusion-comet: native/core/src/parquet/parquet_exec.rs hardcodes "us" for Spark, plus one test assertion.
• delta-io/delta-rs: crates/core/src/datafile/properties.rs uses get_or_insert_with(|| "us".to_string()).
• lakehq/sail: crates/sail-session/src/session_factory/mod.rs assigns "us", and crates/sail-data-source/src/formats/parquet/options.rs copies the field through its own Option<String>.
• DoumanAsh/mishka: src/datafusion.rs assigns an owned string.

Code that already uses DFParquetMetadata::with_coerce_int96 or the opener (apache/auron does) takes an arrow TimeUnit and is unaffected.

I can add a note to the 56.0.0 upgrade guide matching the ExecutionOptions::time_zone entry from #25788 if that's wanted.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 91.50327% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.85%. Comparing base (0d79252) to head (920e9ab).
⚠️ Report is 36 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/common/src/parquet_config.rs	91.17%	6 Missing ⚠️
datafusion/proto-models/src/from_proto.rs	37.50%	3 Missing and 2 partials ⚠️
datafusion/datasource-parquet/src/file_format.rs	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25628      +/-   ##
==========================================
+ Coverage   82.80%   82.85%   +0.05%     
==========================================
  Files        1149     1151       +2     
  Lines      454146   456125    +1979     
  Branches   454146   456125    +1979     
==========================================
+ Hits       376035   377923    +1888     
+ Misses      55260    55245      -15     
- Partials    22851    22957     +106     

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

[subhramit]

https://dontpastetheai.com/ may be useful for you (checked your prior GitHub contributions as well)

[kosiew]

@akashjainn
Can you resolve the merge conflicts?
I would love to review this after.

[akashjainn]

@kosiew I have resolved merge conflicts! Would love a review when you get a chance. Thanks!

[kosiew]

@akashjainn,

Thanks for working on this. The typed configuration validates INT96 time units when they are set, preserves the existing value on invalid input, and removes the deferred parsing and panic path.

The changes look good to me. I have one non-blocking suggestion to strengthen the SQL regression coverage.

[kosiew]

Could you add a SQL regression test that rejects CREATE EXTERNAL TABLE ... OPTIONS ('format.coerce_int96' 'nanos') at CREATE time? Using an explicit schema and an existing Parquet fixture would also cover the reported panic path, which the current SET test doesn't exercise.

[akashjainn]

Added here: 920e9ab. Thank you so much!