dwsmith1983 commented on code in PR #6116: URL: https://github.com/apache/datafusion-comet/pull/6116#discussion_r4100367642
########## native/core/src/parquet/eager_page_index_reader_factory.rs: ########## @@ -45,6 +45,13 @@ //! //! Filed upstream as apache/datafusion#23978. Revert this once the opener merges its deferred //! page-index load back into `FileMetadataCache` instead of bypassing it. +//! +//! The reader also carries Spark's missing field id check, because the footer is first at hand +//! here. Spark's `ParquetReadSupport` refuses to open a file whose raw schema carries no field +//! id when the requested schema carries one, unless `ignoreMissing` is set, and it walks the +//! raw `MessageType` to decide. The Arrow schema the schema adapter sees later cannot stand in +//! for that walk: the INT96 coercion rebuilds container fields without their metadata, and an +//! id on a `list` or `key_value` group, or on the message root, never reaches an Arrow field. Review Comment: The module doc now says a revert of the eager policy has to keep the factory, since `get_metadata` hosts this check and the Variant footer rewrite. It leads with the ids on repeated `list` and `key_value` groups and on the message root, and cites apache/datafusion#24790 once for the INT96 part. The repeated explanations in `parquet_support.rs`, `parquet_exec.rs`, `schema_adapter.rs`, `errors.rs` and `ParquetReadSuite` are gone, with a short comment left at the check. -- 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]
