Skip to content

Conversation

@peter-toth
Copy link
Contributor

@peter-toth peter-toth commented Mar 18, 2023

What changes were proposed in this pull request?

This PR reverts the follow-up PR of SPARK-41468: #39046

Why are the changes needed?

These changes are not needed and actually might cause performance regression due to preventing higher order function subexpression elimination in EquivalentExpressions. Please find related conversation here: #40473 (comment)

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing UTs.

…ambdaVariables in EquivalentExpressions"

This reverts commit 27f4d1e.
@HyukjinKwon
Copy link
Member

HyukjinKwon commented Mar 20, 2023

Merged to master and branch-3.4.

@HyukjinKwon
Copy link
Member

Thanks for filing a separate JIRA.

HyukjinKwon pushed a commit that referenced this pull request Mar 20, 2023
…uivalentExpressions

### What changes were proposed in this pull request?
This PR reverts the follow-up PR of SPARK-41468: #39046

### Why are the changes needed?
These changes are not needed and actually might cause performance regression due to preventing higher order function subexpression elimination in `EquivalentExpressions`. Please find related conversation here: #40473 (comment)

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Existing UTs.

Closes #40475 from peter-toth/SPARK-42852-revert-namedlambdavariable-changes.

Authored-by: Peter Toth <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
(cherry picked from commit ce3b03d)
Signed-off-by: Hyukjin Kwon <[email protected]>
@peter-toth
Copy link
Contributor Author

Thanks for filing a separate JIRA.

Thanks @HyukjinKwon for the quick review!
Actually I've just noticed that the SPARK-41468 follow-up PR was only merged to master (3.4 at that time) only: #39046 (comment).
So probably a simple revert commit (wihtout a new ticket) on 3.4 and master would have been sufficient... For the same reason I'm removing the 3.3.2 affect version from SPARK-42852. Please let me know if you disagree.

@HyukjinKwon
Copy link
Member

Yeah so I just reverted it from master and 3.4 ;-).

snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
…uivalentExpressions

### What changes were proposed in this pull request?
This PR reverts the follow-up PR of SPARK-41468: apache#39046

### Why are the changes needed?
These changes are not needed and actually might cause performance regression due to preventing higher order function subexpression elimination in `EquivalentExpressions`. Please find related conversation here: apache#40473 (comment)

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Existing UTs.

Closes apache#40475 from peter-toth/SPARK-42852-revert-namedlambdavariable-changes.

Authored-by: Peter Toth <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
(cherry picked from commit ce3b03d)
Signed-off-by: Hyukjin Kwon <[email protected]>
peter-toth added a commit to peter-toth/spark that referenced this pull request Nov 26, 2024
…uivalentExpressions

This PR reverts the follow-up PR of SPARK-41468: apache#39046

These changes are not needed and actually might cause performance regression due to preventing higher order function subexpression elimination in `EquivalentExpressions`. Please find related conversation here: apache#40473 (comment)

No.

Existing UTs.

Closes apache#40475 from peter-toth/SPARK-42852-revert-namedlambdavariable-changes.

Change-Id: Ia5ce83848956254664d9c51a2f0079bb968f5433
Authored-by: Peter Toth <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants