Sorry about the long delay in answering that, I hope to get to a consensus
on how to do that feature, which I think it is really valuable. Sending few
options and observations bellow...
On Sun, Jul 28, 2019 at 2:37 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> Matheus de Oliveira <matioli.math...@gmail.com> writes:
> > [ postgresql-alter-constraint.v5.patch ]
>
> Somebody seems to have marked this Ready For Committer without posting
> any review, which is not very kosher,
Sorry. I know Lucas, will talk to him for a better review ;D
> but I took a quick look at it
> anyway.
>
>
Thank you so much by that.
* It's failing to apply, as noted by the cfbot, because somebody added
> an unrelated test to the same spot in foreign_key.sql. I fixed that
> in the attached rebase.
>
>
That was a mistake on rebase, sorry.
> * It also doesn't pass "git diff --check" whitespace checks, so
> I ran it through pgindent.
>
>
Still learning here, will take more care.
> * Grepping around for other references to struct Constraint,
> I noticed that you'd missed updating equalfuncs.c. I did not
> fix that.
>
>
Certainly true, I fixed that just to keep it OK for now.
> The main issue I've got though is a definitional one --- I'm not at all
> sold on your premise that constraint deferrability syntax should mean
> different things depending on the previous state of the constraint.
>
I see the point, but I really believe we should have a simpler way to
change just specific properties
of the constraint without touching the others, and I do believe it is
valuable. So I'd like to check with
you all what would be a good option to have that.
Just as a demonstration, and a PoC, I have changed the patch to accept two
different syntaxes:
1. The one we have today with ALTER CONSTRAINT, and it change every
constraint property
2. A similar one with SET keyword in the middle, to force changing only the
given properties, e.g.:
ALTER TABLE table_name ALTER CONSTRAINT constr_name *SET* ON UPDATE
CASCADE;
I'm not at all happy with the syntax, doens't seem very clear. But I
proceeded this way nonetheless
just to verify the code on tablecmds.c would work. Please, does NOT
consider the patch as "ready",
it is more like a WIP and demonstration now (specially the test part, which
is no longer complete,
and gram.y that I changed the lazy way - both easy to fix if the syntax is
good).
I would really appreciate opinions on that, and I'm glad to work on a
better patch after we decide
the best syntax and approach.
> We don't generally act that way in other ALTER commands
That is true. I think one exception is ALTER COLUMN, which just acts on the
changes explicitly provided.
And I truly believe most people would expect changes on only provided
information on ALTER CONSTRAINT
as well. But I have no real research on that, more like a feeling :P
> and I don't see
> a strong argument to start doing so here. If you didn't do this then
> you wouldn't (I think) need the extra struct Constraint fields in the
> first place, which is why I didn't run off and change equalfuncs.c.
>
>
Indeed true, changes on `Constraint` struct were only necessary due to
that, the patch would in fact
be way simpler without it (that is why I still insist on finding some way
to make it happen, perhaps
with a better syntax).
> In short, I'm inclined to argue that this variant of ALTER TABLE
> should replace *all* the fields of the constraint with the same
> properties it'd have if you'd created it fresh using the same syntax.
> This is by analogy to CREATE OR REPLACE commands, which don't
> preserve any of the old properties of the replaced object.
I agree for CREATE OR REPLACE, but in my POV REPLACE makes it clearer to
the user that
*everything* is changed, ALTER not so much. Again, this is just *my
opinion*, not a very strong
one though, but following first messages on that thread current behaviour
can be easily confused
with a bug (although it is not, the code clear shows it is expected,
specially on tests).
> Given
> the interactions between these fields, I think you're going to end up
> with a surprising mess of ad-hoc choices if you do differently.
> Indeed, you already have, but I think it'll get worse if anyone
> tries to extend the feature set further.
>
>
Certainly agree with that, the code is harder that way, as I said above.
Still thinking that
having the option is valuable though, we should be able to find a better
syntax/approach
for that.
> Perhaps the right way to attack it, given that, is to go ahead and
> invent "ALTER TABLE t ADD OR REPLACE CONSTRAINT c ...". At least
> in the case at hand with FK constraints, we could apply suitable
> optimizations (ie skip revalidation) when the new definition shares
> the right properties with the old, and otherwise treat it like a
> drop-and-add.
>
I believe this path is quite easy for me to do now, if you all agree it is
a good approach.
What worries me is that we already have ALTER CONSTRAINT syntax, so what
would
we do with that? I see a few options:
1. Leave ALTER CONSTRAINT to only change given properties (as I proposed at
first), and let
ADD OR REPLACE to do a full change
2. Have only ADD OR REPLACE and deprecate ALTER CONSTRAINT (I think it is
too harsh
for users already using it, a big compatibility change)
3. Just have both syntaxes and add a syntax similar to the SET I'm sending
here to keep
current properties (works well, but the syntax seems ugly to me, better
ideas?)
Best regards,
--
Matheus de Oliveira
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index cb9b60415d..e22f4f924e 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -55,7 +55,9 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
ADD <replaceable class="parameter">table_constraint</replaceable> [ NOT VALID ]
ADD <replaceable class="parameter">table_constraint_using_index</replaceable>
- ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
+ ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable>
+ [ ON DELETE <replaceable class="parameter">action</replaceable> ] [ ON UPDATE <replaceable class="parameter">action</replaceable> ]
+ [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
VALIDATE CONSTRAINT <replaceable class="parameter">constraint_name</replaceable>
DROP CONSTRAINT [ IF EXISTS ] <replaceable class="parameter">constraint_name</replaceable> [ RESTRICT | CASCADE ]
DISABLE TRIGGER [ <replaceable class="parameter">trigger_name</replaceable> | ALL | USER ]
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 05593f3316..689bd8add5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9007,8 +9007,43 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
errmsg("constraint \"%s\" of relation \"%s\" is not a foreign key constraint",
cmdcon->conname, RelationGetRelationName(rel))));
+ /*
+ * Verify for FKCONSTR_ACTION_UNKNOWN, if found, replace by current
+ * action. We could handle FKCONSTR_ACTION_UNKNOWN bellow, but since we
+ * already have to handle the case of changing to the same action, seems
+ * simpler to simple replace FKCONSTR_ACTION_UNKNOWN by the current action
+ * here.
+ */
+ if (cmdcon->fk_del_action == FKCONSTR_ACTION_UNKNOWN)
+ cmdcon->fk_del_action = currcon->confdeltype;
+
+ if (cmdcon->fk_upd_action == FKCONSTR_ACTION_UNKNOWN)
+ cmdcon->fk_upd_action = currcon->confupdtype;
+
+ /*
+ * Do the same for deferrable attributes. But consider that if changed
+ * only initdeferred attribute and to true, force deferrable to be also
+ * true. On the other hand, if changed only deferrable attribute and to
+ * false, force initdeferred to be also false.
+ */
+ if (!cmdcon->was_deferrable_set)
+ cmdcon->deferrable = cmdcon->initdeferred ? true : currcon->condeferrable;
+
+ if (!cmdcon->was_initdeferred_set)
+ cmdcon->initdeferred = !cmdcon->deferrable ? false : currcon->condeferred;
+
+ /*
+ * This is a safe check only, should never happen here.
+ */
+ if (cmdcon->initdeferred && !cmdcon->deferrable)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("constraint declared INITIALLY DEFERRED must be DEFERRABLE")));
+
if (currcon->condeferrable != cmdcon->deferrable ||
- currcon->condeferred != cmdcon->initdeferred)
+ currcon->condeferred != cmdcon->initdeferred ||
+ currcon->confdeltype != cmdcon->fk_del_action ||
+ currcon->confupdtype != cmdcon->fk_upd_action)
{
HeapTuple copyTuple;
HeapTuple tgtuple;
@@ -9026,6 +9061,8 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
copy_con->condeferrable = cmdcon->deferrable;
copy_con->condeferred = cmdcon->initdeferred;
+ copy_con->confdeltype = cmdcon->fk_del_action;
+ copy_con->confupdtype = cmdcon->fk_upd_action;
CatalogTupleUpdate(conrel, ©Tuple->t_self, copyTuple);
InvokeObjectPostAlterHook(ConstraintRelationId,
@@ -9062,23 +9099,106 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
otherrelids = list_append_unique_oid(otherrelids,
tgform->tgrelid);
- /*
- * Update deferrability of RI_FKey_noaction_del,
- * RI_FKey_noaction_upd, RI_FKey_check_ins and RI_FKey_check_upd
- * triggers, but not others; see createForeignKeyActionTriggers
- * and CreateFKCheckTrigger.
- */
- if (tgform->tgfoid != F_RI_FKEY_NOACTION_DEL &&
- tgform->tgfoid != F_RI_FKEY_NOACTION_UPD &&
- tgform->tgfoid != F_RI_FKEY_CHECK_INS &&
- tgform->tgfoid != F_RI_FKEY_CHECK_UPD)
- continue;
-
copyTuple = heap_copytuple(tgtuple);
copy_tg = (Form_pg_trigger) GETSTRUCT(copyTuple);
+ /*
+ * Set deferrability here, but note that it may be overridden
+ * bellow if the pg_trigger entry is on the referencing table and
+ * depending on the action used for ON UPDATE/DELETE. But for
+ * check triggers (in the referenced table) it is kept as is
+ * (since ON UPDATE/DELETE actions makes no difference for the
+ * check triggers).
+ */
copy_tg->tgdeferrable = cmdcon->deferrable;
copy_tg->tginitdeferred = cmdcon->initdeferred;
+
+ /*
+ * Set ON DELETE action
+ */
+ if (tgform->tgfoid == F_RI_FKEY_NOACTION_DEL ||
+ tgform->tgfoid == F_RI_FKEY_RESTRICT_DEL ||
+ tgform->tgfoid == F_RI_FKEY_CASCADE_DEL ||
+ tgform->tgfoid == F_RI_FKEY_SETNULL_DEL ||
+ tgform->tgfoid == F_RI_FKEY_SETDEFAULT_DEL)
+ {
+ switch (cmdcon->fk_del_action)
+ {
+ case FKCONSTR_ACTION_NOACTION:
+ copy_tg->tgdeferrable = cmdcon->deferrable;
+ copy_tg->tginitdeferred = cmdcon->initdeferred;
+ copy_tg->tgfoid = F_RI_FKEY_NOACTION_DEL;
+ break;
+ case FKCONSTR_ACTION_RESTRICT:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_RESTRICT_DEL;
+ break;
+ case FKCONSTR_ACTION_CASCADE:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_CASCADE_DEL;
+ break;
+ case FKCONSTR_ACTION_SETNULL:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_SETNULL_DEL;
+ break;
+ case FKCONSTR_ACTION_SETDEFAULT:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_SETDEFAULT_DEL;
+ break;
+ default:
+ elog(ERROR, "unrecognized FK action type: %d",
+ (int) cmdcon->fk_del_action);
+ break;
+ }
+ }
+
+ /*
+ * Set ON UPDATE action
+ */
+ if (tgform->tgfoid == F_RI_FKEY_NOACTION_UPD ||
+ tgform->tgfoid == F_RI_FKEY_RESTRICT_UPD ||
+ tgform->tgfoid == F_RI_FKEY_CASCADE_UPD ||
+ tgform->tgfoid == F_RI_FKEY_SETNULL_UPD ||
+ tgform->tgfoid == F_RI_FKEY_SETDEFAULT_UPD)
+ {
+ switch (cmdcon->fk_upd_action)
+ {
+ case FKCONSTR_ACTION_NOACTION:
+ copy_tg->tgdeferrable = cmdcon->deferrable;
+ copy_tg->tginitdeferred = cmdcon->initdeferred;
+ copy_tg->tgfoid = F_RI_FKEY_NOACTION_UPD;
+ break;
+ case FKCONSTR_ACTION_RESTRICT:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_RESTRICT_UPD;
+ break;
+ case FKCONSTR_ACTION_CASCADE:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_CASCADE_UPD;
+ break;
+ case FKCONSTR_ACTION_SETNULL:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_SETNULL_UPD;
+ break;
+ case FKCONSTR_ACTION_SETDEFAULT:
+ copy_tg->tgdeferrable = false;
+ copy_tg->tginitdeferred = false;
+ copy_tg->tgfoid = F_RI_FKEY_SETDEFAULT_UPD;
+ break;
+ default:
+ elog(ERROR, "unrecognized FK action type: %d",
+ (int) cmdcon->fk_upd_action);
+ break;
+ }
+ }
+
CatalogTupleUpdate(tgrel, ©Tuple->t_self, copyTuple);
InvokeObjectPostAlterHook(TriggerRelationId, currcon->oid, 0);
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 3432bb921d..58241c1214 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2910,6 +2910,8 @@ _copyConstraint(const Constraint *from)
COPY_SCALAR_FIELD(deferrable);
COPY_SCALAR_FIELD(initdeferred);
COPY_LOCATION_FIELD(location);
+ COPY_SCALAR_FIELD(was_deferrable_set);
+ COPY_SCALAR_FIELD(was_initdeferred_set);
COPY_SCALAR_FIELD(is_no_inherit);
COPY_NODE_FIELD(raw_expr);
COPY_STRING_FIELD(cooked_expr);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 18cb014373..584e2e32a0 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2594,6 +2594,8 @@ _equalConstraint(const Constraint *a, const Constraint *b)
COMPARE_SCALAR_FIELD(deferrable);
COMPARE_SCALAR_FIELD(initdeferred);
COMPARE_LOCATION_FIELD(location);
+ COMPARE_SCALAR_FIELD(was_deferrable_set);
+ COMPARE_SCALAR_FIELD(was_initdeferred_set);
COMPARE_SCALAR_FIELD(is_no_inherit);
COMPARE_NODE_FIELD(raw_expr);
COMPARE_STRING_FIELD(cooked_expr);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index b0dcd02ff6..dc45f154e3 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -3459,6 +3459,8 @@ _outConstraint(StringInfo str, const Constraint *node)
WRITE_BOOL_FIELD(deferrable);
WRITE_BOOL_FIELD(initdeferred);
WRITE_LOCATION_FIELD(location);
+ WRITE_BOOL_FIELD(was_deferrable_set);
+ WRITE_BOOL_FIELD(was_initdeferred_set);
appendStringInfoString(str, " :contype ");
switch (node->contype)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3f67aaf30e..3cd5579515 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -185,7 +185,8 @@ static void SplitColQualList(List *qualList,
List **constraintList, CollateClause **collClause,
core_yyscan_t yyscanner);
static void processCASbits(int cas_bits, int location, const char *constrType,
- bool *deferrable, bool *initdeferred, bool *not_valid,
+ bool *deferrable, bool *was_deferrable_set,
+ bool *initdeferred, bool *was_initdeferred_set, bool *not_valid,
bool *no_inherit, core_yyscan_t yyscanner);
static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
@@ -543,7 +544,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <ival> TableLikeOptionList TableLikeOption
%type <list> ColQualList
%type <node> ColConstraint ColConstraintElem ConstraintAttr
-%type <ival> key_actions key_delete key_match key_update key_action
+%type <ival> key_actions opt_key_actions
+%type <ival> key_delete key_match key_update key_action
%type <ival> ConstraintAttributeSpec ConstraintAttributeElem
%type <str> ExistingIndex
@@ -2275,8 +2277,25 @@ alter_table_cmd:
n->def = $2;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> ALTER CONSTRAINT ... SET */
+ | ALTER CONSTRAINT name SET opt_key_actions ConstraintAttributeSpec
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ Constraint *c = makeNode(Constraint);
+ n->subtype = AT_AlterConstraint;
+ n->def = (Node *) c;
+ c->contype = CONSTR_FOREIGN; /* others not supported, yet */
+ c->conname = $3;
+ c->fk_upd_action = (char) ($5 >> 8);
+ c->fk_del_action = (char) ($5 & 0xFF);
+ processCASbits($6, @5, "ALTER CONSTRAINT statement",
+ &c->deferrable, &c->was_deferrable_set,
+ &c->initdeferred, &c->was_initdeferred_set,
+ NULL, NULL, yyscanner);
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> ALTER CONSTRAINT ... */
- | ALTER CONSTRAINT name ConstraintAttributeSpec
+ | ALTER CONSTRAINT name key_actions ConstraintAttributeSpec
{
AlterTableCmd *n = makeNode(AlterTableCmd);
Constraint *c = makeNode(Constraint);
@@ -2284,9 +2303,14 @@ alter_table_cmd:
n->def = (Node *) c;
c->contype = CONSTR_FOREIGN; /* others not supported, yet */
c->conname = $3;
- processCASbits($4, @4, "ALTER CONSTRAINT statement",
- &c->deferrable,
- &c->initdeferred,
+ c->fk_upd_action = (char) ($4 >> 8);
+ c->fk_del_action = (char) ($4 & 0xFF);
+ /* Without SET, always change deferrability */
+ c->was_deferrable_set = true;
+ c->was_initdeferred_set = true;
+ processCASbits($5, @4, "ALTER CONSTRAINT statement",
+ &c->deferrable, NULL,
+ &c->initdeferred, NULL,
NULL, NULL, yyscanner);
$$ = (Node *)n;
}
@@ -3643,7 +3667,7 @@ ConstraintElem:
n->raw_expr = $3;
n->cooked_expr = NULL;
processCASbits($5, @5, "CHECK",
- NULL, NULL, &n->skip_validation,
+ NULL, NULL, NULL, NULL, &n->skip_validation,
&n->is_no_inherit, yyscanner);
n->initially_valid = !n->skip_validation;
$$ = (Node *)n;
@@ -3660,8 +3684,8 @@ ConstraintElem:
n->indexname = NULL;
n->indexspace = $7;
processCASbits($8, @8, "UNIQUE",
- &n->deferrable, &n->initdeferred, NULL,
- NULL, yyscanner);
+ &n->deferrable, NULL, &n->initdeferred, NULL,
+ NULL, NULL, yyscanner);
$$ = (Node *)n;
}
| UNIQUE ExistingIndex ConstraintAttributeSpec
@@ -3675,8 +3699,8 @@ ConstraintElem:
n->indexname = $2;
n->indexspace = NULL;
processCASbits($3, @3, "UNIQUE",
- &n->deferrable, &n->initdeferred, NULL,
- NULL, yyscanner);
+ &n->deferrable, NULL, &n->initdeferred, NULL,
+ NULL, NULL, yyscanner);
$$ = (Node *)n;
}
| PRIMARY KEY '(' columnList ')' opt_c_include opt_definition OptConsTableSpace
@@ -3691,8 +3715,8 @@ ConstraintElem:
n->indexname = NULL;
n->indexspace = $8;
processCASbits($9, @9, "PRIMARY KEY",
- &n->deferrable, &n->initdeferred, NULL,
- NULL, yyscanner);
+ &n->deferrable, NULL, &n->initdeferred, NULL,
+ NULL, NULL, yyscanner);
$$ = (Node *)n;
}
| PRIMARY KEY ExistingIndex ConstraintAttributeSpec
@@ -3706,8 +3730,8 @@ ConstraintElem:
n->indexname = $3;
n->indexspace = NULL;
processCASbits($4, @4, "PRIMARY KEY",
- &n->deferrable, &n->initdeferred, NULL,
- NULL, yyscanner);
+ &n->deferrable, NULL, &n->initdeferred, NULL,
+ NULL, NULL, yyscanner);
$$ = (Node *)n;
}
| EXCLUDE access_method_clause '(' ExclusionConstraintList ')'
@@ -3725,8 +3749,8 @@ ConstraintElem:
n->indexspace = $8;
n->where_clause = $9;
processCASbits($10, @10, "EXCLUDE",
- &n->deferrable, &n->initdeferred, NULL,
- NULL, yyscanner);
+ &n->deferrable, NULL, &n->initdeferred, NULL,
+ NULL, NULL, yyscanner);
$$ = (Node *)n;
}
| FOREIGN KEY '(' columnList ')' REFERENCES qualified_name
@@ -3742,7 +3766,8 @@ ConstraintElem:
n->fk_upd_action = (char) ($10 >> 8);
n->fk_del_action = (char) ($10 & 0xFF);
processCASbits($11, @11, "FOREIGN KEY",
- &n->deferrable, &n->initdeferred,
+ &n->deferrable, NULL,
+ &n->initdeferred, NULL,
&n->skip_validation, NULL,
yyscanner);
n->initially_valid = !n->skip_validation;
@@ -3822,7 +3847,7 @@ ExclusionWhereClause:
* We combine the update and delete actions into one value temporarily
* for simplicity of parsing, and then break them down again in the
* calling production. update is in the left 8 bits, delete in the right.
- * Note that NOACTION is the default.
+ * Note that NOACTION is the default. See also opt_key_actions.
*/
key_actions:
key_update
@@ -3837,6 +3862,23 @@ key_actions:
{ $$ = (FKCONSTR_ACTION_NOACTION << 8) | (FKCONSTR_ACTION_NOACTION & 0xFF); }
;
+/*
+ * Basically the same as key_actions, but using FKCONSTR_ACTION_UNKNOWN
+ * as the default one instead of NOACTION.
+ */
+opt_key_actions:
+ key_update
+ { $$ = ($1 << 8) | (FKCONSTR_ACTION_UNKNOWN & 0xFF); }
+ | key_delete
+ { $$ = (FKCONSTR_ACTION_UNKNOWN << 8) | ($1 & 0xFF); }
+ | key_update key_delete
+ { $$ = ($1 << 8) | ($2 & 0xFF); }
+ | key_delete key_update
+ { $$ = ($2 << 8) | ($1 & 0xFF); }
+ | /*EMPTY*/
+ { $$ = (FKCONSTR_ACTION_UNKNOWN << 8) | (FKCONSTR_ACTION_UNKNOWN & 0xFF); }
+ ;
+
key_update: ON UPDATE key_action { $$ = $3; }
;
@@ -5390,8 +5432,8 @@ CreateTrigStmt:
n->transitionRels = NIL;
n->isconstraint = true;
processCASbits($10, @10, "TRIGGER",
- &n->deferrable, &n->initdeferred, NULL,
- NULL, yyscanner);
+ &n->deferrable, NULL, &n->initdeferred, NULL,
+ NULL, NULL, yyscanner);
n->constrrel = $9;
$$ = (Node *)n;
}
@@ -16247,7 +16289,8 @@ SplitColQualList(List *qualList,
*/
static void
processCASbits(int cas_bits, int location, const char *constrType,
- bool *deferrable, bool *initdeferred, bool *not_valid,
+ bool *deferrable, bool *was_deferrable_set,
+ bool *initdeferred, bool *was_initdeferred_set, bool *not_valid,
bool *no_inherit, core_yyscan_t yyscanner)
{
/* defaults */
@@ -16258,6 +16301,14 @@ processCASbits(int cas_bits, int location, const char *constrType,
if (not_valid)
*not_valid = false;
+ if (was_deferrable_set)
+ *was_deferrable_set = cas_bits & (CAS_DEFERRABLE
+ | CAS_NOT_DEFERRABLE) ? true : false;
+
+ if (was_initdeferred_set)
+ *was_initdeferred_set = cas_bits & (CAS_INITIALLY_DEFERRED
+ | CAS_INITIALLY_IMMEDIATE) ? true : false;
+
if (cas_bits & (CAS_DEFERRABLE | CAS_INITIALLY_DEFERRED))
{
if (deferrable)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index d93a79a554..141dd43df7 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2117,6 +2117,8 @@ typedef enum ConstrType /* types of constraints */
#define FKCONSTR_ACTION_CASCADE 'c'
#define FKCONSTR_ACTION_SETNULL 'n'
#define FKCONSTR_ACTION_SETDEFAULT 'd'
+#define FKCONSTR_ACTION_UNKNOWN 'u' /* unknown is used only for ALTER
+ * CONSTRAINT */
/* Foreign key matchtype codes */
#define FKCONSTR_MATCH_FULL 'f'
@@ -2134,6 +2136,10 @@ typedef struct Constraint
bool initdeferred; /* INITIALLY DEFERRED? */
int location; /* token location, or -1 if unknown */
+ /* Fields used by ALTER CONSTRAINT to verify if a change was actually made */
+ bool was_deferrable_set; /* Was DEFERRABLE informed? */
+ bool was_initdeferred_set; /* Was INITIALLY DEFERRED informed? */
+
/* Fields used for constraints with expressions (CHECK and DEFAULT): */
bool is_no_inherit; /* is constraint non-inheritable? */
Node *raw_expr; /* expr, as untransformed parse tree */
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 23d4265555..8146ad9eb5 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -704,15 +704,15 @@ ORDER BY 1,2,3;
---------+------------------------+--------+--------------+----------------
fkdd | "RI_FKey_cascade_del" | 9 | f | f
fkdd | "RI_FKey_noaction_upd" | 17 | t | t
- fkdd2 | "RI_FKey_cascade_del" | 9 | f | f
+ fkdd2 | "RI_FKey_noaction_del" | 9 | t | t
fkdd2 | "RI_FKey_noaction_upd" | 17 | t | t
fkdi | "RI_FKey_cascade_del" | 9 | f | f
fkdi | "RI_FKey_noaction_upd" | 17 | t | f
- fkdi2 | "RI_FKey_cascade_del" | 9 | f | f
+ fkdi2 | "RI_FKey_noaction_del" | 9 | t | f
fkdi2 | "RI_FKey_noaction_upd" | 17 | t | f
fknd | "RI_FKey_cascade_del" | 9 | f | f
fknd | "RI_FKey_noaction_upd" | 17 | f | f
- fknd2 | "RI_FKey_cascade_del" | 9 | f | f
+ fknd2 | "RI_FKey_noaction_del" | 9 | f | f
fknd2 | "RI_FKey_noaction_upd" | 17 | f | f
(12 rows)
@@ -736,6 +736,28 @@ ORDER BY 1,2,3;
fknd2 | "RI_FKey_check_upd" | 17 | f | f
(12 rows)
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 DEFERRABLE INITIALLY DEFERRED;
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2;
+SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass AND conname = 'fkdi2';
+ pg_get_constraintdef
+-------------------------------------------------
+ FOREIGN KEY (ftest1) REFERENCES pktable(ptest1)
+(1 row)
+
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 INITIALLY IMMEDIATE;
+SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass AND conname = 'fkdi2';
+ pg_get_constraintdef
+-------------------------------------------------
+ FOREIGN KEY (ftest1) REFERENCES pktable(ptest1)
+(1 row)
+
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 NOT DEFERRABLE;
+SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass AND conname = 'fkdi2';
+ pg_get_constraintdef
+-------------------------------------------------
+ FOREIGN KEY (ftest1) REFERENCES pktable(ptest1)
+(1 row)
+
-- temp tables should go away by themselves, need not drop them.
-- test check constraint adding
create table atacc1 ( test int );
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 894084f94f..5792b032b1 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2394,3 +2394,128 @@ DROP SCHEMA fkpart8 CASCADE;
NOTICE: drop cascades to 2 other objects
DETAIL: drop cascades to table fkpart8.tbl1
drop cascades to table fkpart8.tbl2
+\set VERBOSITY default
+-- ALTER CONSTRAINT changing ON UPDATE/DELETE.
+-- Try all combinations and validate the diff with a created constraint
+CREATE SCHEMA createtest; -- created constraints with target action, validation
+CREATE SCHEMA altertest; -- created with source and altered to target, test
+DO
+$test_alter_con$
+DECLARE
+ v_result json;
+ method text;
+ from_action text;
+ to_action text;
+BEGIN
+ FOR method, from_action, to_action IN
+ WITH act(action) AS (
+ SELECT unnest('{NO ACTION,RESTRICT,CASCADE,SET DEFAULT,SET NULL}'::text[])
+ )
+ SELECT
+ m.method, a1.action, a2.action
+ FROM unnest('{UPDATE,DELETE}'::text[]) AS m(method), act a1, act a2
+ LOOP
+ EXECUTE format(
+ $sql$
+ -- Alter from ON %1$s %2$s to ON %1$s %3$s
+ CREATE TABLE createtest.foo(id integer primary key);
+ CREATE TABLE createtest.bar(foo_id integer DEFAULT 0 REFERENCES createtest.foo ON %1$s %3$s, val text);
+
+ CREATE TABLE altertest.foo(id integer primary key);
+ INSERT INTO altertest.foo VALUES(0),(1),(2),(3);
+
+ CREATE TABLE altertest.bar(foo_id integer DEFAULT 0 REFERENCES altertest.foo ON %1$s %2$s, val text);
+
+ ALTER TABLE altertest.bar ALTER CONSTRAINT bar_foo_id_fkey SET ON %1$s %3$s;
+
+ $sql$, method, from_action, to_action);
+
+ SELECT json_agg(t)
+ INTO v_result
+ FROM (
+ -- Do EXCEPT of the "altertest" and "createtest" constraints, if they are equal (as expected), it should return empty
+ SELECT
+ rel.relname, replace(tg.tgname, tg.oid::text, 'OID') AS tgname,
+ tg.tgfoid::regproc, con.conname, con.confupdtype, con.confdeltype, tg.tgdeferrable,
+ regexp_replace(pg_get_constraintdef(con.oid), '(createtest\.|altertest\.)', '') AS condef
+ FROM pg_trigger tg
+ JOIN pg_constraint con ON con.oid = tg.tgconstraint
+ JOIN pg_class rel ON tg.tgrelid = rel.oid
+ WHERE tg.tgrelid IN ('altertest.foo'::regclass, 'altertest.bar'::regclass)
+ EXCEPT
+ SELECT
+ rel.relname, replace(tg.tgname, tg.oid::text, 'OID') AS tgname,
+ tg.tgfoid::regproc, con.conname, con.confupdtype, con.confdeltype, tg.tgdeferrable,
+ regexp_replace(pg_get_constraintdef(con.oid), '(createtest\.|altertest\.)', '') AS condef
+ FROM pg_trigger tg
+ JOIN pg_constraint con ON con.oid = tg.tgconstraint
+ JOIN pg_class rel ON tg.tgrelid = rel.oid
+ WHERE tg.tgrelid IN ('createtest.foo'::regclass, 'createtest.bar'::regclass)
+ ) t;
+
+ DROP TABLE createtest.bar;
+ DROP TABLE createtest.foo;
+ DROP TABLE altertest.bar;
+ DROP TABLE altertest.foo;
+
+ IF (v_result IS NULL) THEN
+ RAISE INFO 'ON % from % to %: OK.', method, from_action, to_action;
+ ELSE
+ RAISE EXCEPTION 'ON % from % to %. FAILED! Unmatching rows: %', method, from_action, to_action, v_result;
+ END IF;
+ END LOOP;
+END;
+$test_alter_con$
+;
+INFO: ON UPDATE from NO ACTION to NO ACTION: OK.
+INFO: ON UPDATE from RESTRICT to NO ACTION: OK.
+INFO: ON UPDATE from CASCADE to NO ACTION: OK.
+INFO: ON UPDATE from SET DEFAULT to NO ACTION: OK.
+INFO: ON UPDATE from SET NULL to NO ACTION: OK.
+INFO: ON DELETE from NO ACTION to NO ACTION: OK.
+INFO: ON DELETE from RESTRICT to NO ACTION: OK.
+INFO: ON DELETE from CASCADE to NO ACTION: OK.
+INFO: ON DELETE from SET DEFAULT to NO ACTION: OK.
+INFO: ON DELETE from SET NULL to NO ACTION: OK.
+INFO: ON UPDATE from NO ACTION to RESTRICT: OK.
+INFO: ON UPDATE from RESTRICT to RESTRICT: OK.
+INFO: ON UPDATE from CASCADE to RESTRICT: OK.
+INFO: ON UPDATE from SET DEFAULT to RESTRICT: OK.
+INFO: ON UPDATE from SET NULL to RESTRICT: OK.
+INFO: ON DELETE from NO ACTION to RESTRICT: OK.
+INFO: ON DELETE from RESTRICT to RESTRICT: OK.
+INFO: ON DELETE from CASCADE to RESTRICT: OK.
+INFO: ON DELETE from SET DEFAULT to RESTRICT: OK.
+INFO: ON DELETE from SET NULL to RESTRICT: OK.
+INFO: ON UPDATE from NO ACTION to CASCADE: OK.
+INFO: ON UPDATE from RESTRICT to CASCADE: OK.
+INFO: ON UPDATE from CASCADE to CASCADE: OK.
+INFO: ON UPDATE from SET DEFAULT to CASCADE: OK.
+INFO: ON UPDATE from SET NULL to CASCADE: OK.
+INFO: ON DELETE from NO ACTION to CASCADE: OK.
+INFO: ON DELETE from RESTRICT to CASCADE: OK.
+INFO: ON DELETE from CASCADE to CASCADE: OK.
+INFO: ON DELETE from SET DEFAULT to CASCADE: OK.
+INFO: ON DELETE from SET NULL to CASCADE: OK.
+INFO: ON UPDATE from NO ACTION to SET DEFAULT: OK.
+INFO: ON UPDATE from RESTRICT to SET DEFAULT: OK.
+INFO: ON UPDATE from CASCADE to SET DEFAULT: OK.
+INFO: ON UPDATE from SET DEFAULT to SET DEFAULT: OK.
+INFO: ON UPDATE from SET NULL to SET DEFAULT: OK.
+INFO: ON DELETE from NO ACTION to SET DEFAULT: OK.
+INFO: ON DELETE from RESTRICT to SET DEFAULT: OK.
+INFO: ON DELETE from CASCADE to SET DEFAULT: OK.
+INFO: ON DELETE from SET DEFAULT to SET DEFAULT: OK.
+INFO: ON DELETE from SET NULL to SET DEFAULT: OK.
+INFO: ON UPDATE from NO ACTION to SET NULL: OK.
+INFO: ON UPDATE from RESTRICT to SET NULL: OK.
+INFO: ON UPDATE from CASCADE to SET NULL: OK.
+INFO: ON UPDATE from SET DEFAULT to SET NULL: OK.
+INFO: ON UPDATE from SET NULL to SET NULL: OK.
+INFO: ON DELETE from NO ACTION to SET NULL: OK.
+INFO: ON DELETE from RESTRICT to SET NULL: OK.
+INFO: ON DELETE from CASCADE to SET NULL: OK.
+INFO: ON DELETE from SET DEFAULT to SET NULL: OK.
+INFO: ON DELETE from SET NULL to SET NULL: OK.
+DROP SCHEMA createtest;
+DROP SCHEMA altertest;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 99af0b851b..5223142b8f 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -528,6 +528,16 @@ FROM pg_trigger JOIN pg_constraint con ON con.oid = tgconstraint
WHERE tgrelid = 'fktable'::regclass
ORDER BY 1,2,3;
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 DEFERRABLE INITIALLY DEFERRED;
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2;
+SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass AND conname = 'fkdi2';
+
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 INITIALLY IMMEDIATE;
+SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass AND conname = 'fkdi2';
+
+ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 NOT DEFERRABLE;
+SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass AND conname = 'fkdi2';
+
-- temp tables should go away by themselves, need not drop them.
-- test check constraint adding
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index b67bef01df..18cfa1ab16 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1679,3 +1679,81 @@ INSERT INTO fkpart8.tbl2 VALUES(1);
ALTER TABLE fkpart8.tbl2 DROP CONSTRAINT tbl2_f1_fkey;
COMMIT;
DROP SCHEMA fkpart8 CASCADE;
+\set VERBOSITY default
+
+-- ALTER CONSTRAINT changing ON UPDATE/DELETE.
+-- Try all combinations and validate the diff with a created constraint
+CREATE SCHEMA createtest; -- created constraints with target action, validation
+CREATE SCHEMA altertest; -- created with source and altered to target, test
+
+DO
+$test_alter_con$
+DECLARE
+ v_result json;
+ method text;
+ from_action text;
+ to_action text;
+BEGIN
+ FOR method, from_action, to_action IN
+ WITH act(action) AS (
+ SELECT unnest('{NO ACTION,RESTRICT,CASCADE,SET DEFAULT,SET NULL}'::text[])
+ )
+ SELECT
+ m.method, a1.action, a2.action
+ FROM unnest('{UPDATE,DELETE}'::text[]) AS m(method), act a1, act a2
+ LOOP
+ EXECUTE format(
+ $sql$
+ -- Alter from ON %1$s %2$s to ON %1$s %3$s
+ CREATE TABLE createtest.foo(id integer primary key);
+ CREATE TABLE createtest.bar(foo_id integer DEFAULT 0 REFERENCES createtest.foo ON %1$s %3$s, val text);
+
+ CREATE TABLE altertest.foo(id integer primary key);
+ INSERT INTO altertest.foo VALUES(0),(1),(2),(3);
+
+ CREATE TABLE altertest.bar(foo_id integer DEFAULT 0 REFERENCES altertest.foo ON %1$s %2$s, val text);
+
+ ALTER TABLE altertest.bar ALTER CONSTRAINT bar_foo_id_fkey SET ON %1$s %3$s;
+
+ $sql$, method, from_action, to_action);
+
+ SELECT json_agg(t)
+ INTO v_result
+ FROM (
+ -- Do EXCEPT of the "altertest" and "createtest" constraints, if they are equal (as expected), it should return empty
+ SELECT
+ rel.relname, replace(tg.tgname, tg.oid::text, 'OID') AS tgname,
+ tg.tgfoid::regproc, con.conname, con.confupdtype, con.confdeltype, tg.tgdeferrable,
+ regexp_replace(pg_get_constraintdef(con.oid), '(createtest\.|altertest\.)', '') AS condef
+ FROM pg_trigger tg
+ JOIN pg_constraint con ON con.oid = tg.tgconstraint
+ JOIN pg_class rel ON tg.tgrelid = rel.oid
+ WHERE tg.tgrelid IN ('altertest.foo'::regclass, 'altertest.bar'::regclass)
+ EXCEPT
+ SELECT
+ rel.relname, replace(tg.tgname, tg.oid::text, 'OID') AS tgname,
+ tg.tgfoid::regproc, con.conname, con.confupdtype, con.confdeltype, tg.tgdeferrable,
+ regexp_replace(pg_get_constraintdef(con.oid), '(createtest\.|altertest\.)', '') AS condef
+ FROM pg_trigger tg
+ JOIN pg_constraint con ON con.oid = tg.tgconstraint
+ JOIN pg_class rel ON tg.tgrelid = rel.oid
+ WHERE tg.tgrelid IN ('createtest.foo'::regclass, 'createtest.bar'::regclass)
+ ) t;
+
+ DROP TABLE createtest.bar;
+ DROP TABLE createtest.foo;
+ DROP TABLE altertest.bar;
+ DROP TABLE altertest.foo;
+
+ IF (v_result IS NULL) THEN
+ RAISE INFO 'ON % from % to %: OK.', method, from_action, to_action;
+ ELSE
+ RAISE EXCEPTION 'ON % from % to %. FAILED! Unmatching rows: %', method, from_action, to_action, v_result;
+ END IF;
+ END LOOP;
+END;
+$test_alter_con$
+;
+
+DROP SCHEMA createtest;
+DROP SCHEMA altertest;
