Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

[PGPRO-5255] fix that ALTER TABLE IF EXISTS ... RENAME TO of not exis… #228

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

Merged
merged 2 commits into from
Jun 28, 2021

Conversation

kulaginm
Copy link
Member

…ted table generate ERROR instead of NOTICE

@kulaginm kulaginm self-assigned this Jun 28, 2021
@kulaginm kulaginm requested a review from ziva777 June 28, 2021 12:33
@@ -175,7 +175,10 @@ is_pathman_related_table_rename(Node *parsetree,
/* Fetch Oid of this relation */
relation_oid = RangeVarGetRelid(rename_stmt->relation,
AccessShareLock,
false);
rename_stmt->missing_ok);
/* PGPRO-5255: check ALTER TABLE IF EXISTS of non existent table */
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably, I'd not use problem numbers here. Otherwise all the code would look like this in near future. Just simple description why this code is here would be enough.

* Test, that ALTER TABLE IF EXISTS ... RENAME TO of not existed table generate NOTICE instead of ERROR
*/
CREATE SCHEMA rename_nonexistent;
ALTER TABLE IF EXISTS rename_nonexistent.nonexistent_table RENAME TO other_table_name;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add some more cases here?

ALTER TABLE IF EXISTS foo SET SCHEMA baz;
ALTER TABLE IF EXISTS foo RENAME COLUMN baz TO bar;
DROP TABLE IF EXISTS foo;```

Copy link
Contributor

@ziva777 ziva777 left a comment

Choose a reason for hiding this comment

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

That looks good for me.

@ziva777 ziva777 merged commit 8cf5f29 into master Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants