Skip to content

Commit 286d336

Browse files
allisonwang-dbcloud-fan
authored andcommitted
[SPARK-40149][SQL][FOLLOWUP] Avoid adding extra Project in AddMetadataColumns
### What changes were proposed in this pull request? This PR is a follow-up for #37758. It updates the rule `AddMetadataColumns` to avoid introducing extra `Project`. ### Why are the changes needed? To fix an issue introduced by #37758. ```sql -- t1: [key, value] t2: [key, value] select t1.key, t2.key from t1 full outer join t2 using (key) ``` Before this PR, the rule `AddMetadataColumns` will add a new Project between the using join and the select list: ``` Project [key, key] +- Project [key, key, key, key] <--- extra project +- Project [coalesce(key, key) AS key, value, value, key, key] +- Join FullOuter, (key = key) :- LocalRelation <empty>, [key#0, value#0] +- LocalRelation <empty>, [key#0, value#0] ``` After this PR, this extra Project will be removed. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Add a new UT. Closes #39895 from allisonwang-db/spark-40149-follow-up. Authored-by: allisonwang-db <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
1 parent 3e40b38 commit 286d336

File tree

2 files changed

+19
-2
lines changed

2 files changed

+19
-2
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,7 @@ class Analyzer(override val catalogManager: CatalogManager) extends RuleExecutor
992992
if (metaCols.isEmpty) {
993993
node
994994
} else {
995-
val newNode = addMetadataCol(node, metaCols.map(_.exprId).toSet)
995+
val newNode = node.mapChildren(addMetadataCol(_, metaCols.map(_.exprId).toSet))
996996
// We should not change the output schema of the plan. We should project away the extra
997997
// metadata columns if necessary.
998998
if (newNode.sameOutput(node)) {

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala

+18-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import org.apache.spark.sql.catalyst.encoders.ExpressionEncoder
3636
import org.apache.spark.sql.catalyst.expressions._
3737
import org.apache.spark.sql.catalyst.expressions.aggregate.{AggregateExpression, Count, Sum}
3838
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser.parsePlan
39-
import org.apache.spark.sql.catalyst.plans.{Cross, Inner}
39+
import org.apache.spark.sql.catalyst.plans.{Cross, FullOuter, Inner, UsingJoin}
4040
import org.apache.spark.sql.catalyst.plans.logical._
4141
import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partitioning, RangePartitioning, RoundRobinPartitioning}
4242
import org.apache.spark.sql.catalyst.util._
@@ -1416,4 +1416,21 @@ class AnalysisSuite extends AnalysisTest with Matchers {
14161416
assert(!cg.rightOrder.flatMap(_.references).exists(cg.left.output.contains))
14171417
}
14181418
}
1419+
1420+
test("SPARK-40149: add metadata column with no extra project") {
1421+
val t1 = LocalRelation($"key".int, $"value".string).as("t1")
1422+
val t2 = LocalRelation($"key".int, $"value".string).as("t2")
1423+
val query =
1424+
Project(Seq($"t1.key", $"t2.key"),
1425+
Join(t1, t2, UsingJoin(FullOuter, Seq("key")), None, JoinHint.NONE))
1426+
checkAnalysis(
1427+
query,
1428+
Project(Seq($"t1.key", $"t2.key"),
1429+
Project(Seq(coalesce($"t1.key", $"t2.key").as("key"),
1430+
$"t1.value", $"t2.value", $"t1.key", $"t2.key"),
1431+
Join(t1, t2, FullOuter, Some($"t1.key" === $"t2.key"), JoinHint.NONE)
1432+
)
1433+
).analyze
1434+
)
1435+
}
14191436
}

0 commit comments

Comments
 (0)