andygrove commented on PR #5255: URL: https://github.com/apache/datafusion-comet/pull/5255#issuecomment-5182588137
## Audit sweep results — CI run [30920419488](https://github.com/apache/datafusion-comet/actions/runs/30920419488) Full matrix completed on `9100a84` with `run-iceberg-tests` applied, so all four Iceberg versions ran. **9 of 76 jobs failed**, in two distinct groups. ### Group 1 — Iceberg's own `iceberg-spark-extensions` suite: fails on every version | Iceberg | Spark | Scala | Java | `iceberg-spark` | `iceberg-spark-extensions` | `iceberg-spark-runtime` | | ------- | ----- | ----- | ---- | --------------- | -------------------------- | ----------------------- | | 1.8.1 | 3.4.3 | 2.13 | 11 | pass | **FAIL** | pass | | 1.9.1 | 3.5.9 | 2.13 | 17 | pass | **FAIL** | pass | | 1.10.0 | 3.5.9 | 2.13 | 17 | pass | **FAIL** | pass | | 1.11.0 | 4.1.3 | 2.13 | 17 | pass | **FAIL** | pass | The failure is version-independent, and it is confined to `extensions`. That is a useful narrowing: `iceberg-spark` (core read/write paths, DDL/DML, pushdown) and `iceberg-spark-runtime` (shaded-JAR smoke test) are **green on all four versions** — so plain appends and overwrites survive the split operator. `iceberg-spark-extensions` is where the row-level copy-on-write `UPDATE`/`DELETE`/`MERGE` tests and the stored procedures (`rewrite_data_files`, `rewrite_manifests`, `expire_snapshots`, ...) live, which is exactly the surface `ReplaceData` interception and procedure-driven writes touch. I do not yet have the per-test breakdown for these four jobs — see "Blocked on" below. ### Group 2 — Comet's own `scans` bucket: fails on every profile except Spark 4.2 | Runner | Profile | Result | | ------ | --------------------------- | -------- | | Linux | Spark 3.4, JDK 11, Scala 2.12 | **FAIL** | | Linux | Spark 3.5, JDK 17, Scala 2.13 | **FAIL** | | Linux | Spark 4.0, JDK 21 | **FAIL** | | Linux | Spark 4.1, JDK 17 | **FAIL** | | Linux | Spark 4.2, JDK 17 | pass | | macOS | Spark 4.0, JDK 17, Scala 2.13 | **FAIL** | Root cause confirmed locally on Spark 3.5 / Scala 2.13 / JDK 17 — `org.apache.comet.CometIcebergRewriteActionSuite`, 4 of 5 tests: - `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)` All four fail on `Expected at least one captured plan with AppendData but got none`. Iceberg's `rewrite_data_files` action writes its compacted output through `AppendData`; the split operator replaces that with `IcebergWrite` under `IcebergCommit`, so the `hasNode("AppendData")` filter at `CometIcebergRewriteActionSuite.scala:207` and `:269` matches nothing. The rewrite itself is correct — the row-preservation, file-count, and delete-application assertions all pass; only the plan-shape expectation is stale. Control run: the same suite is **5/5 green** with the flag back at `false`, so this PR is the cause. Spark 4.2 passing is a red herring rather than a signal that 4.2 is unaffected: that profile pulls `iceberg-spark-runtime-4.0` 1.10.0 (`spark/pom.xml:318-324`) because no 4.2 runtime is published, so the rewrite-action tests most likely cancel there instead of running. Worth confirming rather than assuming. ### Everything else is green All other `pr_build` buckets (`exec`, `shuffle`, `expressions`) on all six profiles, `Spark SQL Tests (Spark 3.5)`, `Spark SQL Tests (Spark 4.1)`, the Rust tests, and every lint job passed. `CometIcebergWriteActionSuite` (19 tests, added by #4658) passes locally with the new default. ### Proposed follow-on work 1. **Teach `CometIcebergRewriteActionSuite` to accept either write shape.** Replace the bare `hasNode("AppendData")` filter with one that also matches `IcebergWrite`, so the suite is correct with the feature on or off. Small and well understood — this clears all of Group 2. 2. **Triage `iceberg-spark-extensions`.** The real work. Needs the per-test failure list before it can be scoped; it may be more stale plan-shape expectations, or genuine gaps in `ReplaceData` / procedure handling. 3. **Confirm the Spark 4.2 pass** is test-cancellation rather than real coverage. 4. **Reconsider the config category.** The entry is `CATEGORY_TESTING`; if it ships on by default it probably belongs under `CATEGORY_EXEC` so it appears in the user-facing config table rather than under "Development & Testing Settings". ### Blocked on The Iceberg reusable workflow uploads only the native library — no test reports — so the `iceberg-spark-extensions` failure detail exists only in the job logs, and those are not reachable from my environment (the run's log/artifact blob host is not on my egress allowlist). If someone can paste the failing test names from any one of the four `iceberg-spark-extensions` jobs, that is enough to scope item 2. Separately, it may be worth having `iceberg_spark_test_reusable.yml` upload the Gradle test reports on failure, so this is self-service next time. -- 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]
