Skip to content
Closed
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
04959c2
refactor analyzer adding a new object
anchovYu Nov 23, 2022
6f44c85
lca code
anchovYu Nov 23, 2022
725e5ac
add tests, refine logic
anchovYu Nov 28, 2022
660e1d2
move lca rule to a new file
anchovYu Nov 28, 2022
fd06094
rename conf
anchovYu Nov 28, 2022
7d4f80f
test failure
anchovYu Nov 29, 2022
b9704d5
small fix
anchovYu Nov 29, 2022
777f13a
temp commit, still in implementation
anchovYu Nov 29, 2022
09480ea
a temporary solution, but still fail certain cases
anchovYu Nov 30, 2022
c972738
working solution, needs some refinement
anchovYu Dec 1, 2022
97ee293
Merge remote-tracking branch 'apache/master' into SPARK-27561-refactor
anchovYu Dec 1, 2022
5785943
make changes to accomodate the recent refactor
anchovYu Dec 2, 2022
757cffb
introduce leaf exp in Project as well
anchovYu Dec 5, 2022
29de892
handle a corner case
anchovYu Dec 5, 2022
72991c6
add more tests; add check rule
anchovYu Dec 6, 2022
d45fe31
uplift the necessity to resolve expression in second phase; add more …
anchovYu Dec 8, 2022
1f55f73
address comments to add tests for LCA off
anchovYu Dec 8, 2022
f753529
revert the refactor, split LCA into two rules
anchovYu Dec 9, 2022
b9f706f
better refactor
anchovYu Dec 9, 2022
94d5c9e
address comments
anchovYu Dec 9, 2022
d2e75fd
Merge branch 'SPARK-27561-refactor' into SPARK-27561-agg
anchovYu Dec 9, 2022
edde37c
basic version passing all tests
anchovYu Dec 9, 2022
fb7b18c
update the logic, add and refactor tests
anchovYu Dec 12, 2022
3698cff
update comments
anchovYu Dec 13, 2022
e700d6a
add a corner case comment
anchovYu Dec 13, 2022
8d20986
address comments
anchovYu Dec 13, 2022
d952aa7
Merge branch 'SPARK-27561-refactor' into SPARK-27561-agg
anchovYu Dec 13, 2022
44d5a3d
Merge remote-tracking branch 'apache/master' into SPARK-27561-agg
anchovYu Dec 13, 2022
ccebc1c
revert some changes
anchovYu Dec 13, 2022
5540b70
fix few todos
anchovYu Dec 13, 2022
338ba11
Merge remote-tracking branch 'apache/master' into SPARK-27561-agg
anchovYu Dec 16, 2022
136a930
fix the failing test
anchovYu Dec 16, 2022
5076ad2
fix the missing_aggregate issue, turn on conf to see failed tests
anchovYu Dec 19, 2022
2f2dee5
remove few todos
anchovYu Dec 19, 2022
3a5509a
better fix to maintain aggregate error: only lift up in certain cases
anchovYu Dec 20, 2022
a23debb
Merge remote-tracking branch 'apache/master' into SPARK-27561-agg
anchovYu Dec 20, 2022
b200da0
typo
anchovYu Dec 20, 2022
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
Prev Previous commit
Next Next commit
update comments
  • Loading branch information
