Skip to content

[SPARK-30600][SQL] Migrate ALTER VIEW SET/UNSET commands to the new framework#27314

Closed
imback82 wants to merge 3 commits intoapache:masterfrom
imback82:alter_view_new_framework
Closed

[SPARK-30600][SQL] Migrate ALTER VIEW SET/UNSET commands to the new framework#27314
imback82 wants to merge 3 commits intoapache:masterfrom
imback82:alter_view_new_framework

Conversation

@imback82
Copy link
Contributor

@imback82 imback82 commented Jan 21, 2020

What changes were proposed in this pull request?

Use the new framework to resolve the ALTER VIEW SET/UNSET TBLPROPERTIES commands.

This PR also introduces UnresolvedView and UnresolvedViewWithTableExists to the framework to handle commands with views.

Why are the changes needed?

This is a part of effort to make the relation lookup behavior consistent: SPARK-29900.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added new tests.

@SparkQA
Copy link

SparkQA commented Jan 22, 2020

Test build #117204 has finished for PR 27314 at commit d3b3c27.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 22, 2020

Test build #117202 has finished for PR 27314 at commit da90079.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@imback82
Copy link
Contributor Author

cc @cloud-fan

val cleanedTableProperties = cleanTableProperties(ctx, properties)
if (ctx.VIEW != null) {
AlterViewSetPropertiesStatement(identifier, cleanedTableProperties)
AlterTableSetProperties(UnresolvedView(identifier), cleanedTableProperties)
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we use AlterViewSetProperties?

val ifExists = ctx.EXISTS != null
if (ctx.VIEW != null) {
AlterViewUnsetPropertiesStatement(identifier, cleanedProperties, ifExists)
AlterTableUnsetProperties(UnresolvedView(identifier), cleanedProperties, ifExists)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@cloud-fan
Copy link
Contributor

I think we shouldn't reuse the AlterTableXYZ commands for views. We should create new AlterViewXYZ commands, although they don't have physical plans and always fails during analysis.

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label May 16, 2020
@github-actions github-actions bot closed this May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants