Uh oh!
There was an error while loading. Please reload this page.
[SPARK-47430][SQL] Rework group by map type to fix bind reference exception - #47545
[SPARK-47430][SQL] Rework group by map type to fix bind reference exception#47545ulysses-you wants to merge 2 commits into
Conversation
ulysses-you
commented
Jul 31, 2024
cc @cloud-fan@stevomitric thank you |
stevomitric
commented
Jul 31, 2024
cc @nebojsa-db |
HyukjinKwon
commented
Aug 2, 2024
To fix which issue? |
ulysses-you
commented
Aug 2, 2024
@HyukjinKwon to fix the issue memtioned in pr description.. for example: I add this case in description to make it clear. |
yaooqinn
commented
Aug 2, 2024
Could you please ensure that the PR title does not sound like it's for refactoring if it's a bugfix? |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
is it a simple rename? not sure why git diff doesn't detect it
a575672 to
a443354CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
why rename? It's indeed inserting MapSort
| exprToMapSort.getOrElseUpdate( | ||
| expr.canonicalized, Alias(inserted, "_groupingmapsort")()) | ||
| .toAttribute |
There was a problem hiding this comment.
| exprToMapSort.getOrElseUpdate( | |
| expr.canonicalized, Alias(inserted, "_groupingmapsort")()) | |
| .toAttribute | |
| exprToMapSort.getOrElseUpdate( | |
| expr.canonicalized, | |
| Alias(inserted, "_groupingmapsort")() | |
| ).toAttribute |
| ReplaceExpressions, | ||
| RewriteNonCorrelatedExists, | ||
| PullOutGroupingExpressions, | ||
| InsertMapSortInGroupingExpressions, |
There was a problem hiding this comment.
let's add some comments to explain the rule order reasoning.
ulysses-you
commented
Aug 12, 2024
thank you all, merged to master |
What changes were proposed in this pull request?
This pr reworks the group by map type to fix issues:
MapSortand we didi not transform the plan with new outputMapSortrule should be put beforePullOutGroupingExpressionsto avoid complex expr existing in grouping keysWhy are the changes needed?
To fix issues.
for example:
Does this PR introduce any user-facing change?
no, not released
How was this patch tested?
improve the tests to add more cases
Was this patch authored or co-authored using generative AI tooling?
no