-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-21904] [SQL] Rename tempTables to tempViews in SessionCatalog #19117
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
|
Test build #81375 has finished for PR 19117 at commit
|
|
Test build #81384 has finished for PR 19117 at commit
|
| new SQLConf().copy(SQLConf.CASE_SENSITIVE -> true)) | ||
| } | ||
|
|
||
| /** List of temporary tables, mapping from table name to their logical plan. */ |
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.
The code comment at the beginning of SessionCatalog also includes wording ...it also manages temporary tables and ...
|
LGTM |
| } | ||
| assert(e.getMessage.contains( | ||
| "RENAME TEMPORARY TABLE from '`tab1`' to '`default`.`tab2`': " + | ||
| "RENAME TEMPORARY VIEW from '`tab1`' to '`default`.`tab2`': " + |
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.
We can rename this test name from rename temporary table... to ``rename temporary view...`.
| } | ||
| assert(e.getMessage.contains( | ||
| "RENAME TEMPORARY TABLE from '`tab1`' to '`tab2`': destination table already exists")) | ||
| "RENAME TEMPORARY VIEW from '`tab1`' to '`tab2`': destination table already exists")) |
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.
ditto. This test name is also rename temporary table...
| "RENAME TEMPORARY VIEW from '`tab1`' to '`default`.`tab2`': " + | ||
| "cannot specify database name 'default' in the destination table")) | ||
|
|
||
| val catalog = spark.sessionState.catalog |
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.
At L823 there is a test named rename temporary table too.
|
Test build #81433 has finished for PR 19117 at commit
|
|
try rebasing? it should be fixed by this commit 36b48ee |
|
Test build #82337 has finished for PR 19117 at commit
|
|
Thanks! Merged to master. |
What changes were proposed in this pull request?
tempTablesis not right. To be consistent, we need to rename the internal variable names/comments to tempViews in SessionCatalog too.How was this patch tested?
N/A