mbutrovich opened a new pull request, #5180: URL: https://github.com/apache/datafusion-comet/pull/5180
## Which issue does this PR close? No linked issue. Found via `TestMergeOnReadMerge#testSelfMerge` failing on Iceberg 1.8.1 / Spark 3.4 CI in #4752. ## Rationale for this change `PlanDataInjector` keys a native Iceberg scan's planning data (schema, file scan tasks, pools) by the scan's `metadata_location` alone. A query can read the same table twice within one native plan (a self-join, or a self-merge's target and source reads) via two scans that are logically different: different projected columns, or different pushed filters. Both scans compute the same key, so one scan's serialized data silently overwrites the other's, and the wrong data gets injected into one of the two scan operators. This surfaced as `TestMergeOnReadMerge#testSelfMerge`/`testSelfMergeWithCaching` failing on Iceberg 1.8.1 / Spark 3.4 CI with `CometNativeException: Column index N is out of bound` once metadata columns (`_file`, `_pos`, `_spec_id`, `_partition`) stopped forcing a fallback to Spark. The underlying collision is not specific to that feature: any two native Iceberg scans of the same table with different pushed filters or projections, landing in one native plan without an intervening shuffle, can hit it. ## What changes are included in this PR? - Add `scan_hash_code` to `IcebergScanCommon` (native/proto), set from Iceberg's own `SparkScan.hashCode()`, which already folds in pushed filters, snapshot, branch, and read schema. - `IcebergPlanDataInjector.getKey`, `PlanDataInjector.findAllPlanData`'s Iceberg case, and `CometIcebergNativeScanExec.doExecuteColumnar` now key on `(metadata_location, scan_hash_code)` instead of `metadata_location` alone. ## How are these changes tested? - New tests in `PlanDataInjectorSuite` construct two Iceberg scan operators sharing a `metadata_location` with different `scan_hash_code`s and assert each receives its own planning data rather than the other's. - Verified against the original regression: reproduced `TestMergeOnReadMerge#testSelfMerge` and `testSelfMergeWithCaching` failing on Iceberg 1.8.1 / Spark 3.4 without this fix, and passing with it applied. -- 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]
