Make new expression eval code reject references to dropped columns.

Formerly, a Var referencing an already-dropped column was allowed and would
always produce a NULL value.  However, that behavior was implemented in
slot_getattr which the new expression code doesn't use; thus there is now a
risk of returning theoretically-deleted data.  We had regression test cases
that purported to exercise this, but they failed to expose any problem,
apparently because plpgsql filters the dropped column and produces an
output tuple that has a NULL there already.

Ideally the DROP or ALTER attempt in these test cases would get rejected
due to dependency checks; but until that happens, let's modify the behavior
so that we fail the query during executor start.  This was already true for
the related case of a column having changed type underneath us, and there's
no obvious reason why we need to be laxer for dropped columns.

In passing, adjust the error messages in CheckVarSlotCompatibility to
include the composite type name.  In the cases shown in the regression
tests this is always just "record", but it should be more useful in
actual stale-plan cases, where the slot tupdesc would be a table's
tupdesc directly.

Discussion: https://postgr.es/m/16803.1490723570@sss.pgh.pa.us

1 files changed, 23 insertions, 22 deletions

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 982d16c6c88..4fbb5c1e746 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -1516,22 +1516,20 @@ CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype)
 {
 	/*
 	 * What we have to check for here is the possibility of an attribute
-	 * having been changed in type since the plan tree was created.  Ideally
-	 * the plan will get invalidated and not re-used, but just in case, we
-	 * keep these defenses.  Fortunately it's sufficient to check once on the
-	 * first time through.
-	 *
-	 * System attributes don't require checking since their types never
-	 * change.
-	 *
-	 * Note: we allow a reference to a dropped attribute.  slot_getattr will
-	 * force a NULL result in such cases.
+	 * having been dropped or changed in type since the plan tree was created.
+	 * Ideally the plan will get invalidated and not re-used, but just in
+	 * case, we keep these defenses.  Fortunately it's sufficient to check
+	 * once on the first time through.
 	 *
 	 * Note: ideally we'd check typmod as well as typid, but that seems
 	 * impractical at the moment: in many cases the tupdesc will have been
 	 * generated by ExecTypeFromTL(), and that can't guarantee to generate an
 	 * accurate typmod in all cases, because some expression node types don't
-	 * carry typmod.
+	 * carry typmod.  Fortunately, for precisely that reason, there should be
+	 * no places with a critical dependency on the typmod of a value.
+	 *
+	 * System attributes don't require checking since their types never
+	 * change.
 	 */
 	if (attnum > 0)
 	{
@@ -1544,17 +1542,20 @@ CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype)
 
 		attr = slot_tupdesc->attrs[attnum - 1];
 
-		/* can't check type if dropped, since atttypid is probably 0 */
-		if (!attr->attisdropped)
-		{
-			if (vartype != attr->atttypid)
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("attribute %d has wrong type", attnum),
-						 errdetail("Table has type %s, but query expects %s.",
-								   format_type_be(attr->atttypid),
-								   format_type_be(vartype))));
-		}
+		if (attr->attisdropped)
+			ereport(ERROR,
+					(errcode(ERRCODE_UNDEFINED_COLUMN),
+					 errmsg("attribute %d of type %s has been dropped",
+							attnum, format_type_be(slot_tupdesc->tdtypeid))));
+
+		if (vartype != attr->atttypid)
+			ereport(ERROR,
+					(errcode(ERRCODE_DATATYPE_MISMATCH),
+					 errmsg("attribute %d of type %s has wrong type",
+							attnum, format_type_be(slot_tupdesc->tdtypeid)),
+					 errdetail("Table has type %s, but query expects %s.",
+							   format_type_be(attr->atttypid),
+							   format_type_be(vartype))));
 	}
 }