Skip to content

[SPARK-56030] [SQL] Normalize inner Project when below Project -> Filter #54859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mihailoale-db wants to merge 1 commit into
base: master
Choose a base branch
from
+19 −0
Open

[SPARK-56030] [SQL] Normalize inner Project when below Project -> Filter #54859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/NormalizePlan.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ object NormalizePlan extends PredicateHelper {
case project @ Project(_, innerAggregate: Aggregate) =>
project.copy(child = normalizeAggregateListOrder(innerAggregate))

case project @ Project(_, filter @ Filter(_, innerProject: Project)) =>
Copy link
Contributor

@cloud-fan cloud-fan Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make it more general? we can turn transformUpWithSubqueries into a manual top-down recursion. When we hit a project, we set a bool flag projectListOrderSensitive to false when invoking the recursive method, and we do not set projectListOrderSensitive back to true for a whitelist of plan nodes: Filter, Sample, Sort, etc.

Copy link
Contributor Author

@mihailoale-db mihailoale-db Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about that. It's not just Filter, Sort, Sample and some small set of operators but all the operators that have override def output: Seq[Attribute] = child.output. I don't think that it's robust in a way that you can easily miss some and besides that if someone adds another operator there is 99% chance that they will forget to add it to the list. So I propose to go with this approach and think of a better solution in a background in order to avoid polluting NormalizePlan too much.

project.copy(child = filter.copy(child = normalizeProjectListOrder(innerProject)))

/**
* ORDER BY covered by an output-retaining project on top of GROUP BY
*/
Expand Down
16 changes: 16 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/NormalizePlanSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,22 @@ class NormalizePlanSuite extends SparkFunSuite with SQLConfHelper {
assert(NormalizePlan(baselinePlan) == NormalizePlan(testPlan))
}

test("Normalize ordering in a project list of an inner Project under Project and Filter") {
val baselinePlan =
LocalRelation($"col1".int, $"col2".string)
.select($"col1", $"col2")
.where($"col1" === 1)
.select($"col1")
val testPlan =
LocalRelation($"col1".int, $"col2".string)
.select($"col2", $"col1")
.where($"col1" === 1)
.select($"col1")

assert(baselinePlan != testPlan)
assert(NormalizePlan(baselinePlan) == NormalizePlan(testPlan))
}

test("Normalize InheritAnalysisRules expressions") {
val castWithoutTimezone =
Cast(child = Literal(1), dataType = BooleanType, ansiEnabled = conf.ansiEnabled)
Expand Down