This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit bcee4cb23494d265502497be77457bd1fbc8f8b2 Author: Huansong Fu <[email protected]> AuthorDate: Tue Oct 11 15:35:24 2022 -0700 Make ALTER TABLE ... OWNER recurse by default In GPDB6 ALTER TABLE ... OWNER on partition root recurses into child tables. However, the upstream does not do the same and during merging, GPDB7 lost the recurse behavior and became inconsistent with GPDB6. We have decided to restore that behavior for GPDB7. We will support the 'ONLY' keyword for ALTER TABLE ... OWNER so that the user has the option to just alter the root, which will be done & verified with the 'ONLY' support for other commands altogether. Have to adjust the vacuum test case after this change. Discussion thread: https://groups.google.com/a/greenplum.org/g/gpdb-dev/c/ImJrvQEECwA fix vacuum --- src/backend/commands/tablecmds.c | 5 ++-- src/test/regress/expected/alter_table.out | 37 +++++++++++++++++++++++++ src/test/regress/expected/vacuum.out | 4 ++- src/test/regress/sql/alter_table.sql | 18 ++++++++++++ src/test/regress/sql/vacuum.sql | 4 ++- src/test/singlenode_regress/expected/vacuum.out | 4 ++- src/test/singlenode_regress/sql/vacuum.sql | 4 ++- 7 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index bb0f4a435d..14c3c0490a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5548,8 +5548,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, pass = AT_PASS_MISC; break; case AT_ChangeOwner: /* ALTER OWNER */ - /* This command never recurses */ - /* No command-specific prep needed */ + /* GPDB: we have historically been performing recurse by default for partition tables. */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); pass = AT_PASS_MISC; break; case AT_ClusterOn: /* CLUSTER ON */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 2d460feea6..209655f655 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -4763,3 +4763,40 @@ create index float2double_table_idx_c1c2c3 on float2double_table(c1,c2,c3); create unique index float2double_table_uniqidx_c1c2c3 on float2double_table(c1,c2,c3); ALTER TABLE float2double_table ALTER COLUMN c1 TYPE double precision; DROP TABLE float2double_table; +-- Test that altering owner of partition root should recurse into the child tables. +create role atown_r1; +create role atown_r2 in role atown_r1; +set role atown_r2; +create table atown_part(a int, b int) partition by range(a) (partition p1 start (1) end (100)); +select c.relname, r.rolname from pg_class c join pg_roles r on c.relowner = r.oid where relname like 'atown_part%'; + relname | rolname +---------------------+---------- + atown_part | atown_r2 + atown_part_1_prt_p1 | atown_r2 +(2 rows) + +alter table atown_part owner to atown_r1; +alter table atown_part add partition start(100) end(200); +-- both existing and new child tables should have the new owner +select c.relname, r.rolname from pg_class c join pg_roles r on c.relowner = r.oid where relname like 'atown_part%'; + relname | rolname +---------------------+---------- + atown_part | atown_r1 + atown_part_1_prt_1 | atown_r1 + atown_part_1_prt_p1 | atown_r1 +(3 rows) + +-- should only alter the partition root with ONLY keyword +alter table only atown_part owner to atown_r2; +select c.relname, r.rolname from pg_class c join pg_roles r on c.relowner = r.oid where relname like 'atown_part%'; + relname | rolname +---------------------+---------- + atown_part | atown_r2 + atown_part_1_prt_1 | atown_r1 + atown_part_1_prt_p1 | atown_r1 +(3 rows) + +drop table atown_part; +reset role; +drop role atown_r1; +drop role atown_r2; diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index bf5afe9aaf..ce164e1224 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -348,7 +348,7 @@ WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum RESET ROLE; -- Partitioned table and one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; -ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it @@ -368,6 +368,7 @@ WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum RESET ROLE; -- Only one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; SET ROLE regress_vacuum; VACUUM vacowned_parted; WARNING: skipping "vacowned_parted" --- only table or database owner can vacuum it @@ -391,6 +392,7 @@ RESET ROLE; -- Only partitioned table owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 6561e6b4ff..ed8941a890 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -3131,3 +3131,21 @@ create index float2double_table_idx_c1c2c3 on float2double_table(c1,c2,c3); create unique index float2double_table_uniqidx_c1c2c3 on float2double_table(c1,c2,c3); ALTER TABLE float2double_table ALTER COLUMN c1 TYPE double precision; DROP TABLE float2double_table; +-- Test that altering owner of partition root should recurse into the child tables. +create role atown_r1; +create role atown_r2 in role atown_r1; +set role atown_r2; +create table atown_part(a int, b int) partition by range(a) (partition p1 start (1) end (100)); +select c.relname, r.rolname from pg_class c join pg_roles r on c.relowner = r.oid where relname like 'atown_part%'; +alter table atown_part owner to atown_r1; +alter table atown_part add partition start(100) end(200); +-- both existing and new child tables should have the new owner +select c.relname, r.rolname from pg_class c join pg_roles r on c.relowner = r.oid where relname like 'atown_part%'; +-- should only alter the partition root with ONLY keyword +alter table only atown_part owner to atown_r2; +select c.relname, r.rolname from pg_class c join pg_roles r on c.relowner = r.oid where relname like 'atown_part%'; + +drop table atown_part; +reset role; +drop role atown_r1; +drop role atown_r2; diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index 758b508a57..8e7afab68b 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -281,7 +281,7 @@ VACUUM (ANALYZE) vacowned_part2; RESET ROLE; -- Partitioned table and one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; -ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; VACUUM vacowned_part1; @@ -295,6 +295,7 @@ VACUUM (ANALYZE) vacowned_part2; RESET ROLE; -- Only one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; SET ROLE regress_vacuum; VACUUM vacowned_parted; VACUUM vacowned_part1; @@ -309,6 +310,7 @@ RESET ROLE; -- Only partitioned table owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; VACUUM vacowned_part1; diff --git a/src/test/singlenode_regress/expected/vacuum.out b/src/test/singlenode_regress/expected/vacuum.out index 112fb54807..f93b74e4c3 100644 --- a/src/test/singlenode_regress/expected/vacuum.out +++ b/src/test/singlenode_regress/expected/vacuum.out @@ -346,7 +346,7 @@ WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum RESET ROLE; -- Partitioned table and one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; -ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it @@ -366,6 +366,7 @@ WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum RESET ROLE; -- Only one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; SET ROLE regress_vacuum; VACUUM vacowned_parted; WARNING: skipping "vacowned_parted" --- only table or database owner can vacuum it @@ -389,6 +390,7 @@ RESET ROLE; -- Only partitioned table owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it diff --git a/src/test/singlenode_regress/sql/vacuum.sql b/src/test/singlenode_regress/sql/vacuum.sql index 9faa8a34a6..b14ab576b2 100644 --- a/src/test/singlenode_regress/sql/vacuum.sql +++ b/src/test/singlenode_regress/sql/vacuum.sql @@ -276,7 +276,7 @@ VACUUM (ANALYZE) vacowned_part2; RESET ROLE; -- Partitioned table and one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; -ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; VACUUM vacowned_part1; @@ -290,6 +290,7 @@ VACUUM (ANALYZE) vacowned_part2; RESET ROLE; -- Only one partition owned by other user. ALTER TABLE vacowned_parted OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part1 OWNER TO regress_vacuum; SET ROLE regress_vacuum; VACUUM vacowned_parted; VACUUM vacowned_part1; @@ -304,6 +305,7 @@ RESET ROLE; -- Only partitioned table owned by other user. ALTER TABLE vacowned_parted OWNER TO regress_vacuum; ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER; +ALTER TABLE vacowned_part2 OWNER TO CURRENT_USER; SET ROLE regress_vacuum; VACUUM vacowned_parted; VACUUM vacowned_part1; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
