Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit074c5cf

Browse files
committed
Fix handling of inherited check constraints in ALTER COLUMN TYPE (again).
The previous way of reconstructing check constraints was to do a separate"ALTER TABLE ONLY tab ADD CONSTRAINT" for each table in an inheritancehierarchy. However, that way has no hope of reconstructing the checkconstraints' own inheritance properties correctly, as pointed out inbug #13779 from Jan Dirk Zijlstra. What we should do instead is to doa regular "ALTER TABLE", allowing recursion, at the topmost table thathas a particular constraint, and then suppress the work queue entriesfor inherited instances of the constraint.Annoyingly, we'd tried to fix this behavior before, in commit5ed6546,but we failed to notice that it wasn't reconstructing the pg_constraintfield values correctly.As long as I'm touching pg_get_constraintdef_worker anyway, tweak it toalways schema-qualify the target table name; this seems like useful backupto the protections installed by commit5f17304.In HEAD/9.5, get rid of get_constraint_relation_oids, which is now unused.(I could alternatively have modified it to also return conislocal, but thatseemed like a pretty single-purpose API, so let's not pretend it has someother use.) It's unused in the back branches as well, but I left it inplace just in case some third-party code has decided to use it.In HEAD/9.5, also rename pg_get_constraintdef_string topg_get_constraintdef_command, as the previous name did nothing to explainwhat that entry point did differently from others (and its comment wasequally useless). Again, that change doesn't seem like material forback-patching.I did a bit of re-pgindenting in tablecmds.c in HEAD/9.5, as well.Otherwise, back-patch to all supported branches.
1 parent6c878a7 commit074c5cf

File tree

7 files changed

+252
-78
lines changed

7 files changed

+252
-78
lines changed

‎src/backend/catalog/pg_constraint.c

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -754,25 +754,6 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
754754
heap_close(conRel,RowExclusiveLock);
755755
}
756756

757-
/*
758-
* get_constraint_relation_oids
759-
*Find the IDs of the relations to which a constraint refers.
760-
*/
761-
void
762-
get_constraint_relation_oids(Oidconstraint_oid,Oid*conrelid,Oid*confrelid)
763-
{
764-
HeapTupletup;
765-
Form_pg_constraintcon;
766-
767-
tup=SearchSysCache1(CONSTROID,ObjectIdGetDatum(constraint_oid));
768-
if (!HeapTupleIsValid(tup))/* should not happen */
769-
elog(ERROR,"cache lookup failed for constraint %u",constraint_oid);
770-
con= (Form_pg_constraint)GETSTRUCT(tup);
771-
*conrelid=con->conrelid;
772-
*confrelid=con->confrelid;
773-
ReleaseSysCache(tup);
774-
}
775-
776757
/*
777758
* get_relation_constraint_oid
778759
*Find a constraint on the specified relation with the specified name.

‎src/backend/commands/tablecmds.c

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -328,9 +328,10 @@ static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recu
328328
boolis_view,AlterTableCmd*cmd,LOCKMODElockmode);
329329
staticObjectAddressATExecAddColumn(List**wqueue,AlteredTableInfo*tab,
330330
Relationrel,ColumnDef*colDef,boolisOid,
331-
boolrecurse,boolrecursing,boolif_not_exists,LOCKMODElockmode);
331+
boolrecurse,boolrecursing,
332+
boolif_not_exists,LOCKMODElockmode);
332333
staticboolcheck_for_column_name_collision(Relationrel,constchar*colname,
333-
boolif_not_exists);
334+
boolif_not_exists);
334335
staticvoidadd_column_datatype_dependency(Oidrelid,int32attnum,Oidtypid);
335336
staticvoidadd_column_collation_dependency(Oidrelid,int32attnum,Oidcollid);
336337
staticvoidATPrepAddOids(List**wqueue,Relationrel,boolrecurse,
@@ -3457,11 +3458,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
34573458
caseAT_AddColumnToView:/* add column via CREATE OR REPLACE
34583459
* VIEW */
34593460
address=ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3460-
false, false, false, false,lockmode);
3461+
false, false, false,
3462+
false,lockmode);
34613463
break;
34623464
caseAT_AddColumnRecurse:
34633465
address=ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3464-
false, true, false,cmd->missing_ok,lockmode);
3466+
false, true, false,
3467+
cmd->missing_ok,lockmode);
34653468
break;
34663469
caseAT_ColumnDefault:/* ALTER COLUMN DEFAULT */
34673470
address=ATExecColumnDefault(rel,cmd->name,cmd->def,lockmode);
@@ -3516,7 +3519,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
35163519
* constraint */
35173520
address=
35183521
ATExecAddConstraint(wqueue,tab,rel, (Constraint*)cmd->def,
3519-
false, true,lockmode);
3522+
true, true,lockmode);
35203523
break;
35213524
caseAT_ReAddComment:/* Re-add existing comment */
35223525
address=CommentObject((CommentStmt*)cmd->def);
@@ -3574,14 +3577,16 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
35743577
if (cmd->def!=NULL)
35753578
address=
35763579
ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3577-
true, false, false,cmd->missing_ok,lockmode);
3580+
true, false, false,
3581+
cmd->missing_ok,lockmode);
35783582
break;
35793583
caseAT_AddOidsRecurse:/* SET WITH OIDS */
35803584
/* Use the ADD COLUMN code, unless prep decided to do nothing */
35813585
if (cmd->def!=NULL)
35823586
address=
35833587
ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3584-
true, true, false,cmd->missing_ok,lockmode);
3588+
true, true, false,
3589+
cmd->missing_ok,lockmode);
35853590
break;
35863591
caseAT_DropOids:/* SET WITHOUT OIDS */
35873592

