database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-03 09:13:20 +03:00
Code Activity

Fix ALTER TABLE code to update domain constraints when needed.

Browse Source 
It's possible for dropping a column, or altering its type, to require
changes in domain CHECK constraint expressions; but the code was
previously only expecting to find dependent table CHECK constraints.
Make the necessary adjustments.

This is a fairly old oversight, but it's a lot easier to encounter
the problem in the context of domains over composite types than it
was before.  Given the lack of field complaints, I'm not going to
bother with a back-patch, though I'd be willing to reconsider that
decision if someone does complain.

Patch by me, reviewed by Michael Paquier

Discussion: https://postgr.es/m/30656.1509128130@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2017-11-01 13:32:23 -04:00
parent 387ec70322
commit af20e2d728
5 changed files with 186 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
src/backend/commands/tablecmds.c
View File

@@ -425,7 +425,8 @@ 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);
Oid objid, Relation rel, List *domname,
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,
@@ -3319,6 +3320,7 @@ AlterTableGetLockLevel(List *cmds)
case AT_ProcessedConstraint: /* becomes AT_AddConstraint */
case AT_AddConstraintRecurse: /* becomes AT_AddConstraint */
case AT_ReAddConstraint: /* becomes AT_AddConstraint */
case AT_ReAddDomainConstraint: /* becomes AT_AddConstraint */
if (IsA(cmd->def, Constraint))
{
Constraint *con = (Constraint *) cmd->def;
@@ -3819,7 +3821,9 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode)
rel = relation_open(tab->relid, NoLock);
foreach(lcmd, subcmds)
ATExecCmd(wqueue, tab, rel, (AlterTableCmd *) lfirst(lcmd), lockmode);
ATExecCmd(wqueue, tab, rel,
castNode(AlterTableCmd, lfirst(lcmd)),
lockmode);
/*
* After the ALTER TYPE pass, do cleanup work (this is not done in
@@ -3936,6 +3940,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
true, true, lockmode);
break;
case AT_ReAddDomainConstraint: /* Re-add pre-existing domain check
* constraint */
address =
AlterDomainAddConstraint(((AlterDomainStmt *) cmd->def)->typeName,
((AlterDomainStmt *) cmd->def)->def,
NULL);
break;
case AT_ReAddComment: /* Re-add existing comment */
address = CommentObject((CommentStmt *) cmd->def);
break;
@@ -9616,7 +9627,15 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
if (!HeapTupleIsValid(tup)) /* should not happen */
elog(ERROR, "cache lookup failed for constraint %u", oldId);
con = (Form_pg_constraint) GETSTRUCT(tup);
relid = con->conrelid;
if (OidIsValid(con->conrelid))
relid = con->conrelid;
else
{
/* must be a domain constraint */
relid = get_typ_typrelid(getBaseType(con->contypid));
if (!OidIsValid(relid))
elog(ERROR, "could not identify relation associated with constraint %u", oldId);
}
confrelid = con->confrelid;
conislocal = con->conislocal;
ReleaseSysCache(tup);
@@ -9753,7 +9772,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
foreach(lcmd, stmt->cmds)
{
AlterTableCmd *cmd = (AlterTableCmd *) lfirst(lcmd);
AlterTableCmd *cmd = castNode(AlterTableCmd, lfirst(lcmd));
if (cmd->subtype == AT_AddIndex)
{
@@ -9777,13 +9796,14 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
RebuildConstraintComment(tab,
AT_PASS_OLD_INDEX,
oldId,
rel, indstmt->idxname);
rel,
NIL,
indstmt->idxname);
}
else if (cmd->subtype == AT_AddConstraint)
{
Constraint *con;
Constraint *con = castNode(Constraint, cmd->def);
con = castNode(Constraint, cmd->def);
con->old_pktable_oid = refRelId;
/* rewriting neither side of a FK */
if (con->contype == CONSTR_FOREIGN &&
@@ -9797,13 +9817,41 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
RebuildConstraintComment(tab,
AT_PASS_OLD_CONSTR,
oldId,
rel, con->conname);
rel,
NIL,
con->conname);
}
else
elog(ERROR, "unexpected statement subtype: %d",
(int) cmd->subtype);
}
}
else if (IsA(stm, AlterDomainStmt))
{
AlterDomainStmt *stmt = (AlterDomainStmt *) stm;
if (stmt->subtype == 'C') /* ADD CONSTRAINT */
{
Constraint *con = castNode(Constraint, stmt->def);
AlterTableCmd *cmd = makeNode(AlterTableCmd);
cmd->subtype = AT_ReAddDomainConstraint;
cmd->def = (Node *) stmt;
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,
NULL,
stmt->typeName,
con->conname);
}
else
elog(ERROR, "unexpected statement subtype: %d",
(int) stmt->subtype);
}
else
elog(ERROR, "unexpected statement type: %d",
(int) nodeTag(stm));
@@ -9813,12 +9861,19 @@ 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.
* Subroutine for ATPostAlterTypeParse() to recreate any existing comment
* for a table or domain constraint that is being rebuilt.
*
* objid is the OID of the constraint.
* Pass "rel" for a table constraint, or "domname" (domain's qualified name
* as a string list) for a domain constraint.
* (We could dig that info, as well as the conname, out of the pg_constraint
* entry; but callers already have them so might as well pass them.)
*/
static void
RebuildConstraintComment(AlteredTableInfo *tab, int pass, Oid objid,
Relation rel, char *conname)
Relation rel, List *domname,
char *conname)
{
CommentStmt *cmd;
char *comment_str;
@@ -9829,12 +9884,23 @@ RebuildConstraintComment(AlteredTableInfo *tab, int pass, Oid objid,
if (comment_str == NULL)
return;
/* Build node CommentStmt */
/* Build CommentStmt node, copying all input data for safety */
cmd = makeNode(CommentStmt);
cmd->objtype = OBJECT_TABCONSTRAINT;
cmd->object = (Node *) list_make3(makeString(get_namespace_name(RelationGetNamespace(rel))),
makeString(pstrdup(RelationGetRelationName(rel))),
makeString(pstrdup(conname)));
if (rel)
{
cmd->objtype = OBJECT_TABCONSTRAINT;
cmd->object = (Node *)
list_make3(makeString(get_namespace_name(RelationGetNamespace(rel))),
makeString(pstrdup(RelationGetRelationName(rel))),
makeString(pstrdup(conname)));
}
else
{
cmd->objtype = OBJECT_DOMCONSTRAINT;
cmd->object = (Node *)
list_make2(makeTypeNameFromNameList(copyObject(domname)),
makeString(pstrdup(conname)));
}
cmd->comment = comment_str;
/* Append it to list of commands */

67
src/backend/utils/adt/ruleutils.c
View File

@@ -460,6 +460,7 @@ static char *generate_function_name(Oid funcid, int nargs,
bool has_variadic, bool *use_variadic_p,
ParseExprKind special_exprkind);
static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2);
static char *generate_qualified_type_name(Oid typid);
static text *string_to_text(char *str);
static char *flatten_reloptions(Oid relid);
@@ -1867,15 +1868,27 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
if (fullCommand)
{
/*
* Currently, callers want ALTER TABLE (without ONLY) for CHECK
* constraints, and other types of constraints don't inherit anyway so
* it doesn't matter whether we say ONLY or not. Someday we might
* need to let callers specify whether to put ONLY in the command.
*/
appendStringInfo(&buf, "ALTER TABLE %s ADD CONSTRAINT %s ",
generate_qualified_relation_name(conForm->conrelid),
quote_identifier(NameStr(conForm->conname)));
if (OidIsValid(conForm->conrelid))
{
/*
* Currently, callers want ALTER TABLE (without ONLY) for CHECK
* constraints, and other types of constraints don't inherit
* anyway so it doesn't matter whether we say ONLY or not. Someday
* we might need to let callers specify whether to put ONLY in the
* command.
*/
appendStringInfo(&buf, "ALTER TABLE %s ADD CONSTRAINT %s ",
generate_qualified_relation_name(conForm->conrelid),
quote_identifier(NameStr(conForm->conname)));
}
else
{
/* Must be a domain constraint */
Assert(OidIsValid(conForm->contypid));
appendStringInfo(&buf, "ALTER DOMAIN %s ADD CONSTRAINT %s ",
generate_qualified_type_name(conForm->contypid),
quote_identifier(NameStr(conForm->conname)));
}
}
switch (conForm->contype)
@@ -10778,6 +10791,42 @@ generate_operator_name(Oid operid, Oid arg1, Oid arg2)
return buf.data;
}
/*
* generate_qualified_type_name
* Compute the name to display for a type specified by OID
*
* This is different from format_type_be() in that we unconditionally
* schema-qualify the name. That also means no special syntax for
* SQL-standard type names ... although in current usage, this should
* only get used for domains, so such cases wouldn't occur anyway.
*/
static char *
generate_qualified_type_name(Oid typid)
{
HeapTuple tp;
Form_pg_type typtup;
char *typname;
char *nspname;
char *result;
tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typid));
if (!HeapTupleIsValid(tp))
elog(ERROR, "cache lookup failed for type %u", typid);
typtup = (Form_pg_type) GETSTRUCT(tp);
typname = NameStr(typtup->typname);
nspname = get_namespace_name(typtup->typnamespace);
if (!nspname)
elog(ERROR, "cache lookup failed for namespace %u",
typtup->typnamespace);
result = quote_qualified_identifier(nspname, typname);
ReleaseSysCache(tp);
return result;
}
/*
* generate_collation_name
* Compute the name to display for a collation specified by OID