LuciferYang commented on code in PR #12840:
URL: https://github.com/apache/gluten/pull/12840#discussion_r3840555031
##########
gluten-ut/spark34/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala:
##########
@@ -560,6 +560,8 @@ class VeloxTestSettings extends BackendTestSettings {
// Rewrite to match SortExecTransformer.
.excludeByPrefix("SPARK-41914:")
enableSuite[GlutenEnsureRequirementsSuite]
+ // Rewrite to change the shuffle partitions for optimizing repartition
+ .excludeByPrefix("SPARK-35675")
Review Comment:
Good catch, it should not be there. Removed.
The migration treated SPARK-35675 as rewrite-style, the same shape as
`default` in `GlutenLiteralExpressionSuite`: spark33 excludes Spark's own case
and supplies a Gluten copy, the four surviving modules had neither half, so I
restored both. The premise behind that rewrite is that Gluten's UT conf sets
`spark.sql.shuffle.partitions` to 1 while the case asserts two
`ShuffleExchangeLike` nodes, so the copy pins it back to 5 inside `withSQLConf`.
That premise does not hold here. `GlutenEnsureRequirementsSuite` already
overrides `sparkConf` with `.set("spark.sql.shuffle.partitions", "5")`, in all
five modules and at the base commit, and Spark's own case never touches
`SHUFFLE_PARTITIONS`. Its text is byte-identical between 3.3.1 and 3.5. So the
vanilla case has been running on 3.4+ with exactly the conf the rewrite was
written to supply, and passing, since the Velox settings enable that suite in
run-all mode with no exclusions on this name. My exclude turned off a green
case and put a functionally identical copy in its place. Read that way the
exclude is redundant on 3.3 as well, so this was a 2023 workaround I propagated
forward rather than a gap I closed.
Gone: the four `excludeByPrefix` entries, the three `.exclude` entries I had
added to the 3.5/4.0/4.1 ClickHouse settings, and the four copies of the case,
which returns `GlutenEnsureRequirementsSuite.scala` to its base content in
every module. spark34's ClickHouse settings keep the entry they already had.
The `SPARK-37652` exclude I had added to spark34's ClickHouse settings goes
too, for the same reason plus one more: its replacement is `ignoreGluten`, so
that exclude removed coverage and put nothing back. The PR is down from 43
files to 35.
--
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]
