diff options
| -rw-r--r-- | src/backend/commands/tablecmds.c | 65 | ||||
| -rw-r--r-- | src/include/nodes/parsenodes.h | 1 | ||||
| -rw-r--r-- | src/test/regress/expected/alter_table.out | 63 | ||||
| -rw-r--r-- | src/test/regress/sql/alter_table.sql | 36 |
4 files changed, 163 insertions, 2 deletions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e7b23f1621c..1c7eded9a79 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -386,6 +386,8 @@ static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, List **wqueue, LOCKMODE lockmode, bool rewrite); +static void RebuildConstraintComment(AlteredTableInfo *tab, int pass, + Oid objid, Relation rel, char *conname); static void TryReuseIndex(Oid oldId, IndexStmt *stmt); static void TryReuseForeignKey(Oid oldId, Constraint *con); static void change_owner_fix_column_acls(Oid relationOid, @@ -3514,6 +3516,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, false, true, lockmode); break; + case AT_ReAddComment: /* Re-add existing comment */ + address = CommentObject((CommentStmt *) cmd->def); + break; case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */ address = ATExecAddIndexConstraint(tab, rel, (IndexStmt *) cmd->def, lockmode); @@ -8654,6 +8659,8 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, if (!rewrite) TryReuseIndex(oldId, stmt); + /* keep the index's comment */ + stmt->idxcomment = GetComment(oldId, RelationRelationId, 0); newcmd = makeNode(AlterTableCmd); newcmd->subtype = AT_ReAddIndex; @@ -8672,15 +8679,29 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, if (cmd->subtype == AT_AddIndex) { + IndexStmt *indstmt; + Oid indoid; + Assert(IsA(cmd->def, IndexStmt)); + indstmt = (IndexStmt *) cmd->def; + indoid = get_constraint_index(oldId); + if (!rewrite) - TryReuseIndex(get_constraint_index(oldId), - (IndexStmt *) cmd->def); + TryReuseIndex(indoid, indstmt); + /* keep any comment on the index */ + indstmt->idxcomment = GetComment(indoid, + RelationRelationId, 0); cmd->subtype = AT_ReAddIndex; tab->subcmds[AT_PASS_OLD_INDEX] = lappend(tab->subcmds[AT_PASS_OLD_INDEX], cmd); + + /* recreate any comment on the constraint */ + RebuildConstraintComment(tab, + AT_PASS_OLD_INDEX, + oldId, + rel, indstmt->idxname); } else if (cmd->subtype == AT_AddConstraint) { @@ -8697,6 +8718,12 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, cmd->subtype = AT_ReAddConstraint; tab->subcmds[AT_PASS_OLD_CONSTR] = lappend(tab->subcmds[AT_PASS_OLD_CONSTR], cmd); + + /* recreate any comment on the constraint */ + RebuildConstraintComment(tab, + AT_PASS_OLD_CONSTR, + oldId, + rel, con->conname); } else elog(ERROR, "unexpected statement type: %d", @@ -8712,6 +8739,40 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, } /* + * Subroutine for ATPostAlterTypeParse() to recreate a comment entry for + * a constraint that is being re-added. + */ +static void +RebuildConstraintComment(AlteredTableInfo *tab, int pass, Oid objid, + Relation rel, char *conname) +{ + CommentStmt *cmd; + char *comment_str; + AlterTableCmd *newcmd; + + /* Look for comment for object wanted, and leave if none */ + comment_str = GetComment(objid, ConstraintRelationId, 0); + if (comment_str == NULL) + return; + + /* Build node CommentStmt */ + cmd = makeNode(CommentStmt); + cmd->objtype = OBJECT_TABCONSTRAINT; + cmd->objname = list_make3( + makeString(get_namespace_name(RelationGetNamespace(rel))), + makeString(RelationGetRelationName(rel)), + makeString(conname)); + cmd->objargs = NIL; + cmd->comment = comment_str; + + /* Append it to list of commands */ + newcmd = makeNode(AlterTableCmd); + newcmd->subtype = AT_ReAddComment; + newcmd->def = (Node *) cmd; + tab->subcmds[pass] = lappend(tab->subcmds[pass], newcmd); +} + +/* * Subroutine for ATPostAlterTypeParse(). Calls out to CheckIndexCompatible() * for the real analysis, then mutates the IndexStmt based on that verdict. */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 868905b0c16..a567c50da72 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1474,6 +1474,7 @@ typedef enum AlterTableType AT_AddConstraint, /* add constraint */ AT_AddConstraintRecurse, /* internal to commands/tablecmds.c */ AT_ReAddConstraint, /* internal to commands/tablecmds.c */ + AT_ReAddComment, /* internal to commands/tablecmds.c */ AT_AlterConstraint, /* alter constraint */ AT_ValidateConstraint, /* validate constraint */ AT_ValidateConstraintRecurse, /* internal to commands/tablecmds.c */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 3ad2c55775c..6b9291b7242 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2400,6 +2400,69 @@ Check constraints: DROP TABLE alter2.tt8; DROP SCHEMA alter2; +-- Check that comments on constraints and indexes are not lost at ALTER TABLE. +CREATE TABLE comment_test ( + id int, + positive_col int CHECK (positive_col > 0), + indexed_col int, + CONSTRAINT comment_test_pk PRIMARY KEY (id)); +CREATE INDEX comment_test_index ON comment_test(indexed_col); +COMMENT ON COLUMN comment_test.id IS 'Column ''id'' on comment_test'; +COMMENT ON INDEX comment_test_index IS 'Simple index on comment_test'; +COMMENT ON CONSTRAINT comment_test_positive_col_check ON comment_test IS 'CHECK constraint on comment_test.positive_col'; +COMMENT ON CONSTRAINT comment_test_pk ON comment_test IS 'PRIMARY KEY constraint of comment_test'; +COMMENT ON INDEX comment_test_pk IS 'Index backing the PRIMARY KEY of comment_test'; +SELECT col_description('comment_test'::regclass, 1) as comment; + comment +----------------------------- + Column 'id' on comment_test +(1 row) + +SELECT indexrelid::regclass as index, obj_description(indexrelid, 'pg_class') as comment FROM pg_index where indrelid = 'comment_test'::regclass; + index | comment +--------------------+----------------------------------------------- + comment_test_index | Simple index on comment_test + comment_test_pk | Index backing the PRIMARY KEY of comment_test +(2 rows) + +SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment FROM pg_constraint where conrelid = 'comment_test'::regclass; + constraint | comment +---------------------------------+----------------------------------------------- + comment_test_pk | PRIMARY KEY constraint of comment_test + comment_test_positive_col_check | CHECK constraint on comment_test.positive_col +(2 rows) + +-- Change the datatype of all the columns. ALTER TABLE is optimized to not +-- rebuild an index if the new data type is binary compatible with the old +-- one. Check do a dummy ALTER TABLE that doesn't change the datatype +-- first, to test that no-op codepath, and another one that does. +ALTER TABLE comment_test ALTER COLUMN indexed_col SET DATA TYPE int; +ALTER TABLE comment_test ALTER COLUMN indexed_col SET DATA TYPE text; +ALTER TABLE comment_test ALTER COLUMN id SET DATA TYPE int; +ALTER TABLE comment_test ALTER COLUMN id SET DATA TYPE text; +ALTER TABLE comment_test ALTER COLUMN positive_col SET DATA TYPE int; +ALTER TABLE comment_test ALTER COLUMN positive_col SET DATA TYPE bigint; +-- Check that the comments are intact. +SELECT col_description('comment_test'::regclass, 1) as comment; + comment +----------------------------- + Column 'id' on comment_test +(1 row) + +SELECT indexrelid::regclass as index, obj_description(indexrelid, 'pg_class') as comment FROM pg_index where indrelid = 'comment_test'::regclass; + index | comment +--------------------+----------------------------------------------- + comment_test_pk | Index backing the PRIMARY KEY of comment_test + comment_test_index | Simple index on comment_test +(2 rows) + +SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment FROM pg_constraint where conrelid = 'comment_test'::regclass; + constraint | comment +---------------------------------+----------------------------------------------- + comment_test_positive_col_check | CHECK constraint on comment_test.positive_col + comment_test_pk | PRIMARY KEY constraint of comment_test +(2 rows) + -- Check that we map relation oids to filenodes and back correctly. Only -- display bad mappings so the test output doesn't change all the time. A -- filenode function call can return NULL for a relation dropped concurrently diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 29c1875d2ea..9f755fae33f 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1594,6 +1594,42 @@ ALTER TABLE IF EXISTS tt8 SET SCHEMA alter2; DROP TABLE alter2.tt8; DROP SCHEMA alter2; + +-- Check that comments on constraints and indexes are not lost at ALTER TABLE. +CREATE TABLE comment_test ( + id int, + positive_col int CHECK (positive_col > 0), + indexed_col int, + CONSTRAINT comment_test_pk PRIMARY KEY (id)); +CREATE INDEX comment_test_index ON comment_test(indexed_col); + +COMMENT ON COLUMN comment_test.id IS 'Column ''id'' on comment_test'; +COMMENT ON INDEX comment_test_index IS 'Simple index on comment_test'; +COMMENT ON CONSTRAINT comment_test_positive_col_check ON comment_test IS 'CHECK constraint on comment_test.positive_col'; +COMMENT ON CONSTRAINT comment_test_pk ON comment_test IS 'PRIMARY KEY constraint of comment_test'; +COMMENT ON INDEX comment_test_pk IS 'Index backing the PRIMARY KEY of comment_test'; + +SELECT col_description('comment_test'::regclass, 1) as comment; +SELECT indexrelid::regclass as index, obj_description(indexrelid, 'pg_class') as comment FROM pg_index where indrelid = 'comment_test'::regclass; +SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment FROM pg_constraint where conrelid = 'comment_test'::regclass; + +-- Change the datatype of all the columns. ALTER TABLE is optimized to not +-- rebuild an index if the new data type is binary compatible with the old +-- one. Check do a dummy ALTER TABLE that doesn't change the datatype +-- first, to test that no-op codepath, and another one that does. +ALTER TABLE comment_test ALTER COLUMN indexed_col SET DATA TYPE int; +ALTER TABLE comment_test ALTER COLUMN indexed_col SET DATA TYPE text; +ALTER TABLE comment_test ALTER COLUMN id SET DATA TYPE int; +ALTER TABLE comment_test ALTER COLUMN id SET DATA TYPE text; +ALTER TABLE comment_test ALTER COLUMN positive_col SET DATA TYPE int; +ALTER TABLE comment_test ALTER COLUMN positive_col SET DATA TYPE bigint; + +-- Check that the comments are intact. +SELECT col_description('comment_test'::regclass, 1) as comment; +SELECT indexrelid::regclass as index, obj_description(indexrelid, 'pg_class') as comment FROM pg_index where indrelid = 'comment_test'::regclass; +SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment FROM pg_constraint where conrelid = 'comment_test'::regclass; + + -- Check that we map relation oids to filenodes and back correctly. Only -- display bad mappings so the test output doesn't change all the time. A -- filenode function call can return NULL for a relation dropped concurrently |