Fix infer_arbiter_indexes() to not barf on system columns.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 11 May 2016 21:06:53 +0000 (17:06 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 11 May 2016 21:06:53 +0000 (17:06 -0400)
While it could be argued that rejecting system column mentions in the
ON CONFLICT list is an unsupported feature, falling over altogether
just because the table has a unique index on OID is indubitably a bug.

As far as I can tell, fixing infer_arbiter_indexes() is sufficient to
make ON CONFLICT (oid) actually work, though making a regression test
for that case is problematic because of the impossibility of setting
the OID counter to a known value.

Minor cosmetic cleanups along with the bug fix.

src/backend/optimizer/util/plancat.c

index 4f399a5b85471f39348a4e50b5bb9f0db1de4cab..b0b606ebc82380306dbbefc22b6aa7ee3526ebf6 100644 (file)
@@ -558,26 +558,18 @@ infer_arbiter_indexes(PlannerInfo *root)
 
    /*
     * Build normalized/BMS representation of plain indexed attributes, as
-    * well as direct list of inference elements.  This is required for
-    * matching the cataloged definition of indexes.
+    * well as a separate list of expression items.  This simplifies matching
+    * the cataloged definition of indexes.
     */
    foreach(l, onconflict->arbiterElems)
    {
-       InferenceElem *elem;
+       InferenceElem *elem = (InferenceElem *) lfirst(l);
        Var        *var;
        int         attno;
 
-       elem = (InferenceElem *) lfirst(l);
-
-       /*
-        * Parse analysis of inference elements performs full parse analysis
-        * of Vars, even for non-expression indexes (in contrast with utility
-        * command related use of IndexElem).  However, indexes are cataloged
-        * with simple attribute numbers for non-expression indexes.  Those
-        * are handled later.
-        */
        if (!IsA(elem->expr, Var))
        {
+           /* If not a plain Var, just shove it in inferElems for now */
            inferElems = lappend(inferElems, elem->expr);
            continue;
        }
@@ -585,14 +577,13 @@ infer_arbiter_indexes(PlannerInfo *root)
        var = (Var *) elem->expr;
        attno = var->varattno;
 
-       if (attno < 0)
+       if (attno == 0)
            ereport(ERROR,
-                   (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
-                    errmsg("system columns cannot be used in an ON CONFLICT clause")));
-       else if (attno == 0)
-           elog(ERROR, "whole row unique index inference specifications are not valid");
+                   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                    errmsg("whole row unique index inference specifications are not supported")));
 
-       inferAttrs = bms_add_member(inferAttrs, attno);
+       inferAttrs = bms_add_member(inferAttrs,
+                                attno - FirstLowInvalidHeapAttributeNumber);
    }
 
    /*
@@ -609,18 +600,18 @@ infer_arbiter_indexes(PlannerInfo *root)
                     errmsg("constraint in ON CONFLICT clause has no associated index")));
    }
 
-   indexList = RelationGetIndexList(relation);
-
    /*
     * Using that representation, iterate through the list of indexes on the
     * target relation to try and find a match
     */
+   indexList = RelationGetIndexList(relation);
+
    foreach(l, indexList)
    {
        Oid         indexoid = lfirst_oid(l);
        Relation    idxRel;
        Form_pg_index idxForm;
-       Bitmapset  *indexedAttrs = NULL;
+       Bitmapset  *indexedAttrs;
        List       *idxExprs;
        List       *predExprs;
        AttrNumber  natt;
@@ -679,17 +670,15 @@ infer_arbiter_indexes(PlannerInfo *root)
        if (!idxForm->indisunique)
            goto next;
 
-       /* Build BMS representation of cataloged index attributes */
+       /* Build BMS representation of plain (non expression) index attrs */
+       indexedAttrs = NULL;
        for (natt = 0; natt < idxForm->indnatts; natt++)
        {
            int         attno = idxRel->rd_index->indkey.values[natt];
 
-           /* XXX broken */
-           if (attno < 0)
-               elog(ERROR, "system column in index");
-
            if (attno != 0)
-               indexedAttrs = bms_add_member(indexedAttrs, attno);
+               indexedAttrs = bms_add_member(indexedAttrs,
+                                attno - FirstLowInvalidHeapAttributeNumber);
        }
 
        /* Non-expression attributes (if any) must match */