andygrove opened a new pull request, #5255:
URL: https://github.com/apache/datafusion-comet/pull/5255

   ## Which issue does this PR close?
   
   Relates to #4322. Follow-up experiment on top of #4658.
   
   ## Rationale for this change
   
   **This is a CI audit sweep, not a merge candidate.** It is intentionally 
left red.
   
   #4658 adds Comet's split-operator plan for Iceberg V2 writes behind
   `spark.comet.write.iceberg.splitOperator.enabled`, defaulting to `false`. 
With the feature off,
   no existing test exercises it, so we have no visibility into what breaks 
once it is on.
   
   This PR rebases #4658 onto current `main` and flips that default to `true`, 
so the full CI
   matrix — Comet's own suites, the Spark SQL suites, and the Iceberg Spark 
suites — runs against
   the split-operator plan. The point is to enumerate every failure the feature 
causes so we can
   scope the follow-on work needed before it can ship enabled.
   
   ## What changes are included in this PR?
   
   Commits 1-5 are #4658 rebased onto `main` (only conflict was an additive one 
in
   `IcebergReflection.ClassNames`).
   
   Commit 6 is the actual change here:
   
   - `spark.comet.write.iceberg.splitOperator.enabled` default `false` -> `true`
   - `docs/source/user-guide/latest/iceberg-writes.md` updated to describe the 
new default
   
   No test expectations were adjusted — the whole point is to let them fail and 
be catalogued.
   
   ## Known failures so far
   
   Reproduced locally (Spark 3.5, Scala 2.13, JDK 17), all four from a single 
root cause:
   
   `org.apache.comet.CometIcebergRewriteActionSuite` — 4 of 5 tests fail:
   
   - `binPack rewrite reads each file group via CometIcebergNativeScan`
   - `sort rewrite runs scan, exchange, and sort natively in Comet`
   - `single-column zOrder rewrite runs scan, native exchange, and sort 
natively in Comet`
   - `binPack rewrite applies positional and equality deletes during compaction 
(MOR)`
   
   Each fails with `Expected at least one captured plan with AppendData but got 
none`. Iceberg's
   `rewrite_data_files` action writes its compacted output through 
`AppendData`, which the split
   operator now replaces with `IcebergWrite` under `IcebergCommit`. The rewrite 
itself is correct
   (the row-preservation and file-count assertions pass); only the plan-shape 
expectation is stale.
   The same suite passes 5/5 with the flag back at `false`, confirming this PR 
is the cause.
   
   Likely fix for follow-on work: teach the suite to accept either write shape, 
rather than
   matching `AppendData` alone.
   
   ## How are these changes tested?
   
   By CI, deliberately. `CometIcebergWriteActionSuite` (19 tests, added by 
#4658) passes locally
   with the new default. Iceberg 1.11 / Spark 4.1 runs on every PR, so the 
Iceberg Spark suites
   will report against the split-operator plan without needing the 
`run-iceberg-tests` label; add
   that label if we want 1.8 / 1.9 / 1.10 coverage too.
   


-- 
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]

Reply via email to