Skip to content

Conversation

@imback82
Copy link
Contributor

What changes were proposed in this pull request?

This PR address: #27243 (comment).

The error is now the same whether a table is resolved to a view or a temp view for ALTER TABLE commands.

Why are the changes needed?

To unify the error message regardless of whether a table is resolved to a view or a temp view for ALTER TABLE commands.

Does this PR introduce any user-facing change?

The previous error message:

temp_view is a temp view not a table.

New error message (same as view):

Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead

How was this patch tested?

Updated existing tests to reflect the new error message.

@SparkQA
Copy link

SparkQA commented Jan 22, 2020

Test build #117205 has finished for PR 27315 at commit 7109bdc.

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

-- !query 20 output
org.apache.spark.sql.AnalysisException
temp_view is a temp view not a table.; line 1 pos 0
Cannot alter a view with ALTER TABLE. Please use ALTER VIEW instead; line 1 pos 0
Copy link
Member

Choose a reason for hiding this comment

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

Hi, @cloud-fan and @imback82
Cannot alter a view with ALTER TABLE. looks okay, but Please use ALTER VIEW instead looks like we mislead the users to try ALTER VIEW temp_view CHANGE a TYPE INT COMMENT 'this is column a'.

Copy link
Member

@dongjoon-hyun dongjoon-hyun Jan 22, 2020

Choose a reason for hiding this comment

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

The other changes in this PR are the same. The directional message parts look misleading.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good point. How about just Cannot alter a temp view with ALTER TABLE. as the error message?

Copy link
Member

Choose a reason for hiding this comment

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

+1 for @cloud-fan 's advice.

@brkyvz
Copy link
Contributor

brkyvz commented Jan 22, 2020

cc @imback82 @cloud-fan I'm planning to revert #27243 because of reasons described in #27243 (comment), therefore please hold off on merging this PR.

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.

5 participants