LuciferYang commented on issue #12863:
URL: https://github.com/apache/gluten/issues/12863#issuecomment-5380538296
First pass done. Settings files only, no compile and no test run, so what
follows is an upper bound that still needs a second look before anyone acts on
it. Base commit `68b2be74d` (current head of #12840).
## Method
Cheaper than what the issue body proposed, and it avoids enumerating each
parent suite's case list. Two steps.
Parse every `VeloxTestSettings.scala` / `ClickHouseTestSettings.scala` into
`enableSuite` blocks, model the DSL the way `BackendTestSettings.shouldRun`
does, then for each suite enabled on both 3.3 and a surviving version, take the
set difference of the exclude names. Two details matter here: `includeCH(...)`
is an empty method body, so it contributes nothing to `inclusion` and a block
holding only `includeCH` entries is run-all rather than include mode; and
`excludeGlutenTest(n)` expands to `exclude("Gluten - " + n)`, so filtering the
`Gluten - ` prefix out is what isolates the vanilla axis. A case counts as a
gap only if it is excluded on all four surviving versions.
That produced 34 candidate names, about half of which turned out not to
exist on 3.3 at all. So the second step: dump the string constants out of the
Spark 3.3.1 test jars (`spark-sql`, `spark-catalyst`, `spark-hive`,
`spark-core`, `-tests.jar`, `unzip -p | strings`) and drop any candidate whose
name does not appear. ScalaTest case names are string literals in the compiled
suite, so this is a reliable filter. It removed 17 names, all of them cases
added to Spark after 3.3: SPARK-39557, SPARK-39731, SPARK-39749, SPARK-41017,
SPARK-41048, SPARK-42635, SPARK-44641, SPARK-45604, SPARK-45786 and a few
unnumbered ones.
## Suite granularity: no gap
Four suites are enabled on 3.3 and on none of the four surviving versions,
and each has an explanation.
`GlutenAnsiCastSuiteWithAnsiModeOff`, `GlutenAnsiCastSuiteWithAnsiModeOn`
and `GlutenCastSuiteWithAnsiModeOn` are the three ANSI cast suites #12840
already covers in its description: `AnsiCast` is a 3.3-only class, and the
`spark-test-spark33` job never turns off
`spark.gluten.sql.columnar.backend.velox.ansiFallback.enabled`, so they do not
reach Velox on 3.3 either.
`GlutenDataSourceV2SQLSuite` was split upstream in Spark 3.4 into
`GlutenDataSourceV2SQLSuiteV1Filter` and `GlutenDataSourceV2SQLSuiteV2Filter`.
Both are enabled on all four surviving versions, so the coverage is there under
different names.
## Vanilla case granularity: 19 candidates
17 distinct names, 19 (suite, case) pairs because two names appear in two
suites each.
| backend | suite | count | notes |
| --- | --- | --- | --- |
| velox | `GlutenTryCastSuite` | 9 | four are `ANSI mode: Throw exception on
casting out-of-range value to *`, same story as the three ANSI suites above |
| velox | `GlutenInsertSuite` | 2 | SPARK-35106, `Stop task set if
FileAlreadyExistsException was thrown` |
| velox | `GlutenCastSuite` | 1 | `cast from timestamp II` |
| velox | `GlutenFileSourceCharVarcharTestSuite` | 1 | `length check for
input string values: nested in array of struct` |
| velox | `GlutenDSV2CharVarcharTestSuite` | 1 | same case name as the row
above |
| velox | `GlutenParquetIOSuite` | 1 | `Write Spark version into Parquet
metadata` |
| clickhouse | `GlutenTryCastSuite` | 3 | `cast from array II`, `cast from
array III`, `cast from struct III` |
| clickhouse | `GlutenMathExpressionsSuite` | 1 | `tanh` |
None of the 19 is excluded on the 3.3 side through `excludeByPrefix`, which
I checked separately since a prefix match would not show up in a name diff.
## One suite the settings cannot answer
`ClickHouseAdaptiveQueryExecSuite` runs in exclude mode on 3.3 and in
include mode on 3.5, 4.0 and 4.1, so every vanilla case outside the whitelist
is off on those three. Resolving it needs the parent suite's case list per
version. This is the same asymmetry that came up while migrating SPARK-37652 in
#12840.
## What still needs verifying
The comparison is on names, so an upstream rename produces a false gap.
`GlutenDataSourceV2SQLSuite` above is exactly that shape at suite level, and it
can happen at case level too. Each of the 19 needs a look at the parent suite
in both versions before it is called a gap.
The settings files also keep changing, so this should be re-run against the
tree as it stands when PR 1 actually starts rather than trusted from here.
## Suggested shape
The set is not empty but it is small, and it does not belong in #12840. That
PR is green with a re-review pending, and the work is a different shape anyway:
enabling a vanilla case means deleting an `.exclude` line in four settings
files and finding out from CI whether the case passes on 3.4+, keeping the
exclude with a written reason where it does not. That is a CI-driven loop,
unlike #12840 which added Gluten-authored cases already known to pass. A small
separate PR ahead of PR 1, covering these eight suite and backend combinations,
fits better.
--
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]