@@ -4691,7 +4696,8 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
46914696
staticObjectAddress
46924697
ATExecAddColumn(List**wqueue,AlteredTableInfo*tab,Relationrel,
46934698
ColumnDef*colDef,boolisOid,
4694-
boolrecurse,boolrecursing,boolif_not_exists,LOCKMODElockmode)
4699+
boolrecurse,boolrecursing,
4700+
boolif_not_exists,LOCKMODElockmode)
46954701
{
46964702
Oidmyrelid=RelationGetRelid(rel);
46974703
Relationpgclass,
@@ -6090,13 +6096,6 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
60906096
* AddRelationNewConstraints would normally assign different names to the
60916097
* child constraints. To fix that, we must capture the name assigned at
60926098
* the parent table and pass that down.
6093-
*
6094-
* When re-adding a previously existing constraint (during ALTER COLUMN TYPE),
6095-
* we don't need to recurse here, because recursion will be carried out at a
6096-
* higher level; the constraint name issue doesn't apply because the names
6097-
* have already been assigned and are just being re-used. We need a separate
6098-
* "is_readd" flag for that; just setting recurse=false would result in an
6099-
* error if there are child tables.
61006099
*/
61016100
staticObjectAddress
61026101
ATAddCheckConstraint(List**wqueue,AlteredTableInfo*tab,Relationrel,
@@ -6125,7 +6124,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
61256124
*/
61266125
newcons=AddRelationNewConstraints(rel,NIL,
61276126
list_make1(copyObject(constr)),
6128-
recursing,/* allow_merge */
6127+
recursing |is_readd,/* allow_merge */
61296128
!recursing,/* is_local */
61306129
is_readd);/* is_internal */
61316130

@@ -6174,10 +6173,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
61746173

61756174
/*
61766175
* If adding a NO INHERIT constraint, no need to find our children.
6177-
* Likewise, in a re-add operation, we don't need to recurse (that will be
6178-
* handled at higher levels).
61796176
*/
6180-
if (constr->is_no_inherit||is_readd)
6177+
if (constr->is_no_inherit)
61816178
returnaddress;
61826179

61836180
/*
@@ -8209,7 +8206,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
82098206
if (!list_member_oid(tab->changedConstraintOids,
82108207
foundObject.objectId))
82118208
{
8212-
char*defstring=pg_get_constraintdef_string(foundObject.objectId);
8209+
char*defstring=pg_get_constraintdef_command(foundObject.objectId);
82138210

82148211
/*
82158212
* Put NORMAL dependencies at the front of the list and
@@ -8584,10 +8581,30 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
85848581
def_item,tab->changedConstraintDefs)
85858582
{
85868583
OidoldId=lfirst_oid(oid_item);
8584+
HeapTupletup;
8585+
Form_pg_constraintcon;
85878586
Oidrelid;
85888587
Oidconfrelid;
8588+
boolconislocal;
8589+
8590+
tup=SearchSysCache1(CONSTROID,ObjectIdGetDatum(oldId));
8591+
if (!HeapTupleIsValid(tup))/* should not happen */
8592+
elog(ERROR,"cache lookup failed for constraint %u",oldId);
8593+
con= (Form_pg_constraint)GETSTRUCT(tup);
8594+
relid=con->conrelid;
8595+
confrelid=con->confrelid;
8596+
conislocal=con->conislocal;
8597+
ReleaseSysCache(tup);
8598+
8599+
/*
8600+
* If the constraint is inherited (only), we don't want to inject a
8601+
* new definition here; it'll get recreated when ATAddCheckConstraint
8602+
* recurses from adding the parent table's constraint. But we had to
8603+
* carry the info this far so that we can drop the constraint below.
8604+
*/
8605+
if (!conislocal)
8606+
continue;
85898607

8590-
get_constraint_relation_oids(oldId,&relid,&confrelid);
85918608
ATPostAlterTypeParse(oldId,relid,confrelid,
85928609
(char*)lfirst(def_item),
85938610
wqueue,lockmode,tab->rewrite);
@@ -8761,7 +8778,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
87618778
rel,con->conname);
87628779
}
87638780
else
8764-
elog(ERROR,"unexpected statementtype: %d",
8781+
elog(ERROR,"unexpected statementsubtype: %d",
87658782
(int)cmd->subtype);
87668783
}
87678784
}
@@ -11272,19 +11289,19 @@ ATPrepChangePersistence(Relation rel, bool toLogged)
1127211289
if (foreignrel->rd_rel->relpersistence!=RELPERSISTENCE_PERMANENT)
1127311290
ereport(ERROR,
1127411291
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
11275-
errmsg("could not change table \"%s\" to logged because it references unlogged table \"%s\"",
11276-
RelationGetRelationName(rel),
11277-
RelationGetRelationName(foreignrel)),
11292+
errmsg("could not change table \"%s\" to logged because it references unlogged table \"%s\"",
11293+
RelationGetRelationName(rel),
11294+
RelationGetRelationName(foreignrel)),
1127811295
errtableconstraint(rel,NameStr(con->conname))));
1127911296
}
1128011297
else
1128111298
{
1128211299
if (foreignrel->rd_rel->relpersistence==RELPERSISTENCE_PERMANENT)
1128311300
ereport(ERROR,
1128411301
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
11285-
errmsg("could not change table \"%s\" to unlogged because it references logged table \"%s\"",
11286-
RelationGetRelationName(rel),
11287-
RelationGetRelationName(foreignrel)),
11302+
errmsg("could not change table \"%s\" to unlogged because it references logged table \"%s\"",
11303+
RelationGetRelationName(rel),
11304+
RelationGetRelationName(foreignrel)),
1128811305
errtableconstraint(rel,NameStr(con->conname))));
1128911306
}
1129011307

