andygrove commented on code in PR #22318:
URL: https://github.com/apache/datafusion/pull/22318#discussion_r3262177830
##########
datafusion/datasource-parquet/src/file_format.rs:
##########
@@ -713,10 +713,31 @@ pub fn apply_file_schema_type_coercions(
}
/// Coerces the file schema's Timestamps to the provided TimeUnit if Parquet
schema contains INT96.
+///
+/// Equivalent to calling [`coerce_int96_to_resolution_with_tz`] with
`timezone: None`,
+/// producing `Timestamp(time_unit, None)` for INT96-derived columns (the
historical
+/// default). Use [`coerce_int96_to_resolution_with_tz`] to attach a timezone.
Review Comment:
Went with the Int96Coercer builder approach in 916029dac. The original
`coerce_int96_to_resolution` is now a `#[deprecated]` thin wrapper, and the
`_with_tz` variant I added in this PR is gone (replaced by
`Int96Coercer::with_timezone`).
##########
datafusion/datasource-parquet/src/opener.rs:
##########
@@ -1748,6 +1755,7 @@ mod test {
enable_bloom_filter: self.enable_bloom_filter,
enable_row_group_stats_pruning:
self.enable_row_group_stats_pruning,
coerce_int96: self.coerce_int96,
+ coerce_int96_tz: None,
Review Comment:
Good catch — added a comment in 916029dac. The test builder has a
`with_coerce_int96` setter but no `with_coerce_int96_tz` yet because none of
the existing coerce_int96 tests need a timezone (they all expect the legacy
`Timestamp(_, None)` output). The comment notes that and points at adding a
setter analogous to `with_coerce_int96` if a future test needs one.
--
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]
