Fix infer_arbiter_indexes() to not barf on system columns.

tglsfdc · tglsfdc · commit 428484ce102b · 2016-05-11T17:06:53.000-04:00
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.
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
@@ -465,41 +465,32 @@ 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;
 		}
 
 		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);
 	}
 
 	/*
@@ -516,18 +507,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;
@@ -586,17 +577,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 */