‎src/backend/utils/adt/ruleutils.c

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ static Node *processIndirection(Node *node, deparse_context *context,
433433
staticvoidprintSubscripts(ArrayRef*aref,deparse_context*context);
434434
staticchar*get_relation_name(Oidrelid);
435435
staticchar*generate_relation_name(Oidrelid,List*namespaces);
436+
staticchar*generate_qualified_relation_name(Oidrelid);
436437
staticchar*generate_function_name(Oidfuncid,intnargs,
437438
List*argnames,Oid*argtypes,
438439
boolhas_variadic,bool*use_variadic_p,
@@ -1314,9 +1315,11 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
13141315
prettyFlags)));
13151316
}
13161317

1317-
/* Internal version that returns a palloc'd C string; no pretty-printing */
1318+
/*
1319+
* Internal version that returns a full ALTER TABLE ... ADD CONSTRAINT command
1320+
*/
13181321
char*
1319-
pg_get_constraintdef_string(OidconstraintId)
1322+
pg_get_constraintdef_command(OidconstraintId)
13201323
{
13211324
returnpg_get_constraintdef_worker(constraintId, true,0);
13221325
}
@@ -1363,10 +1366,16 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
13631366

13641367
initStringInfo(&buf);
13651368

1366-
if (fullCommand&&OidIsValid(conForm->conrelid))
1369+
if (fullCommand)
13671370
{
1368-
appendStringInfo(&buf,"ALTER TABLE ONLY %s ADD CONSTRAINT %s ",
1369-
generate_relation_name(conForm->conrelid,NIL),
1371+
/*
1372+
* Currently, callers want ALTER TABLE (without ONLY) for CHECK
1373+
* constraints, and other types of constraints don't inherit anyway so
1374+
* it doesn't matter whether we say ONLY or not. Someday we might
1375+
* need to let callers specify whether to put ONLY in the command.
1376+
*/
1377+
appendStringInfo(&buf,"ALTER TABLE %s ADD CONSTRAINT %s ",
1378+
generate_qualified_relation_name(conForm->conrelid),
13701379
quote_identifier(NameStr(conForm->conname)));
13711380
}
13721381

@@ -1890,28 +1899,9 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS)
18901899

