sunchao opened a new pull request, #5369: URL: https://github.com/apache/datafusion-comet/pull/5369
## Why are the changes needed? Closes #5305. Comet's existing native Parquet writer derives its output schema from the placeholder scan rather than the Catalyst write schema. The placeholder marks top-level columns nullable and drops their field metadata, so the generated Parquet footer records required columns as optional and omits the field IDs required by Spark, Delta column mapping, and Iceberg-compatible schemas. Delta also stores IDs for synthetic list elements and map keys/values separately on the nearest Catalyst struct field; losing those IDs breaks nested column mapping even when ordinary struct-field IDs are preserved. ## What changes were proposed in this PR? - Carry the complete Catalyst target schema through the existing native Parquet writer protobuf and construct the Arrow/Parquet writer from that schema instead of the placeholder scan schema. - Respect Spark's `spark.sql.parquet.fieldId.write.enabled` setting, including its default-enabled behavior and explicit disablement, for top-level and nested struct fields. - Preserve Delta-style `parquet.field.nested.ids` metadata on synthetic list elements and map key/value fields, including nested collections and the nearest-struct-field path reset. - Allow metadata-only differences between incoming Arrow batches and the target schema without weakening data-type or nullability validation. - Add Rust footer/schema regression coverage and Spark write/read footer regressions that assert the native writer actually runs, verify required/optional repetitions and field IDs, and read renamed/reordered columns by ID. This changes the existing Spark 3.5/4.0 writer path directly and does not depend on or replace the writer-seam refactor in #5293. ## How was this PR tested? - Reproduced the pre-fix failure through the real native writer: `OPTIONAL did not equal REQUIRED` in the new Catalyst-schema footer regression. - `cargo fmt --all -- --check` - `cargo test -p datafusion-comet execution::serde::tests --lib` — 3 passing tests for absent/zero IDs, list/map metadata, and nested collections. - `cargo test -p datafusion-comet execution::operators::parquet_writer::tests --lib` — 2 passing tests, including a real Parquet footer round trip; 4 existing HDFS-cluster tests remain ignored. - `./mvnw -o -Pspark-3.5 test -Dtest=none -Dsuites=org.apache.comet.parquet.CometParquetWriterSuite` — all 33 native Parquet writer tests passed. - `./mvnw -o -Pspark-4.0 test -Dtest=none '-Dsuites=org.apache.comet.parquet.CometParquetWriterSuite native parquet'` — all 3 new write/footer/read regressions passed. - Maven Spotless and Scalastyle checks passed under both Spark profiles. -- 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]