anchovYu committed Dec 13, 2022
commit 3698cfffa865f39f1262be7b407dbae4a9eeee57
Original file line number Diff line number Diff line change
Expand Up @@ -32,51 +32,55 @@ import org.apache.spark.sql.internal.SQLConf
* Plan-wise, it handles two types of operators: Project and Aggregate.
* - in Project, pushing down the referenced lateral alias into a newly created Project, resolve
* the attributes referencing these aliases
* - in Aggregate TODO inserting the Project node above and fall back to the resolution of Project.
* - in Aggregate, inserting the Project node above and falling back to the resolution of Project.
*
* The whole process is generally divided into two phases:
* 1) recognize resolved lateral alias, wrap the attributes referencing them with
* [[LateralColumnAliasReference]]
* 2) when the whole operator is resolved, unwrap [[LateralColumnAliasReference]].
* For Project, it further resolves the attributes and push down the referenced lateral aliases.
* For Aggregate, TODO
* 2) when the whole operator is resolved,
* For Project, it unwrap [[LateralColumnAliasReference]], further resolves the attributes and
* push down the referenced lateral aliases.
* For Aggregate, it goes through the whole aggregation list, extracts the aggregation
* expressions and grouping expressions to keep them in this Aggregate node, and add a Project
* above with the original output. It doesn't do anything on [[LateralColumnAliasReference]], but
* completely leave it to the Project in the future turns of this rule.
*
* Example for Project:
* ** Example for Project:
* Before rewrite:
* Project [age AS a, 'a + 1]
* +- Child
*
* After phase 1:
* Project [age AS a, lateralalias(a) + 1]
* Project [age AS a, lca(a) + 1]
* +- Child
*
* After phase 2:
* Project [a, a + 1]
* +- Project [child output, age AS a]
* +- Child
*
* Example for Aggregate TODO
*
* For Aggregate, it first wraps the attribute resolved by lateral alias with
* [[LateralColumnAliasReference]].
* Before wrap (omit some cast or alias):
* ** Example for Aggregate:
* Before rewrite:
* Aggregate [dept#14] [dept#14 AS a#12, 'a + 1, avg(salary#16) AS b#13, 'b + avg(bonus#17)]
* +- Child [dept#14,name#15,salary#16,bonus#17]
*
* After wrap:
* After phase 1:
* Aggregate [dept#14] [dept#14 AS a#12, lca(a) + 1, avg(salary#16) AS b#13, lca(b) + avg(bonus#17)]
* +- Child [dept#14,name#15,salary#16,bonus#17]
*
* When the whole Aggregate is resolved, it inserts a [[Project]] above with the aggregation
* expression list, but extracts the [[AggregateExpression]] and grouping expressions in the
* list to the current Aggregate. It restores all the [[LateralColumnAliasReference]] back to
* [[UnresolvedAttribute]]. The problem falls back to the lateral alias resolution in Project.
*
* After restore:
* Project [dept#14 AS a#12, 'a + 1, avg(salary)#26 AS b#13, 'b + avg(bonus)#27]
* After phase 2:
* Project [dept#14 AS a#12, lca(a) + 1, avg(salary)#26 AS b#13, lca(b) + avg(bonus)#27]
* +- Aggregate [dept#14] [avg(salary#16) AS avg(salary)#26, avg(bonus#17) AS avg(bonus)#27,dept#14]
* +- Child [dept#14,name#15,salary#16,bonus#17]
*
* Now the problem falls back to the lateral alias resolution in Project.
* After future rounds of this rule:
* Project [a#12, a#12 + 1, b#13, b#13 + avg(bonus)#27]
* +- Project [dept#14 AS a#12, avg(salary)#26 AS b#13]
* +- Aggregate [dept#14] [avg(salary#16) AS avg(salary)#26, avg(bonus#17) AS avg(bonus)#27,
* dept#14]
* +- Child [dept#14,name#15,salary#16,bonus#17]
*
*
* The name resolution priority:
* local table column > local lateral column alias > outer reference
Expand Down Expand Up @@ -172,8 +176,8 @@ object ResolveLateralColumnAliasReference extends Rule[LogicalPlan] {
ne.toAttribute
case e if groupingExpressions.exists(_.semanticEquals(e)) =>
// TODO one concern here, is condition here be able to match all grouping
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I surprisingly found out that this existing query can't analyze:

select 1 + dept + 10 from $testTable group by dept + 10
-- error: [MISSING_AGGREGATION] The non-aggregating expression "dept" is based on columns which are not participating in the GROUP BY clause

Seems in our checkAnalysis, we don't canonicalize to compare the expressions. It is structured as (1 + dept) + 10, and can't match the grouping expression (dept + 10).

// expressions? For example, Agg [age + 10] [a + age + 10], when transforming down,
// is it possible that (a + age) + 10, so that it won't be able to match (age + 10)
// expressions? For example, Agg [age + 10] [1 + age + 10], when transforming down,
// is it possible that (1 + age) + 10, so that it won't be able to match (age + 10)
// add a test.
val ne = expressionMap.getOrElseUpdate(
e.canonicalized,
Expand Down