andygrove opened a new pull request, #6677: URL: https://github.com/apache/datafusion-comet/pull/6677
## Which issue does this PR close? Closes #6141. ## Rationale for this change #6141 reports that the native Iceberg writer fails every task on a format-version-1 spec that mixes a live partition field with a `void` field whose source column was dropped, and expects the insert to succeed as it does with iceberg-java. Reproducing it with the issue's statements shows that iceberg-java fails too: | Writer | Iceberg 1.8.1 (Spark 3.5) | Iceberg 1.11 (Spark 4.1) | | --- | --- | --- | | iceberg-java | `ValidationException: Cannot find source column for partition field: 1001: b` | `IllegalArgumentException: Cannot build accessor for field: null` | | native | `CometNativeException: ... field_id: 3 ... Field not found` | the same | So the outcome to match is iceberg-java's failure. Teaching the native writer to resolve the spec without the dropped column would let Comet write a table that Spark without Comet cannot. Declining the native write at plan time keeps the error iceberg-java's own. This is one of the blockers listed on #6664, which turns the native writer on by default. ## What changes are included in this PR? - `IcebergReflection.voidFieldsWithDroppedSource` names the `void` fields of a spec whose source column is no longer in the schema, when the spec also has a live field. A spec whose fields are all `void` stays eligible: the native writer writes it unpartitioned, as iceberg-java does on Iceberg 1.11. (On Iceberg 1.8 neither writer can write such a table, because iceberg-java cannot bind the original spec on the executors.) - `CometIcebergNativeWrite` gains a rule that declines the write when the write's output spec has such a field. It runs after the float/double identity rule, so a spec that trips both keeps that rule's reason. - The user guide's eligibility table and a paragraph under it, and the contributor guide's pitfall on `void` fields, describe the rule. ## How are these changes tested? - A new `CometIcebergWriteDetectionSuite` test builds the issue's shape (a format-version-1 table partitioned by `region` and `id`, then the `id` partition field and the `id` column dropped) and checks that the gate declines it with the new reason. Before the rule, the native writer was planned for this insert. The insert fails on both paths, so the test checks the gate's decision on a planned insert. - Another test checks that an all-`void` spec whose source column was dropped stays `Compatible`. Removing the rule's "beside a live field" condition makes it fail. - `CometIcebergWriteDetectionSuite` (64 tests) and `CometIcebergWriteActionSuite` pass on Spark 4.1 and Spark 3.5. -- 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]
