Repair misbehavior with duplicate entries in FK SET column lists.

Since v15 we've had an option to apply a foreign key constraint's
ON DELETE SET DEFAULT or SET NULL action to just some of the
referencing columns.  There was not a check for duplicate entries in
the list of columns-to-set, though.  That caused a potential memory
stomp in CreateConstraintEntry(), which incautiously assumed that
the list of columns-to-set couldn't be longer than the number of key
columns.  Even after fixing that, the case doesn't work because you
get an error like "multiple assignments to same column" from the SQL
command that is generated to do the update.

We could either raise an error for duplicate columns or silently
suppress the dups, and after a bit of thought I chose to do the
latter.  This is motivated by the fact that duplicates in the FK
column list are legal, so it's not real clear why duplicates
in the columns-to-set list shouldn't be.  Of course there's no
need to actually set the column more than once.

I left in the fix in CreateConstraintEntry() too, just because
it didn't seem like such low-level code ought to be making
assumptions about what it's handed.

Bug: #18879
Reported-by: Yu Liang <luy70@psu.edu>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18879-259fc59d072bd4d7@postgresql.org
Backpatch-through: 15

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index c269ac251ac1e4b72945b2aea712d7a5a1b54374..b3a610e7fb95c1f8e3a774106da6a26eb9d2a31e 100644 (file)
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -121,8 +121,9 @@ CreateConstraintEntry(const char *constraintName,
        if (foreignNKeys > 0)
        {
                Datum      *fkdatums;
+               int                     nkeys = Max(foreignNKeys, numFkDeleteSetCols);
 
-               fkdatums = (Datum *) palloc(foreignNKeys * sizeof(Datum));
+               fkdatums = (Datum *) palloc(nkeys * sizeof(Datum));
                for (i = 0; i < foreignNKeys; i++)
                        fkdatums[i] = Int16GetDatum(foreignKey[i]);
                confkeyArray = construct_array(fkdatums, foreignNKeys,

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 58e4a7ec9fac42b9b4f27c188152b3ace52f7b2e..55b72acda3699d20b76c516610d4f937c3460121 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -497,8 +497,8 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *
                                                                                           Relation rel, Constraint *fkconstraint,
                                                                                           bool recurse, bool recursing,
                                                                                           LOCKMODE lockmode);
-static void validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
-                                                                                int numfksetcols, const int16 *fksetcolsattnums,
+static int     validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
+                                                                                int numfksetcols, int16 *fksetcolsattnums,
                                                                                 List *fksetcols);
 static ObjectAddress addFkConstraint(addFkConstraintSides fkside,
                                                                         char *constraintname,
@@ -9309,9 +9309,10 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
        numfkdelsetcols = transformColumnNameList(RelationGetRelid(rel),
                                                                                          fkconstraint->fk_del_set_cols,
                                                                                          fkdelsetcols, NULL);
-       validateFkOnDeleteSetColumns(numfks, fkattnum,
-                                                                numfkdelsetcols, fkdelsetcols,
-                                                                fkconstraint->fk_del_set_cols);
+       numfkdelsetcols = validateFkOnDeleteSetColumns(numfks, fkattnum,
+                                                                                                  numfkdelsetcols,
+                                                                                                  fkdelsetcols,
+                                                                                                  fkconstraint->fk_del_set_cols);
 
        /*
         * If the attribute list for the referenced table was omitted, lookup the
@@ -9632,17 +9633,23 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
  * validateFkOnDeleteSetColumns
  *             Verifies that columns used in ON DELETE SET NULL/DEFAULT (...)
  *             column lists are valid.
+ *
+ * If there are duplicates in the fksetcolsattnums[] array, this silently
+ * removes the dups.  The new count of numfksetcols is returned.
  */
-void
+static int
 validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
-                                                        int numfksetcols, const int16 *fksetcolsattnums,
+                                                        int numfksetcols, int16 *fksetcolsattnums,
                                                         List *fksetcols)
 {
+       int                     numcolsout = 0;
+
        for (int i = 0; i < numfksetcols; i++)
        {
                int16           setcol_attnum = fksetcolsattnums[i];
                bool            seen = false;
 
+               /* Make sure it's in fkattnums[] */
                for (int j = 0; j < numfks; j++)
                {
                        if (fkattnums[j] == setcol_attnum)
@@ -9660,7 +9667,21 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
                                        (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
                                         errmsg("column \"%s\" referenced in ON DELETE SET action must be part of foreign key", col)));
                }
+
+               /* Now check for dups */
+               seen = false;
+               for (int j = 0; j < numcolsout; j++)
+               {
+                       if (fksetcolsattnums[j] == setcol_attnum)
+                       {
+                               seen = true;
+                               break;
+                       }
+               }
+               if (!seen)
+                       fksetcolsattnums[numcolsout++] = setcol_attnum;
        }
+       return numcolsout;
 }
 
 /*

diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index c8c2f420a2be992542711d9bab52a9ed69c3f3b6..cf188450ba3a86f73901f922f0f1a6f877601ed6 100644 (file)
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -772,7 +772,8 @@ CREATE TABLE FKTABLE (
   fk_id_del_set_null int,
   fk_id_del_set_default int DEFAULT 0,
   FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES PKTABLE ON DELETE SET NULL (fk_id_del_set_null),
-  FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE ON DELETE SET DEFAULT (fk_id_del_set_default)
+  -- this tests handling of duplicate entries in SET DEFAULT column list
+  FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE ON DELETE SET DEFAULT (fk_id_del_set_default, fk_id_del_set_default)
 );
 SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass::oid ORDER BY oid;
                                                 pg_get_constraintdef                                                

diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 978387740b350473ab1c5580e10706d3bd147446..c0125823256f10c3aec0c5fcaee21f89dfd7654d 100644 (file)
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -473,7 +473,8 @@ CREATE TABLE FKTABLE (
   fk_id_del_set_null int,
   fk_id_del_set_default int DEFAULT 0,
   FOREIGN KEY (tid, fk_id_del_set_null) REFERENCES PKTABLE ON DELETE SET NULL (fk_id_del_set_null),
-  FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE ON DELETE SET DEFAULT (fk_id_del_set_default)
+  -- this tests handling of duplicate entries in SET DEFAULT column list
+  FOREIGN KEY (tid, fk_id_del_set_default) REFERENCES PKTABLE ON DELETE SET DEFAULT (fk_id_del_set_default, fk_id_del_set_default)
 );
 
 SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = 'fktable'::regclass::oid ORDER BY oid;