18911900
if (OidIsValid(sequenceId))
18921901
{
1893-
HeapTupleclasstup;
1894-
Form_pg_classclasstuple;
1895-
char*nspname;
18961902
char*result;
18971903

1898-
/* Get the sequence's pg_class entry */
1899-
classtup=SearchSysCache1(RELOID,ObjectIdGetDatum(sequenceId));
1900-
if (!HeapTupleIsValid(classtup))
1901-
elog(ERROR,"cache lookup failed for relation %u",sequenceId);
1902-
classtuple= (Form_pg_class)GETSTRUCT(classtup);
1903-
1904-
/* Get the namespace */
1905-
nspname=get_namespace_name(classtuple->relnamespace);
1906-
if (!nspname)
1907-
elog(ERROR,"cache lookup failed for namespace %u",
1908-
classtuple->relnamespace);
1909-
1910-
/* And construct the result string */
1911-
result=quote_qualified_identifier(nspname,
1912-
NameStr(classtuple->relname));
1913-
1914-
ReleaseSysCache(classtup);
1904+
result=generate_qualified_relation_name(sequenceId);
19151905

19161906
PG_RETURN_TEXT_P(string_to_text(result));
19171907
}
@@ -9577,6 +9567,39 @@ generate_relation_name(Oid relid, List *namespaces)
95779567
returnresult;
95789568
}
95799569

9570+
/*
9571+
* generate_qualified_relation_name
9572+
*Compute the name to display for a relation specified by OID
9573+
*
9574+
* As above, but unconditionally schema-qualify the name.
9575+
*/
9576+
staticchar*
9577+
generate_qualified_relation_name(Oidrelid)
9578+
{
9579+
HeapTupletp;
9580+
Form_pg_classreltup;
9581+
char*relname;
9582+
char*nspname;
9583+
char*result;
9584+
9585+
tp=SearchSysCache1(RELOID,ObjectIdGetDatum(relid));
9586+
if (!HeapTupleIsValid(tp))
9587+
elog(ERROR,"cache lookup failed for relation %u",relid);
9588+
reltup= (Form_pg_class)GETSTRUCT(tp);
9589+
relname=NameStr(reltup->relname);
9590+
9591+
nspname=get_namespace_name(reltup->relnamespace);
9592+
if (!nspname)
9593+
elog(ERROR,"cache lookup failed for namespace %u",
9594+
reltup->relnamespace);
9595+
9596+
result=quote_qualified_identifier(nspname,relname);
9597+
9598+
ReleaseSysCache(tp);
9599+
9600+
returnresult;
9601+
}
9602+
95809603
/*
95819604
* generate_function_name
95829605
*Compute the name to display for a function specified by OID,

‎src/include/catalog/pg_constraint.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,6 @@ extern char *ChooseConstraintName(const char *name1, const char *name2,
247247

248248
externvoidAlterConstraintNamespaces(OidownerId,OidoldNspId,
249249
OidnewNspId,boolisType,ObjectAddresses*objsMoved);
250-
externvoidget_constraint_relation_oids(Oidconstraint_oid,Oid*conrelid,Oid*confrelid);
251250
externOidget_relation_constraint_oid(Oidrelid,constchar*conname,boolmissing_ok);
252251
externOidget_domain_constraint_oid(Oidtypid,constchar*conname,boolmissing_ok);
253252

‎src/include/utils/ruleutils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
externchar*pg_get_indexdef_string(Oidindexrelid);
2222
externchar*pg_get_indexdef_columns(Oidindexrelid,boolpretty);
2323

24-
externchar*pg_get_constraintdef_string(OidconstraintId);
24+
externchar*pg_get_constraintdef_command(OidconstraintId);
2525
externchar*deparse_expression(Node*expr,List*dpcontext,
2626
boolforceprefix,boolshowimplicit);
2727
externList*deparse_context_for(constchar*aliasname,Oidrelid);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp