Visorgood commented on PR #6696: URL: https://github.com/apache/datafusion-comet/pull/6696#issuecomment-6879097266
This suite only uses `InMemoryRowLevelOperationTableCatalog`, so it can't show one gap for Iceberg tables. On Spark 4.2, `RewriteMergeIntoTable` emits the new `InsertOnlyMerge` node for insert-only MERGEs, where 4.1.3 emitted `AppendData.byPosition(r, ...)` for both the one-clause and the multi-clause shape. `IcebergWriteStrategy` matches `AppendData`, `OverwriteByExpression`, `OverwritePartitionsDynamic` and `ReplaceData`, but not `InsertOnlyMerge`. So, as far as I can tell from the code (I haven't run it yet): on 4.1 such a MERGE into Iceberg is planned as Comet's split write, where `MergeRows` gets the summary-aware tag and both it and the data-file write can run natively. On 4.2 Spark plans it as `InsertOnlyMergeExec` and iceberg-java writes the files, with or without this PR, since this PR only makes the `MergeRows` child native. Fixing it would likely need an `InsertOnlyMerge` case in `IcebergWriteStrategy` behind a 4.2 shim, plus `IcebergWriteSummaryShim` committing a `MergeSummary` with all written rows as inserts, as `InsertOnlyMergeExec` does, so Iceberg still gets the merge metrics. This shouldn't block this PR, since it keeps the writer on Spark by design. Unless it's already tracked, I'll open a follow-up issue once Iceberg runs on Spark 4.2 (#7520). -- 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]
