dwsmith1983 opened a new pull request, #6932: URL: https://github.com/apache/datafusion-comet/pull/6932
## Which issue does this PR close? Closes #5943. ## Rationale for this change The native Delta scan's plan-data key was `delta_<common.source>_<hash>`, where the hash covers the table root, required schema, data filters, projection and fields. `common.source` comes from `simpleStringWithNodeId()`, which is `Scan parquet (unknown)` outside of explain, so two scans of one table that differ only in their partition or file selection got the same key. `PlanDataInjector.getKey` documents that a scan's key must include its op's `plan_id`; the core native scan and Iceberg keys already do this through `PlanDataInjector.withPlanId`, and the Delta key did not. If two such scans ever met in one native block, `findAllPlanData` would throw `Two scans in one native plan have different planning data under key ...`. Today every self-join, MERGE and UNION shape I could find puts a stage boundary between them (the Delta scan reports `UnknownPartitioning`, so joins get an exchange on each side, and unions stop the collection), so this closes a gap in the key contract rather than a failure users hit now. ## What changes are included in this PR? - `DeltaPlanDataInjector.sourceKey` takes the plan id and ends the key with it through `PlanDataInjector.withPlanId`, giving the same `<content>#<plan_id>` shape as the core and Iceberg keys. `CometDeltaNativeScan` passes `scanExec.id`, the value it already sets as the op's `plan_id`. - The scaladoc on `sourceKey` and the `source_key` comment in `operator.proto` now describe the key as it is built. `common.source` is unchanged (core uses the same string), and the key stays out of canonicalization and equality. ## How are these changes tested? New `CometDeltaScanPlanDataKeySuite` converts two scans of one partitioned Delta table (`p = 0` and `p = 1`) whose `NativeScanCommon` protos are equal, and checks that: - their keys differ and each ends with its op's `plan_id`; - `findAllPlanData` over both returns two keys, each mapping to only its own partition's files; - `DeltaPlanDataInjector.getKey` reads back the driver's key, and rewrapping the op keeps it; - a second plan of the `p = 0` query gets a new key but still `sameResult` the first, while the `p = 1` scan does not. On main the test fails with two identical keys (`delta_Scan parquet (unknown)_-972154953`), and it fails the same way if the `withPlanId` suffix is removed. The new suite, `DeltaPlanDataInjectorSuite` and `CometDeltaNativeScanSuite` pass on Spark 3.5, 4.0 and 4.1, and the strict-warnings test-compile passes on 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]
