You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2026/08/22 08:40:04 UTC
[I] [VL] Before removing gluten-ut/spark33: confirm its enabled vanilla test set is a subset of 3.4+ [gluten]
LuciferYang opened a new issue, #12863:
URL: https://github.com/apache/gluten/issues/12863
### What this is
A prerequisite for removing `gluten-ut/spark33` under #12807. It is not a bug, it is an unanswered question that has to be settled before the module goes away, otherwise the removal silently drops test coverage.
### The gap
#12840 migrated the Spark 3.3-only coverage it could enumerate, at three granularities: whole suites, `.sql` files in `VeloxSQLQueryTestSettings`, and Gluten-authored cases (8 names out of 286). None of those is the risky one.
What it did not cover is the vanilla `test(...)` cases that `gluten-ut` inherits from Spark's own suites. Those are governed entirely by the settings files, and `gluten-ut/spark33`'s carry hundreds of `.exclude` entries. Whether the complement, that is the set of vanilla cases actually enabled on 3.3, is a subset of what 3.4/3.5/4.0/4.1 enable, is unanswered. If some vanilla case runs on 3.3 and is excluded in all four surviving modules, deleting the module removes the only place it runs, and nothing in the diff will look like a deletion of coverage.
### How to answer it
Mechanical, once someone writes the script. For each backend, compute the enabled-case set per version module from `VeloxTestSettings` / `ClickHouseTestSettings`: for every `enableSuite`, resolve whether the block is in include mode or exclude mode (`BackendTestSettings.shouldRun` throws if both are non-empty, so each block is one or the other), then apply the prefix and name rules from `BackendTestSettings` to the parent Spark suite's case list. Diff spark33's set against the union of the four surviving versions.
Two details that make a naive diff wrong. `excludeGlutenTest(name)` and `includeGlutenTest(name)` prepend the `"Gluten - "` prefix while bare `exclude` / `include` do not, so a name comparison has to normalise that. And `excludeByPrefix` matches with `startsWith`, so its effect depends on the parent suite's case names in that specific Spark version.
Anything the diff turns up either gets migrated the way #12840 migrated the rest, or gets an explicit "not worth keeping" note, before the module is deleted.
--
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: commits-unsubscribe@gluten.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [I] [VL] Before removing gluten-ut/spark33: confirm its enabled vanilla test set is a subset of 3.4+ [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on issue #12863:
URL: https://github.com/apache/gluten/issues/12863#issuecomment-5407196332
Answered. Removing `gluten-ut/spark33` does not drop coverage that Gluten actually validated, and the four cases where something was really lost now have PRs. Details below so nobody has to redo the diff.
### Method, and one correction worth knowing about
Computed the enabled-case set per version module from `VeloxTestSettings` / `ClickHouseTestSettings` the way this issue describes, then diffed spark33's set against the union of 3.4, 3.5, 4.0 and 4.1. That produced 39 candidates: 33 on Velox across 17 suites, 6 on ClickHouse across 4 suites.
The correction: a settings diff only tells you a name is excluded everywhere, not that the case ever existed on 3.3. Deciding that requires resolving the Gluten wrapper's parent Spark class and then checking that class in **Spark 3.3.1**, which is what `<spark.version>` is under the `spark-3.3` profile. Checking against a later 3.3.x gives wrong answers, because Spark backports test cases into maintenance releases. Two candidates flipped on exactly this: `SPARK-42635: timestampadd near daylight saving transition` and `row group skipping doesn't overflow when reading into larger type` both exist in 3.3.4 and neither exists in 3.3.1, so in this repository they never ran on 3.3. 3.3.1 has no `-test-sources.jar`, but case names are compiled into the class files as string constants, so `unzip -p <tests.jar> '*/<ParentClass>.class' | strings -a` settles it per class.
### Suite granularity: no gap
Four suites are enabled on 3.3 and on none of the surviving versions:
- `GlutenCastSuiteWithAnsiModeOn`, `GlutenAnsiCastSuiteWithAnsiModeOn`, `GlutenAnsiCastSuiteWithAnsiModeOff`. None of the three reaches Velox on CI as configured, so deleting them loses no backend coverage, but the reason differs per suite and is worth writing down. All three live in `gluten-ut/spark33/.../GlutenAnsiCastSuite.scala` and set session ANSI in their own `beforeAll`. `GlutenCastSuiteWithAnsiModeOn` sets it true and builds plain `Cast`, so it depends entirely on `FallbackOnANSIMode` (`FallbackRules.scala:30`, `enableAnsiMode && enableAnsiFallback`) tagging the whole plan; `velox_backend_x86.yml` contains no `ansiFallback` anywhere and the spark33 jobs pass only `-Dspark.test.home=...`, so `enableAnsiFallback` stays at its default of true. `GlutenAnsiCastSuiteWithAnsiModeOn` is covered by the same path and additionally builds `AnsiCast`. `GlutenAnsiCastSuiteWithAnsiModeOff` sets ANSI false, so that path does not apply to it; it never reaches Velox because `AnsiCast` is a
3.3-only class with no `Sig` entry (`ExpressionMappings.scala:45` has only `Sig[Cast](CAST)`, and the main code has zero references to `AnsiCast`), and the mapping is keyed by class. On top of all three, `GlutenTestsTrait.scala:50` defines `ansiTest = !enableAnsiFallback`, which is false here, so `checkExceptionInExpression` delegates to Spark's `ExpressionEvalHelper` without building a DataFrame at all.
- `GlutenDataSourceV2SQLSuite`: upstream split it into `GlutenDataSourceV2SQLSuiteV1Filter` and `...V2Filter` in 3.4, and both are enabled on all four versions. Same coverage, different name.
### Case granularity: 39 candidates, all resolved
25 of the 33 Velox candidates and 2 of the 6 ClickHouse candidates are cases that do not exist in Spark 3.3.1, so they never ran on 3.3 and cannot be a gap. The ticket numbers make most of them obvious: SPARK-39557, 39731, 39749, 41017, 41048, 42635, 44641, 45604, 45786, plus the DEFAULT-value INSERT cases that arrived with 3.4, `filter pushdown - StringPredicate`, `row group skipping doesn't overflow when reading into larger type`, and `CONVERSION_INVALID_INPUT: to_binary conversion function hex`.
Two of those deserve a note because a name diff makes them look like losses when they are renames. `UNSUPPORTED_FEATURE - SPARK-36346: can't read Timestamp as TimestampNTZ` exists from 3.4 on; 3.3 has the same scenario as `UNSUPPORTED_OPERATION - SPARK-36346: ...`, which `gluten-ut/spark33` also excludes, with the same reason. `ordering and partitioning reporting` is likewise 3.4+; 3.3 has `partitioning reporting`, which spark33 excludes and rewrites in its own `GlutenDataSourceV2Suite`, exactly as the surviving modules do.
That leaves 12 candidates that really did run on 3.3:
| case | suite | outcome |
|-|-|-|
| ANSI out-of-range byte / short / int / long | VL `GlutenTryCastSuite` | pass once enabled, removed in #12886 |
| cast from invalid string to numeric should throw NumberFormatException | VL `GlutenTryCastSuite` | same |
| SPARK-26218: Fix the corner case of codegen when casting float to Integer | VL `GlutenTryCastSuite` | same |
| Process Infinity, -Infinity, NaN in case insensitive manner | VL `GlutenTryCastSuite` | `+inf not supported in folly`, backend limit, applies on 3.3 too |
| cast from timestamp II | VL `GlutenTryCastSuite` | rewritten for ANSI mode, comment already there |
| data type casting | VL `GlutenTryCastSuite` | timezone set through config, `testGluten` rewrite already there |
| SPARK-35106: Throw exception when rename custom partition paths returns false | VL `GlutenInsertSuite` | native write staging dir differs from vanilla, not version-specific |
| Write Spark version into Parquet metadata | VL `GlutenParquetIOSuite` | Velox does not write file metadata into Parquet, backend limit |
| Stop task set if FileAlreadyExistsException was thrown | VL `GlutenInsertSuite` | fails; reason now written down in #12888 |
| cast from timestamp II | VL `GlutenCastSuite` | 4.0/4.1 had a rewrite, 3.4/3.5 did not; ported in #12888 |
| cast from array II / III, cast from struct III | CH `GlutenTryCastSuite` | only ran on CH 3.3; #12889 |
| tanh | CH `GlutenMathExpressionsSuite` | only ran on CH 3.3; #12889 |
For `Stop task set if FileAlreadyExistsException was thrown` I removed the exclusion and ran `GlutenInsertSuite` against 3.4.4 with Velox: 70 of 71 pass and this one fails with `Expected exception org.apache.spark.SparkException to be thrown, but org.apache.hadoop.fs.FileAlreadyExistsException was thrown`. The case installs a `FileSystem` that throws on create and expects Spark's write path to wrap the failure; Gluten's native write surfaces the raw exception. So the exclusion was right and only its reason was missing.
### One more axis worth recording here
While inventorying this I also settled the `.sql` file list, which is a different granularity but the same question. Ten files appear only in `gluten-ut/spark33`'s `SUPPORTED_SQL_QUERY_LIST`; one of them is in that module's own `ignoreList`, so nine would stop running. All nine pass on 3.4.4 locally, and #12890 adds them to the four surviving modules.
And `SPARK-33338: GROUP BY using literal map should not fail`, a Gluten-authored case that only really runs on spark33, turns out to be a deliberate disable rather than a missed migration: all four surviving modules carry it as `ignore(...)` with the reason above it, `// Velox throw exception : An unsupported nested encoding was found.`, added in `fd33a939a` when the native Parquet writer landed for Spark 3.4.
### Two limitations of the method
A name diff reports a false gap when upstream renames a case or a suite and Gluten follows the rename. That happened twice here, with the `GlutenDataSourceV2SQLSuite` split and with `CastSuite` becoming `CastWithAnsiOffSuite` upstream while Gluten kept `GlutenCastSuite` on 3.4/3.5 and switched to `GlutenCastWithAnsiOffSuite` on 4.0/4.1. And matching a case name against a version's tests jar as a whole is not the same as finding it in that suite's parent class; the check has to land on the class, and on the right 3.3.x.
Closing: this is not a blocker for removing the module. The four real losses are covered by #12888, #12889 and #12890.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [I] [VL] Before removing gluten-ut/spark33: confirm its enabled vanilla test set is a subset of 3.4+ [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [I] [VL] Before removing gluten-ut/spark33: confirm its enabled vanilla test set is a subset of 3.4+ [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed issue #12863: [VL] Before removing gluten-ut/spark33: confirm its enabled vanilla test set is a subset of 3.4+
URL: https://github.com/apache/gluten/issues/12863
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [I] [VL] Before removing gluten-ut/spark33: confirm its enabled vanilla test set is a subset of 3.4+ [gluten]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on issue #12863:
URL: https://github.com/apache/gluten/issues/12863#issuecomment-5410685235
Correcting one paragraph of the conclusion above, the one about `.sql` files. It said nine files appear only in `gluten-ut/spark33`'s `SUPPORTED_SQL_QUERY_LIST` and would stop running when the module is removed. They would not: they already run on `gluten-ut/spark34`.
The reason is #12897. spark34 matches the supported list with `contains` while 3.5+ match with `==`, so spark34's short entries (`count.sql`, `cross-join.sql`, `having.sql`, `inner-join.sql`, `join-empty-relation.sql`, `natural-join.sql`, `outer-join.sql`, `datetime-special.sql`, `parse-schema-string.sql`) already enable those files there, under names like `udf/udf-count.sql - Scala UDF` that never appear in the list. I ran `GlutenSQLQueryTestSuite` against Spark 3.4.4 with Velox twice, with and without the nine entries added: both runs give 222 tests, 222 succeeded, 0 failed, 84 ignored, and both include the nine. So the addition was a no-op on 3.4 and there is no gap on this axis.
That makes the method the mistake, not just the number: comparing the lists as sets is only valid between modules that match the same way, and spark34 does not. Filed as #12897.
The rest of the conclusion stands. #12890 continues as a coverage extension for 3.5, 4.0 and 4.1, where those nine files genuinely do not run, and it is not a prerequisite for removing the module.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org