-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-36011][SQL] Disallow altering permanent views based on temporary views or UDFs #33204
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
Conversation
|
Can one of the admins verify this patch? |
|
@jerqi can you create a new JIRA instead of reusing SPARK-18217? The fixed versions would be different at least. |
OK, It's done |
sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewSuite.scala
Outdated
Show resolved
Hide resolved
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
Outdated
Show resolved
Hide resolved
linhongliu-db
left a comment
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.
lgtm, cc @cloud-fan
| withTable(table) { | ||
| (1 to 10).toDF("id").write.saveAsTable(table) | ||
| val view = "v1" | ||
| val tView = "v2" |
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.
nit: I feel it's clearer to just hardcode "t", "v1", "v2"
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.
OK
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
Show resolved
Hide resolved
|
thanks, merging to master/3.2! |
…ry views or UDFs ### What changes were proposed in this pull request? PR #15764 disabled creating permanent views based on temporary views or UDFs. But AlterViewCommand didn't block temporary objects. ### Why are the changes needed? More robust view canonicalization. ### Does this PR introduce _any_ user-facing change? Yes, now if you alter a permanent view based on temporary views or UDFs, the operation will fail. ### How was this patch tested? Add new unit tests. Closes #33204 from jerqi/alter_view. Authored-by: RoryQi <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit e0c6b2e) Signed-off-by: Wenchen Fan <[email protected]>
|
@jerqi can you help to create backport PRs for 3.1 and 3.0? thanks! |
Ok, it's my pleasure. |
What changes were proposed in this pull request?
PR #15764 disabled creating permanent views based on temporary views or UDFs. But AlterViewCommand didn't block temporary objects.
Why are the changes needed?
More robust view canonicalization.
Does this PR introduce any user-facing change?
Yes, now if you alter a permanent view based on temporary views or UDFs, the operation will fail.
How was this patch tested?
Add new unit tests.