andygrove opened a new pull request, #6515: URL: https://github.com/apache/datafusion-comet/pull/6515
## Which issue does this PR close? Closes #6506. ## Rationale for this change Spark's vectorized Parquet reader decides whether it can convert a column's file type to the read type in `ParquetVectorUpdaterFactory.getUpdater`, and it calls that only while decoding a row group. A file with nothing to decode, either empty or with every row group pruned by a filter, never reaches the check, so Spark reads it and returns the rows from the other files. The native scan rejected some of the same pairs when it opened the file: BINARY read as a non-string type, decimal narrowing, an integer read as a too-narrow decimal, and every scalar/complex mismatch. Since #5681 applied those rules to nested fields, an empty or fully pruned file with such a nested field fails the query where Spark and 1.0.0 return rows. The top-level versions of these rejections have behaved this way since #4229, which is in 1.0.0. ## What changes are included in this PR? `check_leaf_conversion` now returns the deferred `RejectOnNonEmpty` verdict for every pair Spark rejects in `getUpdater`, at any nesting level. A file with rows still fails with the same error, now when its first batch is decoded rather than when the file is opened. A file with nothing to decode reads. The rejections still raised when the file opens are the shape mismatches that Spark also fails at open, because it can't clip the file's Parquet type to the read type. These are a group read as a different type (`ParquetToSparkSchemaConverter`), and a primitive read as a struct, or as an array or map with a complex element (`ParquetReadSupport.clipParquetType`). Spark doesn't clip a primitive read as an array or map of primitives, so that pair is deferred too, as in SPARK-45604. The shape check now runs first, so a BINARY column read as a struct is still rejected at open instead of matching the BINARY rule. This also applies to top-level columns: an empty or fully pruned file with a top-level BINARY, decimal-narrowing or integer-to-decimal mismatch now reads, as it does in Spark. That part is not a regression fix, since 1.0.0 rejected those at open too. But it is the same function at every level, and Spark treats both levels the same way. The join dynamic filter's schema guard (`RuntimeFilterSchemaAdapterFactory`) disables the runtime filter for a file whose column rewrites to anything other than a plain column or a literal. It treats the newly deferred rejections the same way it treated the open-time errors, so a file the dynamic filter would skip is still read and still fails. ## How are these changes tested? New Rust tests in `schema_adapter.rs` write real Parquet files and scan them through the adapter. All three fail on `main`. - `rejected_conversions_pass_for_empty_file`: seven rejected pairs, at the top level and nested in a struct, read from an empty file without error. Each fails with the right column path once the file has a row. - `rejected_conversion_passes_for_pruned_row_group`: a top-level and a nested decimal narrowing in a file whose only row group `id = 100` prunes reads without error, and fails without the filter. - `shape_mismatch_rejects_at_open_only_where_spark_cannot_clip` pins which shape mismatches are still rejected at open. A new `ParquetReadSuite` test, "native scan reads files with nothing to decode whose types Spark rejects", reproduces the issue end to end. It covers six mismatches: a struct field, a nested decimal, a list element, a map value, a primitive read as an array, and a top-level column. Each runs once with an empty file and once with a file whose only row group a filter prunes. The test compares with Spark, asserts a native scan, and checks that the pruned row group still fails when it is decoded. Before writing the fix, I compared Spark and Comet on `main` for 15 empty-file cases and 4 pruned-row-group cases. Comet failed in every case where Spark returned rows. With this change, Comet returns the same rows in all of them. It still fails in the two cases where Spark fails at open (an array read as an int, and an int read as a struct). Local runs: - `cargo test -p datafusion-comet --lib`: 581 passed. Clippy (`--all-targets -D warnings`) and rustfmt are clean. - `ParquetReadV1Suite` on the default Spark 4.1 profile: all tests passed. - The `native scan` tests in `ParquetReadV1Suite` on Spark 3.5, including the existing rejection tests and the new one: 11 passed. -- 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]
