dwsmith1983 opened a new pull request, #6116: URL: https://github.com/apache/datafusion-comet/pull/6116
## Which issue does this PR close? Closes #5936. ## Rationale for this change Spark's `ParquetReadSupport.getRequestedSchema` raises "Spark read schema expects field Ids, but Parquet file schema doesn't contain any field Ids" whenever the requested schema carries a `parquet.field.id` at any depth and the file carries none at any depth, unless `spark.sql.parquet.fieldId.read.ignoreMissing` is set. It does this on every read, for both readers, without consulting `spark.sql.parquet.fieldId.read.enabled`. The same code is on branch-3.5 and branch-4.0. The native scan ran that check only when the read flag was on and only over root fields, and it ran it inside the name remap, which a case-sensitive session with the flag off never reaches. Two differences followed. A read schema with ids over a file without ids returned rows where Spark raises. A file whose ids sit only on nested fields was rejected where Spark reads it and null-fills the unmatched fields. ## What changes are included in this PR? - `any_nested_field_has_id` in `parquet_support.rs` walks struct children, map keys and values and every list representation, mirroring Spark's `ParquetUtils.hasFieldIds` and `containsFieldIds`. - The check moves to the top of `SparkPhysicalExprAdapterFactory::create`, before the remap, and reads `ignore_missing_field_id` and the two recursive predicates only. `remap_physical_schema` no longer takes `ignore_missing_field_id`. Id matching itself is unchanged and still gated on the read flag and root ids, as Spark's `clipParquetGroupFields` gates it per struct level. - The Iceberg scan sets `ignore_missing_field_id`, since Iceberg resolves columns by id itself and its reader supplies the ids on every field. That path could not reach the check before and cannot now. No JVM change: root ids and nested ids already reach the native requested schema regardless of the read flag. ## How are these changes tested? Rust tests in `schema_adapter.rs` drive the real scan path: a read schema with ids over a file without ids is rejected with the read flag off, at the root and when the only id is on a struct child; a file whose ids sit only below the root is read with the unmatched root null-filled; `ignoreMissing` still suppresses the rejection; a read with no ids on either side is unchanged. A test in `parquet_support.rs` covers the predicate at every depth and for every list type. The first three scan tests fail before the change. `ParquetReadSuite` gains both halves of Spark's `ParquetFieldIdIOSuite."global read/write flag should work correctly"`: with the read flag off, a read schema with ids over a file without ids raises in Spark and in Comet, and reads by name under `ignoreMissing`; with the read flag off and a file that carries ids, differently named fields read as nulls with no error. A Rust test pins the same non-raising case through the scan. A second test reads a file whose ids sit only on nested fields with the flag on and checks the answer and the native operator against Spark. Both fail on main. `ParquetReadV1Suite`, `CometNativeReaderSuite`, `CometJoinSuite` and `CometScanRuleSuite` pass on Spark 3.5. `CometJoinSuite."Broadcast coalescing falls back for array field metadata mismatch"` reads an id-bearing schema over an id-free file with `ignoreMissing` set, so it is the one existing test whose result depends on the new rule being suppressed by the conf, and it still passes. Spark's own `ParquetFieldIdIOSuite` exercises this path and runs in the Spark SQL job rather than the pull request tier, and the Iceberg scan is touched, so this needs the `run-spark-4.1-tests` and `run-iceberg-tests` labels from a maintainer. -- 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]
