Make SPI_fnumber() reject dropped columns.

There's basically no scenario where it's sensible for this to match
dropped columns, so put a test for dropped-ness into SPI_fnumber()
itself, and excise the test from the small number of callers that
were paying attention to the case.  (Most weren't :-(.)

In passing, normalize tests at call sites: always reject attnum <= 0
if we're disallowing system columns.  Previously there was a mixture
of "< 0" and "<= 0" tests.  This makes no practical difference since
SPI_fnumber() never returns 0, but I'm feeling pedantic today.

Also, in the places that are actually live user-facing code and not
legacy cruft, distinguish "column not found" from "can't handle
system column".

Per discussion with Jim Nasby; thi supersedes his original patch
that just changed the behavior at one call site.

Discussion: <b2de8258-c4c0-1cb8-7b97-e8538e5c975c@BlueTreble.com>

5 files changed, 9 insertions, 8 deletions

diff --git a/contrib/spi/autoinc.c b/contrib/spi/autoinc.c
index 41eae4fdc45..fc657a7c06e 100644
--- a/contrib/spi/autoinc.c
+++ b/contrib/spi/autoinc.c
@@ -71,7 +71,7 @@ autoinc(PG_FUNCTION_ARGS)
 		int32		val;
 		Datum		seqname;
 
-		if (attnum < 0)
+		if (attnum <= 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_TRIGGERED_ACTION_EXCEPTION),
 					 errmsg("\"%s\" has no attribute \"%s\"",
diff --git a/contrib/spi/insert_username.c b/contrib/spi/insert_username.c
index 3812525c4c0..617c60a81c5 100644
--- a/contrib/spi/insert_username.c
+++ b/contrib/spi/insert_username.c
@@ -67,7 +67,7 @@ insert_username(PG_FUNCTION_ARGS)
 
 	attnum = SPI_fnumber(tupdesc, args[0]);
 
-	if (attnum < 0)
+	if (attnum <= 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_TRIGGERED_ACTION_EXCEPTION),
 				 errmsg("\"%s\" has no attribute \"%s\"", relname, args[0])));
diff --git a/contrib/spi/moddatetime.c b/contrib/spi/moddatetime.c
index c6d33b73557..cd700fe6d13 100644
--- a/contrib/spi/moddatetime.c
+++ b/contrib/spi/moddatetime.c
@@ -84,9 +84,9 @@ moddatetime(PG_FUNCTION_ARGS)
 
 	/*
 	 * This is where we check to see if the field we are supposed to update
-	 * even exists. The above function must return -1 if name not found?
+	 * even exists.
 	 */
-	if (attnum < 0)
+	if (attnum <= 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_TRIGGERED_ACTION_EXCEPTION),
 				 errmsg("\"%s\" has no attribute \"%s\"",
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index 01dd717522c..78cfedf219f 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -135,7 +135,7 @@ check_primary_key(PG_FUNCTION_ARGS)
 		int			fnumber = SPI_fnumber(tupdesc, args[i]);
 
 		/* Bad guys may give us un-existing column in CREATE TRIGGER */
-		if (fnumber < 0)
+		if (fnumber <= 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_COLUMN),
 					 errmsg("there is no attribute \"%s\" in relation \"%s\"",
@@ -362,7 +362,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 		int			fnumber = SPI_fnumber(tupdesc, args[i]);
 
 		/* Bad guys may give us un-existing column in CREATE TRIGGER */
-		if (fnumber < 0)
+		if (fnumber <= 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_COLUMN),
 					 errmsg("there is no attribute \"%s\" in relation \"%s\"",
@@ -469,6 +469,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 						char	   *type;
 
 						fn = SPI_fnumber(tupdesc, args_temp[k - 1]);
+						Assert(fn > 0); /* already checked above */
 						nv = SPI_getvalue(newtuple, tupdesc, fn);
 						type = SPI_gettype(tupdesc, fn);
 
diff --git a/contrib/spi/timetravel.c b/contrib/spi/timetravel.c
index 5a345841c6a..30dcfd4d3ec 100644
--- a/contrib/spi/timetravel.c
+++ b/contrib/spi/timetravel.c
@@ -157,7 +157,7 @@ timetravel(PG_FUNCTION_ARGS)
 	for (i = 0; i < MinAttrNum; i++)
 	{
 		attnum[i] = SPI_fnumber(tupdesc, args[i]);
-		if (attnum[i] < 0)
+		if (attnum[i] <= 0)
 			elog(ERROR, "timetravel (%s): there is no attribute %s", relname, args[i]);
 		if (SPI_gettypeid(tupdesc, attnum[i]) != ABSTIMEOID)
 			elog(ERROR, "timetravel (%s): attribute %s must be of abstime type",
@@ -166,7 +166,7 @@ timetravel(PG_FUNCTION_ARGS)
 	for (; i < argc; i++)
 	{
 		attnum[i] = SPI_fnumber(tupdesc, args[i]);
-		if (attnum[i] < 0)
+		if (attnum[i] <= 0)
 			elog(ERROR, "timetravel (%s): there is no attribute %s", relname, args[i]);
 		if (SPI_gettypeid(tupdesc, attnum[i]) != TEXTOID)
 			elog(ERROR, "timetravel (%s): attribute %s must be of text type",