Reject duplicate column names in foreign key referenced-columns lists.

Such cases are disallowed by the SQL spec, and even if we wanted to allow
them, the semantics seem ambiguous: how should the FK columns be matched up
with the columns of a unique index?  (The matching could be significant in
the presence of opclasses with different notions of equality, so this issue
isn't just academic.)  However, our code did not previously reject such
cases, but instead would either fail to match to any unique index, or
generate a bizarre opclass-lookup error because of sloppy thinking in the
index-matching code.

David Rowley

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5dc4d18b6a5a63b26244fe27e4c0a649306f830e..89bd31ab93e2e20ec1e0826d7c9ca9d7653bddbd 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6739,6 +6739,26 @@ transformFkeyCheckAttrs(Relation pkrel,
        bool            found_deferrable = false;
        List       *indexoidlist;
        ListCell   *indexoidscan;
+       int                     i,
+                               j;
+
+       /*
+        * Reject duplicate appearances of columns in the referenced-columns list.
+        * Such a case is forbidden by the SQL standard, and even if we thought it
+        * useful to allow it, there would be ambiguity about how to match the
+        * list to unique indexes (in particular, it'd be unclear which index
+        * opclass goes with which FK column).
+        */
+       for (i = 0; i < numattrs; i++)
+       {
+               for (j = i + 1; j < numattrs; j++)
+               {
+                       if (attnums[i] == attnums[j])
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_FOREIGN_KEY),
+                                                errmsg("foreign key referenced-columns list must not contain duplicates")));
+               }
+       }
 
        /*
         * Get the list of index OIDs for the table from the relcache, and look up
@@ -6751,8 +6771,6 @@ transformFkeyCheckAttrs(Relation pkrel,
        {
                HeapTuple       indexTuple;
                Form_pg_index indexStruct;
-               int                     i,
-                                       j;
 
                indexoid = lfirst_oid(indexoidscan);
                indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexoid));
@@ -6771,11 +6789,11 @@ transformFkeyCheckAttrs(Relation pkrel,
                        heap_attisnull(indexTuple, Anum_pg_index_indpred) &&
                        heap_attisnull(indexTuple, Anum_pg_index_indexprs))
                {
-                       /* Must get indclass the hard way */
                        Datum           indclassDatum;
                        bool            isnull;
                        oidvector  *indclass;
 
+                       /* Must get indclass the hard way */
                        indclassDatum = SysCacheGetAttr(INDEXRELID, indexTuple,
                                                                                        Anum_pg_index_indclass, &isnull);
                        Assert(!isnull);
@@ -6783,7 +6801,13 @@ transformFkeyCheckAttrs(Relation pkrel,
 
                        /*
                         * The given attnum list may match the index columns in any order.
-                        * Check that each list is a subset of the other.
+                        * Check for a match, and extract the appropriate opclasses while
+                        * we're at it.
+                        *
+                        * We know that attnums[] is duplicate-free per the test at the
+                        * start of this function, and we checked above that the number of
+                        * index columns agrees, so if we find a match for each attnums[]
+                        * entry then we must have a one-to-one match in some order.
                         */
                        for (i = 0; i < numattrs; i++)
                        {
@@ -6792,6 +6816,7 @@ transformFkeyCheckAttrs(Relation pkrel,
                                {
                                        if (attnums[i] == indexStruct->indkey.values[j])
                                        {
+                                               opclasses[i] = indclass->values[j];
                                                found = true;
                                                break;
                                        }
@@ -6799,24 +6824,6 @@ transformFkeyCheckAttrs(Relation pkrel,
                                if (!found)
                                        break;
                        }
-                       if (found)
-                       {
-                               for (i = 0; i < numattrs; i++)
-                               {
-                                       found = false;
-                                       for (j = 0; j < numattrs; j++)
-                                       {
-                                               if (attnums[j] == indexStruct->indkey.values[i])
-                                               {
-                                                       opclasses[j] = indclass->values[i];
-                                                       found = true;
-                                                       break;
-                                               }
-                                       }
-                                       if (!found)
-                                               break;
-                               }
-                       }
 
                        /*
                         * Refuse to use a deferrable unique/primary key.  This is per SQL