apache / apache/datafusion-comet
Remove stale dev/diffs test exclusions whose tracking issues are closed
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
Three exclusions in `dev/diffs/*.diff` cite Comet issues that have since been fixed, so they now suppress Spark SQL test coverage for no reason. Each is present in more than one version diff, so they should be removed across all affected diffs in one pass rather than in whichever version PR happens to touch them next: new version diffs are seeded from the previous version's, so a partial fix diverges the diffs and the next reseed reintroduces the stale entry.
Surfaced in review of #4950 by @sunchao.
### 1. Variant shredding suites
`VariantShreddingSuite` and `ParquetVariantShreddingSuite` are given an `IgnoreCometSuite` mixin, which skips all 15 tests when `ENABLE_COMET=true`.
- Cited issue: #2209, closed by merged PR #4084.
- #4084 added the shredded-Variant scan fallback and **already removed these same exclusions from `dev/diffs/4.0.4.diff`** (it has zero occurrences of `VariantShreddingSuite`); `4.1.3.diff` was never cleaned up.
- Affected: `dev/diffs/4.1.3.diff`, `dev/diffs/4.2.0.diff`.
- Removing the mixins should also drop the now-unused `IgnoreCometSuite` import in each file.
This is the highest-value one: it drops regression coverage for the silent-null corruption that #4084 fixed.
### 2. `having-and-order-by-recursive-type-name-resolution.sql`
The input file carries a `--SET spark.comet.enabled = false` header, which runs all 45 queries without Comet.
- Cited issue: #4123, closed by merged PR #4157, which added recursive fallback for map-containing aggregate keys plus a regression test for the exact `Struct(Map(...))` HAVING / ORDER BY shape.
- Affected: `dev/diffs/4.1.3.diff`, `dev/diffs/4.2.0.diff`.
### 3. Parquet V2 writer matrix in `ParquetTypeWideningSuite`
`withAllParquetWriters { ... }` is commented out, leaving all 82 generated widening cases on Parquet's default V1 writer.
- Cited issue: #574, closed: the active `native_datafusion` and `native_iceberg_compat` scans support `DELTA_BINARY_PACKED`, `DELTA_BYTE_ARRAY` and `DELTA_LENGTH_BYTE_ARRAY`.
- The related `SPARK-40128 read DELTA_LENGTH_BYTE_ARRAY encoded strings` test in `ParquetIOSuite` is still tagged `IgnoreComet("Comet doesn't support DELTA encoding yet")` and should be re-enabled with it.
- Affected: `dev/diffs/4.0.4.diff`, `dev/diffs/4.1.3.diff`, `dev/diffs/4.2.0.diff` (byte-identical in all three).
- If the legacy `native_comet` scan still cannot read these encodings, scope the exclusion to that scan implementation rather than removing the whole matrix.
### Suggested approach
One PR per item, each removing the exclusion from every diff that carries it, verified by the corresponding `Spark SQL Tests` CI job for each affected version. Where a specific case still fails, replace the blanket exclusion with a case-scoped `IgnoreComet` linked to a *current* issue, per the rule in `docs/source/contributor-guide/adding_a_new_spark_version.md` ("do not disable whole suites").
Contributor guide
Assessment
This issue has not been assessed yet.