LuciferYang commented on code in PR #12840:
URL: https://github.com/apache/gluten/pull/12840#discussion_r3823393161
##########
gluten-ut/spark34/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -1038,6 +1038,7 @@ class ClickHouseTestSettings extends BackendTestSettings {
.exclude("SPARK-37742: AQE reads invalid InMemoryRelation stats and
mistakenly plans BHJ")
.exclude("SPARK-37328: skew join with 3 tables")
.exclude("SPARK-39915: Dataset.repartition(N) may not create N partitions")
+ .exclude("SPARK-37652: optimize skewed join through union")
Review Comment:
`excludeGlutenTest` maps the name through the `"Gluten - "` prefix
(`BackendTestSettings.scala:138-139`, `GLUTEN_TEST` in
`GlutenTestConstants.scala:23`), so it would exclude `Gluten - SPARK-37652:
optimize skewed join through union`, i.e. the rewritten case this PR adds. That
is the opposite of the intent here.
The plain `.exclude(...)` is the right one: it targets Spark's own inherited
`test("SPARK-37652: ...")`, which carries no prefix. Blocking that one is the
point, because on ClickHouse the joins are turned into transformers and Spark's
version counts zero joins via `findTopLevelSortMergeJoin`; the Gluten rewrite
uses `findTopLevelSortMergeJoinTransform` instead. spark33's
`ClickHouseTestSettings` uses the same plain `.exclude(...)` for this case
(line 1170), which is the precedent this line follows.
--
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]
