I have reviewed your patch
referential-actions-on-delete-only-set-cols-v3.patch. Attached are two
patches that go on top of yours that contain my recommended changes.
Basically, this all looks pretty good to me. My changes are mostly
stylistic.
Some notes of substance:
- The omission of the referential actions details from the CREATE TABLE
reference page surprised me. I have committed that separately (without
the column support, of course). So you should rebase your patch on top
of that. Note that ALTER TABLE would now also need to be updated by
your patch.
- I recommend setting pg_constraint.confdelsetcols to null for the
default behavior of setting all columns, instead of an empty array the
way you have done it. I have noted the places in the code that need to
be changed for that.
- The outfuncs.c support shouldn't be included in the final patch.
There is nothing wrong it, but I don't think it should be part of this
patch to add piecemeal support like that. I have included a few changes
there anyway for completeness.
- In gram.y, I moved the error check around to avoid duplication.
- In ri_triggers.c, I follow your renaming of the constants, but
RI_PLAN_ONTRIGGER_RESTRICT seems a little weird. Maybe do _ONBOTH_, or
else reverse the order, like RI_PLAN_SETNULL_ONDELETE, which would then
allow RI_PLAN_RESTRICT.
Please look through this and provide an updated patch, and then it
should be good to go.From cacfc7737bc62e28b8ada7bd6230034050e76946 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 11 Nov 2021 08:06:27 +0100
Subject: [PATCH 1/2] Fix whitespace
---
src/backend/commands/tablecmds.c | 4 +-
src/backend/parser/gram.y | 86 ++++++++++++++++----------------
2 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 38b49694b3..3ef5d6d285 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9403,8 +9403,8 @@ void validateFkOnDeleteSetColumns(
if (!seen) {
char *col = strVal(list_nth(fksetcols, i));
ereport(ERROR,
- (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
- errmsg("column \"%s\" referenced in ON DELETE
SET action must be part of foreign key", col)));
+
(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+ errmsg("column \"%s\" referenced in ON
DELETE SET action must be part of foreign key", col)));
}
}
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b39c15cfeb..7ee0414e0b 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4011,39 +4011,39 @@ OptWhereClause:
key_actions:
key_update
- {
+ {
KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
- n->updateAction = $1;
- n->deleteAction = (KeyAction *) palloc(sizeof(KeyAction));
- n->deleteAction->action = FKCONSTR_ACTION_NOACTION;
- n->deleteAction->cols = NIL;
+ n->updateAction = $1;
+ n->deleteAction = (KeyAction *)
palloc(sizeof(KeyAction));
+ n->deleteAction->action =
FKCONSTR_ACTION_NOACTION;
+ n->deleteAction->cols = NIL;
$$ = n;
- }
+ }
| key_delete
- {
+ {
KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
- n->updateAction = (KeyAction *) palloc(sizeof(KeyAction));
- n->updateAction->action = FKCONSTR_ACTION_NOACTION;
- n->updateAction->cols = NIL;
- n->deleteAction = $1;
+ n->updateAction = (KeyAction *)
palloc(sizeof(KeyAction));
+ n->updateAction->action =
FKCONSTR_ACTION_NOACTION;
+ n->updateAction->cols = NIL;
+ n->deleteAction = $1;
$$ = n;
- }
+ }
| key_update key_delete
- {
+ {
KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
- n->updateAction = $1;
- n->deleteAction = $2;
+ n->updateAction = $1;
+ n->deleteAction = $2;
$$ = n;
- }
+ }
| key_delete key_update
- {
+ {
KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
- n->updateAction = $2;
- n->deleteAction = $1;
+ n->updateAction = $2;
+ n->deleteAction = $1;
$$ = n;
- }
- | /*EMPTY*/
- {
+ }
+ | /*EMPTY*/
+ {
KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
n->updateAction = (KeyAction *)
palloc(sizeof(KeyAction));
n->updateAction->action =
FKCONSTR_ACTION_NOACTION;
@@ -4052,7 +4052,7 @@ key_actions:
n->deleteAction->action =
FKCONSTR_ACTION_NOACTION;
n->deleteAction->cols = NIL;
$$ = n;
- }
+ }
;
key_update: ON UPDATE key_action { $$ = $3; }
@@ -4065,38 +4065,38 @@ key_action:
NO ACTION
{
KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
- n->action = FKCONSTR_ACTION_NOACTION;
- n->cols = NIL;
- $$ = n;
- }
+ n->action = FKCONSTR_ACTION_NOACTION;
+ n->cols = NIL;
+ $$ = n;
+ }
| RESTRICT
{
KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
- n->action = FKCONSTR_ACTION_RESTRICT;
- n->cols = NIL;
- $$ = n;
- }
+ n->action = FKCONSTR_ACTION_RESTRICT;
+ n->cols = NIL;
+ $$ = n;
+ }
| CASCADE
{
KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
- n->action = FKCONSTR_ACTION_CASCADE;
- n->cols = NIL;
- $$ = n;
- }
+ n->action = FKCONSTR_ACTION_CASCADE;
+ n->cols = NIL;
+ $$ = n;
+ }
| SET NULL_P opt_column_list
{
KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
- n->action = FKCONSTR_ACTION_SETNULL;
- n->cols = $3;
- $$ = n;
- }
+ n->action = FKCONSTR_ACTION_SETNULL;
+ n->cols = $3;
+ $$ = n;
+ }
| SET DEFAULT opt_column_list
{
KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
- n->action = FKCONSTR_ACTION_SETDEFAULT;
- n->cols = $3;
- $$ = n;
- }
+ n->action = FKCONSTR_ACTION_SETDEFAULT;
+ n->cols = $3;
+ $$ = n;
+ }
;
OptInherit: INHERITS '(' qualified_name_list ')' { $$ = $3; }
--
2.33.1
From 98a3fc16f49dea24d34d1af257c5017281bfe27a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 11 Nov 2021 11:07:02 +0100
Subject: [PATCH 2/2] Review changes
---
doc/src/sgml/ref/create_table.sgml | 6 +--
src/backend/catalog/pg_constraint.c | 10 +++--
src/backend/commands/tablecmds.c | 29 +++++++-----
src/backend/nodes/outfuncs.c | 12 +++--
src/backend/parser/gram.y | 54 ++++++++++++-----------
src/backend/utils/adt/ri_triggers.c | 32 ++++++++------
src/backend/utils/adt/ruleutils.c | 13 +++---
src/include/catalog/pg_constraint.h | 2 +-
src/include/nodes/parsenodes.h | 1 -
src/test/regress/expected/foreign_key.out | 2 +-
10 files changed, 90 insertions(+), 71 deletions(-)
diff --git a/doc/src/sgml/ref/create_table.sgml
b/doc/src/sgml/ref/create_table.sgml
index c6a9072ca0..eee837d86e 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -106,7 +106,7 @@
{ <replaceable class="parameter">column_name</replaceable> | ( <replaceable
class="parameter">expression</replaceable> ) } [ <replaceable
class="parameter">opclass</replaceable> ] [ ASC | DESC ] [ NULLS { FIRST | LAST
} ]
-<phrase><replaceable class="parameter">referential_action</replaceable> in an
<literal>EXCLUDE</literal> constraint is:</phrase>
+<phrase><replaceable class="parameter">referential_action</replaceable> in a
<literal>FOREIGN KEY</literal>/<literal>REFERENCES</literal> constraint
is:</phrase>
{ NO ACTION | RESTRICT | CASCADE | SET NULL [ ( <replaceable
class="parameter">column_name</replaceable> [, ... ] ) ] | SET DEFAULT [ (
<replaceable class="parameter">column_name</replaceable> [, ... ] ) ] }
@@ -1175,7 +1175,7 @@ <title>Parameters</title>
<para>
Set all of the referencing columns, or a specified subset of the
referencing columns, to null. A subset of columns can only be
- specified for <literal>ON DELETE</literal> triggers.
+ specified for <literal>ON DELETE</literal> actions.
</para>
</listitem>
</varlistentry>
@@ -1186,7 +1186,7 @@ <title>Parameters</title>
<para>
Set all of the referencing columns, or a specified subset of the
referencing columns, to their default values. A subset of columns
- can only be specified for <literal>ON DELETE</literal> triggers.
+ can only be specified for <literal>ON DELETE</literal> actions.
(There must be a row in the referenced table matching the default
values, if they are not null, or the operation will fail.)
</para>
diff --git a/src/backend/catalog/pg_constraint.c
b/src/backend/catalog/pg_constraint.c
index 00af9930f4..35496984b9 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -139,6 +139,7 @@ CreateConstraintEntry(const char *constraintName,
fkdatums[i] = ObjectIdGetDatum(ffEqOp[i]);
conffeqopArray = construct_array(fkdatums, foreignNKeys,
OIDOID, sizeof(Oid), true, TYPALIGN_INT);
+ // FIXME: use null instead of empty array for standard behavior
for (i = 0; i < numFkDeleteSetCols; i++)
fkdatums[i] = Int16GetDatum(fkDeleteSetCols[i]);
confdelsetcolsArray = construct_array(fkdatums,
numFkDeleteSetCols,
@@ -1170,8 +1171,9 @@ get_primary_key_attnos(Oid relid, bool deferrableOk, Oid
*constraintOid)
/*
* Extract data from the pg_constraint tuple of a foreign-key constraint.
*
- * All arguments save the first are output arguments; fields other than
- * numfks, conkey and confkey can be passed as NULL if caller doesn't need
them.
+ * All arguments save the first are output arguments. All output arguments
+ * other than numfks, conkey and confkey can be passed as NULL if caller
+ * doesn't need them.
*/
void
DeconstructFkConstraintRow(HeapTuple tuple, int *numfks,
@@ -1277,10 +1279,12 @@ DeconstructFkConstraintRow(HeapTuple tuple, int *numfks,
if (fk_del_set_cols)
{
int num_delete_cols = 0;
+
+ // FIXME: use null instead of empty array for standard behavior
adatum = SysCacheGetAttr(CONSTROID, tuple,
Anum_pg_constraint_confdelsetcols, &isNull);
if (isNull)
- elog(ERROR, "null confdelsetcols for foreign-key
constraint %u", constrId);
+ elog(ERROR, "null confdelsetcols for constraint %u",
constrId);
arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
if (ARR_NDIM(arr) != 0)
{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3ef5d6d285..d158fef975 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -484,8 +484,8 @@ static ObjectAddress addFkRecurseReferenced(List **wqueue,
Constraint *fkconstra
Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators,
int numfkdelsetcols, int16 *fkdelsetcols,
bool old_check_ok);
-static void validateFkOnDeleteSetColumns(int numfks, int16 *fkattnums,
- int
numfksetcols, int16 *fksetcolsattnums,
+static void validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
+ int
numfksetcols, const int16 *fksetcolsattnums,
List
*fksetcols);
static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint,
Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr,
@@ -9385,22 +9385,27 @@ ATAddForeignKeyConstraint(List **wqueue,
AlteredTableInfo *tab, Relation rel,
* Verifies that columns used in ON DELETE SET NULL/DEFAULT (...)
* column lists are valid.
*/
-void validateFkOnDeleteSetColumns(
- int numfks, int16 *fkattnums,
- int numfksetcols, int16 *fksetcolsattnums,
- List *fksetcols)
+void
+validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
+ int numfksetcols,
const int16 *fksetcolsattnums,
+ List *fksetcols)
{
- for (int i = 0; i < numfksetcols; i++) {
- int setcol_attnum = fksetcolsattnums[i];
+ for (int i = 0; i < numfksetcols; i++)
+ {
+ int16 setcol_attnum = fksetcolsattnums[i];
bool seen = false;
- for (int j = 0; j < numfks; j++) {
- if (fkattnums[j] == setcol_attnum) {
+
+ for (int j = 0; j < numfks; j++)
+ {
+ if (fkattnums[j] == setcol_attnum)
+ {
seen = true;
break;
}
}
- if (!seen) {
+ if (!seen)
+ {
char *col = strVal(list_nth(fksetcols, i));
ereport(ERROR,
(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
@@ -10871,7 +10876,7 @@ ATExecValidateConstraint(List **wqueue, Relation rel,
char *constrName,
/*
* transformColumnNameList - transform list of column names
*
- * Lookup each name and return its attnum and, if needed, type OID
+ * Lookup each name and return its attnum and, optionally, type OID
*/
static int
transformColumnNameList(Oid relId, List *colList,
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 7225434494..58bebbaa2d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2871,10 +2871,11 @@ _outPLAssignStmt(StringInfo str, const PLAssignStmt
*node)
WRITE_LOCATION_FIELD(location);
}
+#if 0
static void
_outAlterTableStmt(StringInfo str, const AlterTableStmt *node)
{
- WRITE_NODE_TYPE("ALTERTABLE");
+ WRITE_NODE_TYPE("ALTERTABLESTMT");
WRITE_NODE_FIELD(relation);
WRITE_NODE_FIELD(cmds);
@@ -2885,7 +2886,7 @@ _outAlterTableStmt(StringInfo str, const AlterTableStmt
*node)
static void
_outAlterTableCmd(StringInfo str, const AlterTableCmd *node)
{
- WRITE_NODE_TYPE("ALTERTABLE_CMD");
+ WRITE_NODE_TYPE("ALTERTABLECMD");
WRITE_ENUM_FIELD(subtype, AlterTableType);
WRITE_STRING_FIELD(name);
@@ -2899,12 +2900,13 @@ _outAlterTableCmd(StringInfo str, const AlterTableCmd
*node)
static void
_outRoleSpec(StringInfo str, const RoleSpec *node)
{
- WRITE_NODE_TYPE("ROLE_SPEC");
+ WRITE_NODE_TYPE("ROLESPEC");
WRITE_ENUM_FIELD(roletype, RoleSpecType);
WRITE_STRING_FIELD(rolename);
- WRITE_INT_FIELD(location);
+ WRITE_LOCATION_FIELD(location);
}
+#endif
static void
_outFuncCall(StringInfo str, const FuncCall *node)
@@ -4408,6 +4410,7 @@ outNode(StringInfo str, const void *obj)
case T_PLAssignStmt:
_outPLAssignStmt(str, obj);
break;
+#if 0
case T_AlterTableStmt:
_outAlterTableStmt(str, obj);
break;
@@ -4417,6 +4420,7 @@ outNode(StringInfo str, const void *obj)
case T_RoleSpec:
_outRoleSpec(str, obj);
break;
+#endif
case T_ColumnDef:
_outColumnDef(str, obj);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7ee0414e0b..2a319eecda 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3708,11 +3708,6 @@ ColConstraintElem:
n->pk_attrs = $3;
n->fk_matchtype = $4;
n->fk_upd_action =
($5)->updateAction->action;
- if (($5)->updateAction->cols != NIL)
- ereport(ERROR,
-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("SET
NULL/DEFAULT <column_list> only supported for ON DELETE triggers"),
-
parser_errposition(@5)));
n->fk_del_action =
($5)->deleteAction->action;
n->fk_del_set_cols =
($5)->deleteAction->cols;
n->skip_validation = false;
@@ -3925,11 +3920,6 @@ ConstraintElem:
n->pk_attrs = $8;
n->fk_matchtype = $9;
n->fk_upd_action =
($10)->updateAction->action;
- if (($10)->updateAction->cols != NIL)
- ereport(ERROR,
-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("SET
NULL/DEFAULT <column_list> only supported for ON DELETE triggers"),
-
parser_errposition(@10)));
n->fk_del_action =
($10)->deleteAction->action;
n->fk_del_set_cols =
($10)->deleteAction->cols;
processCASbits($11, @11, "FOREIGN KEY",
@@ -4012,17 +4002,17 @@ OptWhereClause:
key_actions:
key_update
{
- KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
+ KeyActions *n =
palloc(sizeof(KeyActions));
n->updateAction = $1;
- n->deleteAction = (KeyAction *)
palloc(sizeof(KeyAction));
+ n->deleteAction =
palloc(sizeof(KeyAction));
n->deleteAction->action =
FKCONSTR_ACTION_NOACTION;
n->deleteAction->cols = NIL;
$$ = n;
}
| key_delete
{
- KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
- n->updateAction = (KeyAction *)
palloc(sizeof(KeyAction));
+ KeyActions *n =
palloc(sizeof(KeyActions));
+ n->updateAction =
palloc(sizeof(KeyAction));
n->updateAction->action =
FKCONSTR_ACTION_NOACTION;
n->updateAction->cols = NIL;
n->deleteAction = $1;
@@ -4030,69 +4020,81 @@ key_actions:
}
| key_update key_delete
{
- KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
+ KeyActions *n =
palloc(sizeof(KeyActions));
n->updateAction = $1;
n->deleteAction = $2;
$$ = n;
}
| key_delete key_update
{
- KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
+ KeyActions *n =
palloc(sizeof(KeyActions));
n->updateAction = $2;
n->deleteAction = $1;
$$ = n;
}
| /*EMPTY*/
{
- KeyActions *n = (KeyActions *)
palloc(sizeof(KeyActions));
- n->updateAction = (KeyAction *)
palloc(sizeof(KeyAction));
+ KeyActions *n =
palloc(sizeof(KeyActions));
+ n->updateAction =
palloc(sizeof(KeyAction));
n->updateAction->action =
FKCONSTR_ACTION_NOACTION;
n->updateAction->cols = NIL;
- n->deleteAction = (KeyAction *)
palloc(sizeof(KeyAction));
+ n->deleteAction =
palloc(sizeof(KeyAction));
n->deleteAction->action =
FKCONSTR_ACTION_NOACTION;
n->deleteAction->cols = NIL;
$$ = n;
}
;
-key_update: ON UPDATE key_action { $$ = $3; }
+key_update: ON UPDATE key_action
+ {
+ if (($3)->cols)
+ ereport(ERROR,
+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("a
column list with %s is only supported for ON DELETE actions",
+
($3)->action == FKCONSTR_ACTION_SETNULL ? "SET NULL" : "SET DEFAULT"),
+
parser_errposition(@1)));
+ $$ = $3;
+ }
;
-key_delete: ON DELETE_P key_action { $$ = $3; }
+key_delete: ON DELETE_P key_action
+ {
+ $$ = $3;
+ }
;
key_action:
NO ACTION
{
- KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
+ KeyAction *n =
palloc(sizeof(KeyAction));
n->action = FKCONSTR_ACTION_NOACTION;
n->cols = NIL;
$$ = n;
}
| RESTRICT
{
- KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
+ KeyAction *n =
palloc(sizeof(KeyAction));
n->action = FKCONSTR_ACTION_RESTRICT;
n->cols = NIL;
$$ = n;
}
| CASCADE
{
- KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
+ KeyAction *n =
palloc(sizeof(KeyAction));
n->action = FKCONSTR_ACTION_CASCADE;
n->cols = NIL;
$$ = n;
}
| SET NULL_P opt_column_list
{
- KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
+ KeyAction *n =
palloc(sizeof(KeyAction));
n->action = FKCONSTR_ACTION_SETNULL;
n->cols = $3;
$$ = n;
}
| SET DEFAULT opt_column_list
{
- KeyAction *n = (KeyAction *)
palloc(sizeof(KeyAction));
+ KeyAction *n =
palloc(sizeof(KeyAction));
n->action = FKCONSTR_ACTION_SETDEFAULT;
n->cols = $3;
$$ = n;
diff --git a/src/backend/utils/adt/ri_triggers.c
b/src/backend/utils/adt/ri_triggers.c
index a088622b8b..dc53ce6cad 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -72,15 +72,15 @@
#define RI_PLAN_CHECK_LOOKUPPK 1
#define RI_PLAN_CHECK_LOOKUPPK_FROM_PK 2
#define RI_PLAN_LAST_ON_PK
RI_PLAN_CHECK_LOOKUPPK_FROM_PK
-/* these queries are executed against the FK (referencing) table. */
-#define RI_PLAN_ONDELETE_CASCADE 3
-#define RI_PLAN_ONUPDATE_CASCADE 4
+/* these queries are executed against the FK (referencing) table: */
+#define RI_PLAN_ONDELETE_CASCADE 3
+#define RI_PLAN_ONUPDATE_CASCADE 4
/* the same plan can be used for both ON DELETE and ON UPDATE triggers. */
-#define RI_PLAN_ONTRIGGER_RESTRICT 5
-#define RI_PLAN_ONDELETE_SETNULL 6
-#define RI_PLAN_ONUPDATE_SETNULL 7
-#define RI_PLAN_ONDELETE_SETDEFAULT 8
-#define RI_PLAN_ONUPDATE_SETDEFAULT 9
+#define RI_PLAN_ONTRIGGER_RESTRICT 5 // XXX confusing name
+#define RI_PLAN_ONDELETE_SETNULL 6
+#define RI_PLAN_ONUPDATE_SETNULL 7
+#define RI_PLAN_ONDELETE_SETDEFAULT 8
+#define RI_PLAN_ONUPDATE_SETDEFAULT 9
#define MAX_QUOTED_NAME_LEN (NAMEDATALEN*2+3)
#define MAX_QUOTED_REL_NAME_LEN (MAX_QUOTED_NAME_LEN*2)
@@ -1128,7 +1128,9 @@ ri_set(TriggerData *trigdata, bool is_set_null, int
tgkind)
appendStringInfo(&querybuf, "UPDATE %s%s SET",
fk_only, fkrelname);
- // Add assignment clauses
+ /*
+ * Add assignment clauses
+ */
querysep = "";
for (int i = 0; i < num_cols_to_set; i++)
{
@@ -1140,14 +1142,16 @@ ri_set(TriggerData *trigdata, bool is_set_null, int
tgkind)
querysep = ",";
}
- // Add WHERE clause
+ /*
+ * Add WHERE clause
+ */
qualsep = "WHERE";
for (int i = 0; i < riinfo->nkeys; i++)
{
- Oid pk_type = RIAttType(pk_rel,
riinfo->pk_attnums[i]);
- Oid fk_type = RIAttType(fk_rel,
riinfo->fk_attnums[i]);
- Oid pk_coll = RIAttCollation(pk_rel,
riinfo->pk_attnums[i]);
- Oid fk_coll = RIAttCollation(fk_rel,
riinfo->fk_attnums[i]);
+ Oid pk_type = RIAttType(pk_rel,
riinfo->pk_attnums[i]);
+ Oid fk_type = RIAttType(fk_rel,
riinfo->fk_attnums[i]);
+ Oid pk_coll =
RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
+ Oid fk_coll =
RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
quoteOneName(attname,
RIAttName(fk_rel,
riinfo->fk_attnums[i]));
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index d5d5320c10..062a5b7051 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2262,12 +2262,6 @@ pg_get_constraintdef_worker(Oid constraintId, bool
fullCommand,
if (string)
appendStringInfo(&buf, " ON UPDATE %s",
string);
- val = SysCacheGetAttr(CONSTROID, tup,
-
Anum_pg_constraint_confdelsetcols, &isnull);
- if (isnull)
- elog(ERROR, "null confdelsetcols for
foreign key constraint %u",
- constraintId);
-
switch (conForm->confdeltype)
{
case FKCONSTR_ACTION_NOACTION:
@@ -2295,6 +2289,13 @@ pg_get_constraintdef_worker(Oid constraintId, bool
fullCommand,
appendStringInfo(&buf, " ON DELETE %s",
string);
/* Add columns specified to SET NULL or SET
DEFAULT if provided. */
+ // FIXME: use null instead of empty array for
standard behavior
+ val = SysCacheGetAttr(CONSTROID, tup,
+
Anum_pg_constraint_confdelsetcols, &isnull);
+ if (isnull)
+ elog(ERROR, "null confdelsetcols for
constraint %u",
+ constraintId);
+
if (ARR_NDIM(DatumGetArrayTypeP(val)) > 0)
{
appendStringInfo(&buf, " (");
diff --git a/src/include/catalog/pg_constraint.h
b/src/include/catalog/pg_constraint.h
index 4e256d4d89..dd0d975468 100644
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -142,7 +142,7 @@ CATALOG(pg_constraint,2606,ConstraintRelationId)
* If a foreign key with an ON DELETE SET NULL/DEFAULT action, the
subset
* of conkey to updated. If empty, all columns should be updated.
*/
- Oid confdelsetcols[1];
+ int16 confdelsetcols[1];
/*
* If an exclusion constraint, the OIDs of the exclusion operators for
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b682e9abb1..4c5a8a39bf 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2302,7 +2302,6 @@ typedef struct Constraint
char fk_upd_action; /* ON UPDATE action */
char fk_del_action; /* ON DELETE action */
List *fk_del_set_cols; /* ON DELETE SET NULL/DEFAULT (col1,
col2) */
-
List *old_conpfeqop; /* pg_constraint.conpfeqop of my former
self */
Oid old_pktable_oid; /*
pg_constraint.confrelid of my former
* self
*/
diff --git a/src/test/regress/expected/foreign_key.out
b/src/test/regress/expected/foreign_key.out
index a2680dbc9d..4c5274983d 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -764,7 +764,7 @@ ERROR: column "bar" referenced in foreign key constraint
does not exist
CREATE TABLE FKTABLE (tid int, id int, foo int, FOREIGN KEY (tid, id)
REFERENCES PKTABLE ON DELETE SET NULL (foo));
ERROR: column "foo" referenced in ON DELETE SET action must be part of
foreign key
CREATE TABLE FKTABLE (tid int, id int, foo int, FOREIGN KEY (tid, foo)
REFERENCES PKTABLE ON UPDATE SET NULL (foo));
-ERROR: SET NULL/DEFAULT <column_list> only supported for ON DELETE triggers
+ERROR: a column list with SET NULL is only supported for ON DELETE actions
LINE 1: ...oo int, FOREIGN KEY (tid, foo) REFERENCES PKTABLE ON UPDATE ...
^
CREATE TABLE FKTABLE (
--
2.33.1
