From 2062ab9538b94f2f23e14cf2ba4d9cdac2b07601 Mon Sep 17 00:00:00 2001 From: "Mikhail A. Kulagin" Date: Mon, 28 Jun 2021 15:21:49 +0300 Subject: [PATCH 1/2] [PGPRO-5255] fix that ALTER TABLE IF EXISTS ... RENAME TO of not existed table generate ERROR instead of NOTICE --- expected/pathman_utility_stmt.out | 7 +++++++ sql/pathman_utility_stmt.sql | 6 ++++++ src/utility_stmt_hooking.c | 5 ++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/expected/pathman_utility_stmt.out b/expected/pathman_utility_stmt.out index 4cc4d493..0001b2f0 100644 --- a/expected/pathman_utility_stmt.out +++ b/expected/pathman_utility_stmt.out @@ -370,4 +370,11 @@ SELECT create_hash_partitions('drop_index.test', 'val', 2); DROP INDEX CONCURRENTLY drop_index.test_0_val_idx; DROP SCHEMA drop_index CASCADE; NOTICE: drop cascades to 3 other objects +/* + * 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; +NOTICE: relation "nonexistent_table" does not exist, skipping +DROP SCHEMA rename_nonexistent CASCADE; DROP EXTENSION pg_pathman; diff --git a/sql/pathman_utility_stmt.sql b/sql/pathman_utility_stmt.sql index 31232ce1..c5e940ce 100644 --- a/sql/pathman_utility_stmt.sql +++ b/sql/pathman_utility_stmt.sql @@ -250,6 +250,12 @@ DROP INDEX CONCURRENTLY drop_index.test_0_val_idx; DROP SCHEMA drop_index CASCADE; +/* + * 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; +DROP SCHEMA rename_nonexistent CASCADE; DROP EXTENSION pg_pathman; diff --git a/src/utility_stmt_hooking.c b/src/utility_stmt_hooking.c index c9ffbf14..8b160f64 100644 --- a/src/utility_stmt_hooking.c +++ b/src/utility_stmt_hooking.c @@ -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 */ + if (rename_stmt->missing_ok && relation_oid == InvalidOid) + return false; /* Assume it's a parent */ if (has_pathman_relation_info(relation_oid)) From 363efa969a473680f54314df0af0313c9d9dda8b Mon Sep 17 00:00:00 2001 From: "Mikhail A. Kulagin" Date: Mon, 28 Jun 2021 18:34:11 +0300 Subject: [PATCH 2/2] [PGPRO-5255] corrections based on the review --- expected/pathman_declarative.out | 4 ++ expected/pathman_declarative_1.out | 4 ++ expected/pathman_utility_stmt.out | 67 ++++++++++++++++++++++++++++-- sql/pathman_declarative.sql | 3 ++ sql/pathman_utility_stmt.sql | 49 ++++++++++++++++++++-- src/declarative.c | 6 ++- src/utility_stmt_hooking.c | 9 +++- 7 files changed, 131 insertions(+), 11 deletions(-) diff --git a/expected/pathman_declarative.out b/expected/pathman_declarative.out index 011a0f71..c13c0010 100644 --- a/expected/pathman_declarative.out +++ b/expected/pathman_declarative.out @@ -94,6 +94,10 @@ Check constraints: "pathman_r4_check" CHECK (dt >= '06-01-2015'::date AND dt < '01-01-2016'::date) Inherits: test.range_rel +ALTER TABLE IF EXISTS test.nonexistent_table ATTACH PARTITION baz DEFAULT; +NOTICE: relation "nonexistent_table" does not exist, skipping +ALTER TABLE IF EXISTS test.nonexistent_table DETACH PARTITION baz; +NOTICE: relation "nonexistent_table" does not exist, skipping DROP SCHEMA test CASCADE; NOTICE: drop cascades to 8 other objects DROP EXTENSION pg_pathman CASCADE; diff --git a/expected/pathman_declarative_1.out b/expected/pathman_declarative_1.out index 8ef4e556..d720d335 100644 --- a/expected/pathman_declarative_1.out +++ b/expected/pathman_declarative_1.out @@ -94,6 +94,10 @@ Check constraints: "pathman_r4_check" CHECK (dt >= '06-01-2015'::date AND dt < '01-01-2016'::date) Inherits: test.range_rel +ALTER TABLE IF EXISTS test.nonexistent_table ATTACH PARTITION baz DEFAULT; +NOTICE: relation "nonexistent_table" does not exist, skipping +ALTER TABLE IF EXISTS test.nonexistent_table DETACH PARTITION baz; +NOTICE: relation "nonexistent_table" does not exist, skipping DROP SCHEMA test CASCADE; NOTICE: drop cascades to 8 other objects DROP EXTENSION pg_pathman CASCADE; diff --git a/expected/pathman_utility_stmt.out b/expected/pathman_utility_stmt.out index 0001b2f0..6e137b37 100644 --- a/expected/pathman_utility_stmt.out +++ b/expected/pathman_utility_stmt.out @@ -371,10 +371,69 @@ DROP INDEX CONCURRENTLY drop_index.test_0_val_idx; DROP SCHEMA drop_index CASCADE; NOTICE: drop cascades to 3 other objects /* - * Test, that ALTER TABLE IF EXISTS ... RENAME TO of not existed table generate NOTICE instead of ERROR + * Checking that ALTER TABLE IF EXISTS with loaded (and created) pg_pathman extension works the same as in vanilla */ -CREATE SCHEMA rename_nonexistent; -ALTER TABLE IF EXISTS rename_nonexistent.nonexistent_table RENAME TO other_table_name; +CREATE SCHEMA test_nonexistance; +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table RENAME TO other_table_name; NOTICE: relation "nonexistent_table" does not exist, skipping -DROP SCHEMA rename_nonexistent CASCADE; +/* renaming existent tables already tested earlier (see rename.plain_test) */ +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table ADD COLUMN IF NOT EXISTS j INT4; +NOTICE: relation "nonexistent_table" does not exist, skipping +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table ADD COLUMN IF NOT EXISTS i INT4; +NOTICE: column "i" of relation "existent_table" already exists, skipping +ALTER TABLE IF EXISTS test_nonexistance.existent_table ADD COLUMN IF NOT EXISTS j INT4; +SELECT attname FROM pg_attribute WHERE attnum > 0 AND attrelid = 'test_nonexistance.existent_table'::REGCLASS; + attname +--------- + i + j +(2 rows) + +DROP TABLE test_nonexistance.existent_table; +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table DROP COLUMN IF EXISTS i; +NOTICE: relation "nonexistent_table" does not exist, skipping +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table DROP COLUMN IF EXISTS i; +ALTER TABLE IF EXISTS test_nonexistance.existent_table DROP COLUMN IF EXISTS nonexistent_column; +NOTICE: column "nonexistent_column" of relation "existent_table" does not exist, skipping +SELECT attname FROM pg_attribute WHERE attnum > 0 AND attrelid = 'test_nonexistance.existent_table'::REGCLASS; + attname +------------------------------ + ........pg.dropped.1........ +(1 row) + +DROP TABLE test_nonexistance.existent_table; +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table RENAME COLUMN i TO j; +NOTICE: relation "nonexistent_table" does not exist, skipping +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table RENAME COLUMN i TO j; +SELECT attname FROM pg_attribute WHERE attnum > 0 AND attrelid = 'test_nonexistance.existent_table'::REGCLASS; + attname +--------- + j +(1 row) + +DROP TABLE test_nonexistance.existent_table; +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table RENAME CONSTRAINT baz TO bar; +NOTICE: relation "nonexistent_table" does not exist, skipping +CREATE TABLE test_nonexistance.existent_table(i INT4 CONSTRAINT existent_table_i_check CHECK (i < 100)); +ALTER TABLE IF EXISTS test_nonexistance.existent_table RENAME CONSTRAINT existent_table_i_check TO existent_table_i_other_check; +DROP TABLE test_nonexistance.existent_table; +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table SET SCHEMA nonexistent_schema; +NOTICE: relation "nonexistent_table" does not exist, skipping +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table SET SCHEMA nonexistent_schema; +ERROR: schema "nonexistent_schema" does not exist +CREATE SCHEMA test_nonexistance2; +ALTER TABLE IF EXISTS test_nonexistance.existent_table SET SCHEMA test_nonexistance2; +DROP TABLE test_nonexistance2.existent_table; +DROP SCHEMA test_nonexistance2 CASCADE; +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table SET TABLESPACE nonexistent_tablespace; +NOTICE: relation "nonexistent_table" does not exist, skipping +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table SET TABLESPACE nonexistent_tablespace; +ERROR: tablespace "nonexistent_tablespace" does not exist +DROP TABLE test_nonexistance.existent_table; +DROP SCHEMA test_nonexistance CASCADE; DROP EXTENSION pg_pathman; diff --git a/sql/pathman_declarative.sql b/sql/pathman_declarative.sql index 864e3af8..347627a7 100644 --- a/sql/pathman_declarative.sql +++ b/sql/pathman_declarative.sql @@ -39,6 +39,9 @@ CREATE TABLE test.r4 PARTITION OF test.range_rel FOR VALUES FROM ('2015-06-01') TO ('2016-01-01'); \d+ test.r4; +ALTER TABLE IF EXISTS test.nonexistent_table ATTACH PARTITION baz DEFAULT; +ALTER TABLE IF EXISTS test.nonexistent_table DETACH PARTITION baz; + DROP SCHEMA test CASCADE; DROP EXTENSION pg_pathman CASCADE; DROP SCHEMA pathman CASCADE; diff --git a/sql/pathman_utility_stmt.sql b/sql/pathman_utility_stmt.sql index c5e940ce..c0832f34 100644 --- a/sql/pathman_utility_stmt.sql +++ b/sql/pathman_utility_stmt.sql @@ -251,11 +251,52 @@ DROP INDEX CONCURRENTLY drop_index.test_0_val_idx; DROP SCHEMA drop_index CASCADE; /* - * Test, that ALTER TABLE IF EXISTS ... RENAME TO of not existed table generate NOTICE instead of ERROR + * Checking that ALTER TABLE IF EXISTS with loaded (and created) pg_pathman extension works the same as in vanilla */ -CREATE SCHEMA rename_nonexistent; -ALTER TABLE IF EXISTS rename_nonexistent.nonexistent_table RENAME TO other_table_name; -DROP SCHEMA rename_nonexistent CASCADE; +CREATE SCHEMA test_nonexistance; + +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table RENAME TO other_table_name; +/* renaming existent tables already tested earlier (see rename.plain_test) */ + +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table ADD COLUMN IF NOT EXISTS j INT4; +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table ADD COLUMN IF NOT EXISTS i INT4; +ALTER TABLE IF EXISTS test_nonexistance.existent_table ADD COLUMN IF NOT EXISTS j INT4; +SELECT attname FROM pg_attribute WHERE attnum > 0 AND attrelid = 'test_nonexistance.existent_table'::REGCLASS; +DROP TABLE test_nonexistance.existent_table; + +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table DROP COLUMN IF EXISTS i; +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table DROP COLUMN IF EXISTS i; +ALTER TABLE IF EXISTS test_nonexistance.existent_table DROP COLUMN IF EXISTS nonexistent_column; +SELECT attname FROM pg_attribute WHERE attnum > 0 AND attrelid = 'test_nonexistance.existent_table'::REGCLASS; +DROP TABLE test_nonexistance.existent_table; + +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table RENAME COLUMN i TO j; +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table RENAME COLUMN i TO j; +SELECT attname FROM pg_attribute WHERE attnum > 0 AND attrelid = 'test_nonexistance.existent_table'::REGCLASS; +DROP TABLE test_nonexistance.existent_table; + +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table RENAME CONSTRAINT baz TO bar; +CREATE TABLE test_nonexistance.existent_table(i INT4 CONSTRAINT existent_table_i_check CHECK (i < 100)); +ALTER TABLE IF EXISTS test_nonexistance.existent_table RENAME CONSTRAINT existent_table_i_check TO existent_table_i_other_check; +DROP TABLE test_nonexistance.existent_table; + +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table SET SCHEMA nonexistent_schema; +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table SET SCHEMA nonexistent_schema; +CREATE SCHEMA test_nonexistance2; +ALTER TABLE IF EXISTS test_nonexistance.existent_table SET SCHEMA test_nonexistance2; +DROP TABLE test_nonexistance2.existent_table; +DROP SCHEMA test_nonexistance2 CASCADE; + +ALTER TABLE IF EXISTS test_nonexistance.nonexistent_table SET TABLESPACE nonexistent_tablespace; +CREATE TABLE test_nonexistance.existent_table(i INT4); +ALTER TABLE IF EXISTS test_nonexistance.existent_table SET TABLESPACE nonexistent_tablespace; +DROP TABLE test_nonexistance.existent_table; + +DROP SCHEMA test_nonexistance CASCADE; DROP EXTENSION pg_pathman; diff --git a/src/declarative.c b/src/declarative.c index ca4fe165..367df752 100644 --- a/src/declarative.c +++ b/src/declarative.c @@ -75,7 +75,11 @@ is_pathman_related_partitioning_cmd(Node *parsetree, Oid *parent_relid) AlterTableStmt *stmt = (AlterTableStmt *) parsetree; int cnt = 0; - *parent_relid = RangeVarGetRelid(stmt->relation, NoLock, false); + *parent_relid = RangeVarGetRelid(stmt->relation, NoLock, stmt->missing_ok); + + if (stmt->missing_ok && *parent_relid == InvalidOid) + return false; + if ((prel = get_pathman_relation_info(*parent_relid)) == NULL) return false; diff --git a/src/utility_stmt_hooking.c b/src/utility_stmt_hooking.c index 8b160f64..1949d970 100644 --- a/src/utility_stmt_hooking.c +++ b/src/utility_stmt_hooking.c @@ -176,7 +176,8 @@ is_pathman_related_table_rename(Node *parsetree, relation_oid = RangeVarGetRelid(rename_stmt->relation, AccessShareLock, rename_stmt->missing_ok); - /* PGPRO-5255: check ALTER TABLE IF EXISTS of non existent table */ + + /* Check ALTER TABLE ... IF EXISTS of nonexistent table */ if (rename_stmt->missing_ok && relation_oid == InvalidOid) return false; @@ -235,7 +236,11 @@ is_pathman_related_alter_column_type(Node *parsetree, /* Assume it's a parent, fetch its Oid */ parent_relid = RangeVarGetRelid(alter_table_stmt->relation, AccessShareLock, - false); + alter_table_stmt->missing_ok); + + /* Check ALTER TABLE ... IF EXISTS of nonexistent table */ + if (alter_table_stmt->missing_ok && parent_relid == InvalidOid) + return false; /* Is parent partitioned? */ if ((prel = get_pathman_relation_info(parent_relid)) != NULL)