diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressPartitions.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressPartitions.java index 399799044cf6..471da1f1f079 100644 --- a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressPartitions.java +++ b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressPartitions.java @@ -23,7 +23,7 @@ public class TestPgRegressPartitions extends BasePgSQLTest { @Override public int getTestMethodTimeoutSec() { - return getPerfMaxRuntime(500, 700, 900, 900, 900); + return getPerfMaxRuntime(500, 1000, 1200, 1200, 1200); } @Test diff --git a/src/postgres/src/backend/commands/tablecmds.c b/src/postgres/src/backend/commands/tablecmds.c index be55644303e4..0365479c7f80 100644 --- a/src/postgres/src/backend/commands/tablecmds.c +++ b/src/postgres/src/backend/commands/tablecmds.c @@ -371,7 +371,7 @@ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode); static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, - YBCPgStatement *rollbackHandle); + List **rollbackHandles); static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation *mutable_rel, AlterTableCmd *cmd, LOCKMODE lockmode); static void ATRewriteTables(AlterTableStmt *parsetree, @@ -3199,6 +3199,17 @@ renameatt(RenameStmt *stmt) if (IsYugaByteEnabled()) { YBCRename(stmt, relid); + if (stmt->renameType == OBJECT_COLUMN) + { + ListCell *child; + List *children = find_all_inheritors(relid, NoLock, NULL); + foreach(child, children) + { + Oid childrelid = lfirst_oid(child); + if (childrelid != relid) + YBCRename(stmt, childrelid); + } + } } ObjectAddressSubSet(address, RelationRelationId, relid, attnum); @@ -3931,13 +3942,13 @@ ATController(AlterTableStmt *parsetree, relation_close(rel, NoLock); /* Phase 2: update system catalogs */ - YBCPgStatement rollbackHandle = NULL; + List *rollbackHandles = NIL; /* * ATRewriteCatalogs also executes changes to DocDB. * If Phase 3 fails, rollbackHandle will specify how to rollback the * changes done to DocDB. */ - ATRewriteCatalogs(&wqueue, lockmode, &rollbackHandle); + ATRewriteCatalogs(&wqueue, lockmode, &rollbackHandles); /* Phase 3: scan/rewrite tables as needed */ PG_TRY(); @@ -3947,9 +3958,11 @@ ATController(AlterTableStmt *parsetree, PG_CATCH(); { /* Rollback the DocDB changes. */ - if (rollbackHandle) + ListCell *lc = NULL; + foreach(lc, rollbackHandles) { - YBCExecAlterTable(rollbackHandle, RelationGetRelid(rel)); + YBCPgStatement handle = (YBCPgStatement) lfirst(lc); + YBCExecAlterTable(handle, RelationGetRelid(rel)); } PG_RE_THROW(); } @@ -4272,7 +4285,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, - YBCPgStatement *rollbackHandle) + List **rollbackHandles) { int pass; ListCell *ltab; @@ -4288,10 +4301,42 @@ ATRewriteCatalogs(List **wqueue, */ AlteredTableInfo* info = (AlteredTableInfo *) linitial(*wqueue); Oid main_relid = info->relid; - YBCPgStatement handle = YBCPrepareAlterTable(info->subcmds, - AT_NUM_PASSES, - main_relid, - rollbackHandle); + YBCPgStatement rollbackHandle = NULL; + List *handles = NIL; + YBCPgStatement handle = YBCPrepareAlterTable(info->subcmds, + AT_NUM_PASSES, + main_relid, + &rollbackHandle, + false /* isPartitionOfAlteredTable */); + handles = lappend(handles, handle); + if (rollbackHandle) + *rollbackHandles = lappend(*rollbackHandles, rollbackHandle); + + /* + * If this is a partitioned table, prepare alter table handles for all + * the partitions as well. + */ + List *children = find_all_inheritors(main_relid, NoLock, NULL); + ListCell *child; + foreach(child, children) + { + Oid childrelid = lfirst_oid(child); + /* + * find_all_inheritors also returns the oid of the table itself. + * Skip it, as we have already added handles for it. + */ + if (childrelid == main_relid) + continue; + YBCPgStatement childRollbackHandle = NULL; + YBCPgStatement child_handle = YBCPrepareAlterTable(info->subcmds, + AT_NUM_PASSES, + childrelid, + &childRollbackHandle, + true /*isPartitionOfAlteredTable */); + handles = lappend(handles, child_handle); + if (childRollbackHandle) + *rollbackHandles = lappend(*rollbackHandles, childRollbackHandle); + } /* * We process all the tables "in parallel", one pass at a time. This is @@ -4300,6 +4345,7 @@ ATRewriteCatalogs(List **wqueue, * re-adding of the foreign key constraint to the other table). Work can * only be propagated into later passes, however. */ + ListCell *lc = NULL; for (pass = 0; pass < AT_NUM_PASSES; pass++) { /* @@ -4311,9 +4357,13 @@ ATRewriteCatalogs(List **wqueue, * However, we must also do this before we start adding indexes because * the column in question might not be there yet. */ - if (pass == AT_PASS_ADD_INDEX && handle) + if (pass == AT_PASS_ADD_INDEX) { - YBCExecAlterTable(handle, main_relid); + foreach(lc, handles) + { + YBCPgStatement handle = (YBCPgStatement) lfirst(lc); + YBCExecAlterTable(handle, main_relid); + } } /* Go through each table that needs to be processed */ diff --git a/src/postgres/src/backend/commands/ybccmds.c b/src/postgres/src/backend/commands/ybccmds.c index cfe85e100fcb..80c45dbf18f2 100644 --- a/src/postgres/src/backend/commands/ybccmds.c +++ b/src/postgres/src/backend/commands/ybccmds.c @@ -888,9 +888,32 @@ YBCCreateIndex(const char *indexName, static void YBCPrepareAlterTableCmd(AlterTableCmd* cmd, Relation rel, YBCPgStatement handle, - int* col, bool* needsYBAlter, - YBCPgStatement* rollbackHandle) + int* col, bool* needsYBAlter, + YBCPgStatement* rollbackHandle, + bool isPartitionOfAlteredTable) { + if (isPartitionOfAlteredTable) + { + /* + * This function was invoked on a child partition table to reflect + * the effects of Alter on its parent. + */ + switch (cmd->subtype) + { + case AT_AddColumnRecurse: + case AT_DropColumnRecurse: + case AT_AddConstraintRecurse: + case AT_DropConstraintRecurse: + break; + default: + /* + * This is not an alter command on a partitioned table that + * needs to trickle down to its child partitions. Nothing to + * do. + */ + return; + } + } Oid relationId = RelationGetRelid(rel); switch (cmd->subtype) { @@ -920,7 +943,7 @@ YBCPrepareAlterTableCmd(AlterTableCmd* cmd, Relation rel, YBCPgStatement handle, const YBCPgTypeEntity *col_type = YbDataTypeFromOidMod(order, typeOid); HandleYBStatus(YBCPgAlterTableAddColumn(handle, colDef->colname, - order, col_type)); + order, col_type)); ++(*col); ReleaseSysCache(typeTuple); *needsYBAlter = true; @@ -1098,7 +1121,8 @@ YBCPgStatement YBCPrepareAlterTable(List** subcmds, int subcmds_size, Oid relationId, - YBCPgStatement *rollbackHandle) + YBCPgStatement *rollbackHandle, + bool isPartitionOfAlteredTable) { /* Appropriate lock was already taken */ Relation rel = relation_open(relationId, NoLock); @@ -1123,7 +1147,8 @@ YBCPrepareAlterTable(List** subcmds, foreach(lcmd, subcmds[cmd_idx]) { YBCPrepareAlterTableCmd((AlterTableCmd *) lfirst(lcmd), rel, handle, - &col, &needsYBAlter, rollbackHandle); + &col, &needsYBAlter, rollbackHandle, + isPartitionOfAlteredTable); } } relation_close(rel, NoLock); diff --git a/src/postgres/src/include/commands/ybccmds.h b/src/postgres/src/include/commands/ybccmds.h index 4b57369bedb9..9f5663a9a0c3 100644 --- a/src/postgres/src/include/commands/ybccmds.h +++ b/src/postgres/src/include/commands/ybccmds.h @@ -81,7 +81,8 @@ extern void YBCDropIndex(Oid relationId); extern YBCPgStatement YBCPrepareAlterTable(List** subcmds, int subcmds_size, Oid relationId, - YBCPgStatement *rollbackHandle); + YBCPgStatement *rollbackHandle, + bool isPartitionOfAlteredTable); extern void YBCExecAlterTable(YBCPgStatement handle, Oid relationId); diff --git a/src/postgres/src/test/regress/expected/yb_partition_alter.out b/src/postgres/src/test/regress/expected/yb_partition_alter.out new file mode 100644 index 000000000000..acff33b5d97d --- /dev/null +++ b/src/postgres/src/test/regress/expected/yb_partition_alter.out @@ -0,0 +1,61 @@ +-- Create a partitioned hierarchy of LIST, RANGE and HASH. +CREATE TABLE root_list_parent (list_part_key char, hash_part_key int, range_part_key int) PARTITION BY LIST(list_part_key); +CREATE TABLE hash_parent PARTITION OF root_list_parent FOR VALUES in ('a', 'b') PARTITION BY HASH (hash_part_key); +CREATE TABLE range_parent PARTITION OF hash_parent FOR VALUES WITH (modulus 1, remainder 0) PARTITION BY RANGE (range_part_key); +CREATE TABLE child_partition PARTITION OF range_parent FOR VALUES FROM (1) TO (5); +INSERT INTO root_list_parent VALUES ('a', 1, 2); +-- Add a column to the parent table, verify that selecting data still works. +ALTER TABLE root_list_parent ADD COLUMN foo VARCHAR(2); +SELECT * FROM root_list_parent; + list_part_key | hash_part_key | range_part_key | foo +---------------+---------------+----------------+----- + a | 1 | 2 | +(1 row) + +-- Alter column type at the parent table. +ALTER TABLE root_list_parent ALTER COLUMN foo TYPE VARCHAR(3); +INSERT INTO root_list_parent VALUES ('a', 1, 2, 'abc'); +SELECT * FROM root_list_parent ORDER BY foo; + list_part_key | hash_part_key | range_part_key | foo +---------------+---------------+----------------+----- + a | 1 | 2 | abc + a | 1 | 2 | +(2 rows) + +-- Drop a column from the parent table, verify that selecting data still works. +ALTER TABLE root_list_parent DROP COLUMN foo; +SELECT * FROM root_list_parent; + list_part_key | hash_part_key | range_part_key +---------------+---------------+---------------- + a | 1 | 2 + a | 1 | 2 +(2 rows) + +-- Retry adding a column after error. +ALTER TABLE root_list_parent ADD COLUMN foo text not null; -- fails due to not null constraint +ERROR: column "foo" contains null values +ALTER TABLE root_list_parent ADD COLUMN foo text not null DEFAULT 'abc'; -- passes +-- Rename a column belonging to the parent table. +ALTER TABLE root_list_parent RENAME COLUMN list_part_key TO list_part_key_renamed; +-- Note: Incorrect output here for 'foo' because of #9970. +SELECT * FROM child_partition ORDER BY foo; + list_part_key_renamed | hash_part_key | range_part_key | foo +-----------------------+---------------+----------------+----- + a | 1 | 2 | + a | 1 | 2 | +(2 rows) + +TRUNCATE root_list_parent; +-- Add constraint to the parent table, verify that it reflects on the child partition. +ALTER TABLE root_list_parent ADD CONSTRAINT constraint_test UNIQUE (list_part_key_renamed, hash_part_key, range_part_key, foo); +INSERT INTO child_partition VALUES ('a', 1, 2), ('a', 1, 2); +ERROR: duplicate key value violates unique constraint "child_partition_list_part_key_renamed_hash_part_key_range_p_key" +-- Remove constraint from the parent table, verify that it reflects on the child partition. +ALTER TABLE root_list_parent DROP CONSTRAINT constraint_test; +INSERT INTO child_partition VALUES ('a', 1, 2), ('a', 1, 2); +SELECT * FROM root_list_parent ORDER BY foo; + list_part_key_renamed | hash_part_key | range_part_key | foo +-----------------------+---------------+----------------+----- + a | 1 | 2 | abc + a | 1 | 2 | abc +(2 rows) diff --git a/src/postgres/src/test/regress/sql/yb_partition_alter.sql b/src/postgres/src/test/regress/sql/yb_partition_alter.sql new file mode 100644 index 000000000000..7b66208d487a --- /dev/null +++ b/src/postgres/src/test/regress/sql/yb_partition_alter.sql @@ -0,0 +1,38 @@ +-- Create a partitioned hierarchy of LIST, RANGE and HASH. +CREATE TABLE root_list_parent (list_part_key char, hash_part_key int, range_part_key int) PARTITION BY LIST(list_part_key); +CREATE TABLE hash_parent PARTITION OF root_list_parent FOR VALUES in ('a', 'b') PARTITION BY HASH (hash_part_key); +CREATE TABLE range_parent PARTITION OF hash_parent FOR VALUES WITH (modulus 1, remainder 0) PARTITION BY RANGE (range_part_key); +CREATE TABLE child_partition PARTITION OF range_parent FOR VALUES FROM (1) TO (5); +INSERT INTO root_list_parent VALUES ('a', 1, 2); + +-- Add a column to the parent table, verify that selecting data still works. +ALTER TABLE root_list_parent ADD COLUMN foo VARCHAR(2); +SELECT * FROM root_list_parent; + +-- Alter column type at the parent table. +ALTER TABLE root_list_parent ALTER COLUMN foo TYPE VARCHAR(3); +INSERT INTO root_list_parent VALUES ('a', 1, 2, 'abc'); +SELECT * FROM root_list_parent ORDER BY foo; + +-- Drop a column from the parent table, verify that selecting data still works. +ALTER TABLE root_list_parent DROP COLUMN foo; +SELECT * FROM root_list_parent; + +-- Retry adding a column after error. +ALTER TABLE root_list_parent ADD COLUMN foo text not null; -- fails due to not null constraint +ALTER TABLE root_list_parent ADD COLUMN foo text not null DEFAULT 'abc'; -- passes + +-- Rename a column belonging to the parent table. +ALTER TABLE root_list_parent RENAME COLUMN list_part_key TO list_part_key_renamed; +-- Note: Incorrect output here for 'foo' because of #9970. +SELECT * FROM child_partition ORDER BY foo; +TRUNCATE root_list_parent; + +-- Add constraint to the parent table, verify that it reflects on the child partition. +ALTER TABLE root_list_parent ADD CONSTRAINT constraint_test UNIQUE (list_part_key_renamed, hash_part_key, range_part_key, foo); +INSERT INTO child_partition VALUES ('a', 1, 2), ('a', 1, 2); + +-- Remove constraint from the parent table, verify that it reflects on the child partition. +ALTER TABLE root_list_parent DROP CONSTRAINT constraint_test; +INSERT INTO child_partition VALUES ('a', 1, 2), ('a', 1, 2); +SELECT * FROM root_list_parent ORDER BY foo; diff --git a/src/postgres/src/test/regress/yb_partitions_schedule b/src/postgres/src/test/regress/yb_partitions_schedule index ccf273ed84a7..f99fe1f1fa72 100644 --- a/src/postgres/src/test/regress/yb_partitions_schedule +++ b/src/postgres/src/test/regress/yb_partitions_schedule @@ -6,3 +6,4 @@ ############################################################################### test: yb_partition_fk test: yb_partition_pk +test: yb_partition_alter