-
Notifications
You must be signed in to change notification settings - Fork 28k
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.
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
[SPARK-38085][SQL][FOLLOWUP] Do not fail too early for DeleteFromTable #36402
Conversation
It looks good to me. Just a bit confused by:
So, |
When |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @cloud-fan . +1, LGTM.
Merged to master and branch-3.3. |
### What changes were proposed in this pull request? `DeleteFromTable` has been in Spark for a long time and there are existing Spark extensions to compile `DeleteFromTable` to physical plans. However, the new analyzer rule `RewriteDeleteFromTable` fails very early if the v2 table does not support delete. This breaks certain Spark extensions which can still execute `DeleteFromTable` for certain v2 tables. This PR simply removes the error throwing in `RewriteDeleteFromTable`. It's a safe change because: 1. the new delete-related rules only match v2 table with `SupportsRowLevelOperations`, so won't be affected by this change 2. the planner rule will fail eventually if the v2 table doesn't support deletion. Spark eagerly executes commands so Spark users can still see this error immediately. ### Why are the changes needed? To not break existing Spark extesions. ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests Closes #36402 from cloud-fan/follow. Lead-authored-by: Wenchen Fan <wenchen@databricks.com> Co-authored-by: Wenchen Fan <cloud0fan@gmail.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit 5630f70) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
LGTM too. |
What changes were proposed in this pull request?
DeleteFromTable
has been in Spark for a long time and there are existing Spark extensions to compileDeleteFromTable
to physical plans. However, the new analyzer ruleRewriteDeleteFromTable
fails very early if the v2 table does not support delete. This breaks certain Spark extensions which can still executeDeleteFromTable
for certain v2 tables.This PR simply removes the error throwing in
RewriteDeleteFromTable
. It's a safe change because:SupportsRowLevelOperations
, so won't be affected by this changeWhy are the changes needed?
To not break existing Spark extesions.
Does this PR introduce any user-facing change?
no
How was this patch tested?
existing tests