Skip to content

Commit c91c2e9

Browse files
cloud-fandongjoon-hyun
authored andcommitted
[SPARK-38526][SQL] Fix misleading function alias name for RuntimeReplaceable
### What changes were proposed in this pull request? This PR uses a manual recursion to replace `RuntimeReplaceable` expressions instead of `transformAllExpressionsWithPruning`. The problem of `transformAllExpressionsWithPruning` is it will automatically make the replacement expression inherit the function alias name from the parent node, which is quite misleading. For example, `select date_part('month', c) from t`, the optimized plan in EXPLAIN before this PR is ``` Project [date_part(cast(c#18 as date)) AS date_part(month, c)#19] +- Relation default.t[c#18] parquet ``` Now it's ``` Project [month(cast(c#9 as date)) AS date_part(month, c)#10] +- Relation default.t[c#9] parquet ``` ### Why are the changes needed? fix misleading EXPLAIN result ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? new test Closes #35821 from cloud-fan/follow2. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent 60334d7 commit c91c2e9

File tree

2 files changed

+15
-3
lines changed

2 files changed

+15
-3
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/finishAnalysis.scala

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,14 @@ import org.apache.spark.util.Utils
4040
* we use this to replace Every and Any with Min and Max respectively.
4141
*/
4242
object ReplaceExpressions extends Rule[LogicalPlan] {
43-
def apply(plan: LogicalPlan): LogicalPlan = plan.transformAllExpressionsWithPruning(
43+
def apply(plan: LogicalPlan): LogicalPlan = plan.transformWithPruning(
4444
_.containsAnyPattern(RUNTIME_REPLACEABLE)) {
45-
case e: RuntimeReplaceable => e.replacement
45+
case p => p.mapExpressions(replace)
46+
}
47+
48+
private def replace(e: Expression): Expression = e match {
49+
case r: RuntimeReplaceable => replace(r.replacement)
50+
case _ => e.mapChildren(replace)
4651
}
4752
}
4853

sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class ExplainSuite extends ExplainSuiteHelper with DisableAdaptiveExecutionSuite
106106
keywords = "InMemoryRelation", "StorageLevel(disk, memory, deserialized, 1 replicas)")
107107
}
108108

109-
test("optimized plan should show the rewritten aggregate expression") {
109+
test("optimized plan should show the rewritten expression") {
110110
withTempView("test_agg") {
111111
sql(
112112
"""
@@ -125,6 +125,13 @@ class ExplainSuite extends ExplainSuiteHelper with DisableAdaptiveExecutionSuite
125125
"Aggregate [k#x], [k#x, every(v#x) AS every(v)#x, some(v#x) AS some(v)#x, " +
126126
"any(v#x) AS any(v)#x]")
127127
}
128+
129+
withTable("t") {
130+
sql("CREATE TABLE t(col TIMESTAMP) USING parquet")
131+
val df = sql("SELECT date_part('month', col) FROM t")
132+
checkKeywordsExistsInExplain(df,
133+
"Project [month(cast(col#x as date)) AS date_part(month, col)#x]")
134+
}
128135
}
129136

130137
test("explain inline tables cross-joins") {

0 commit comments

Comments
 (0)