Make the world safe (more or less) for dropped columns in plpgsql rowtypes.

2 files changed, 62 insertions, 41 deletions

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 923d418b3a9..74560b1ba1d 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.145 2003/09/25 06:57:59 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.146 2003/09/25 23:02:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -315,23 +315,6 @@ ExecEvalAggref(AggrefExprState *aggref, ExprContext *econtext, bool *isNull)
  *
  *		Returns a Datum whose value is the value of a range
  *		variable with respect to given expression context.
- *
- *
- *		As an entry condition, we expect that the datatype the
- *		plan expects to get (as told by our "variable" argument) is in
- *		fact the datatype of the attribute the plan says to fetch (as
- *		seen in the current context, identified by our "econtext"
- *		argument).
- *
- *		If we fetch a Type A attribute and Caller treats it as if it
- *		were Type B, there will be undefined results (e.g. crash).
- *		One way these might mismatch now is that we're accessing a
- *		catalog class and the type information in the pg_attribute
- *		class does not match the hardcoded pg_attribute information
- *		(in pg_attribute.h) for the class in question.
- *
- *		We have an Assert to make sure this entry condition is met.
- *
  * ---------------------------------------------------------------- */
 static Datum
 ExecEvalVar(Var *variable, ExprContext *econtext, bool *isNull)
@@ -369,11 +352,40 @@ ExecEvalVar(Var *variable, ExprContext *econtext, bool *isNull)
 
 	attnum = variable->varattno;
 
-	/* (See prolog for explanation of this Assert) */
-	Assert(attnum <= 0 ||
-		   (attnum - 1 <= tuple_type->natts - 1 &&
-			tuple_type->attrs[attnum - 1] != NULL &&
-		  variable->vartype == tuple_type->attrs[attnum - 1]->atttypid));
+	/*
+	 * Some checks that are only applied for user attribute numbers
+	 * (bogus system attnums will be caught inside heap_getattr).
+	 */
+	if (attnum > 0)
+	{
+		/*
+		 * This assert checks that the attnum is valid.
+		 */
+		Assert(attnum <= tuple_type->natts &&
+			   tuple_type->attrs[attnum - 1] != NULL);
+
+		/*
+		 * If the attribute's column has been dropped, we force a NULL result.
+		 * This case should not happen in normal use, but it could happen if
+		 * we are executing a plan cached before the column was dropped.
+		 */
+		if (tuple_type->attrs[attnum - 1]->attisdropped)
+		{
+			*isNull = true;
+			return (Datum) 0;
+		}
+
+		/*
+		 * This assert checks that the datatype the plan expects to get (as
+		 * told by our "variable" argument) is in fact the datatype of the
+		 * attribute being fetched (as seen in the current context, identified
+		 * by our "econtext" argument).  Otherwise crashes are likely.
+		 *
+		 * Note that we can't check dropped columns, since their atttypid
+		 * has been zeroed.
+		 */
+		Assert(variable->vartype == tuple_type->attrs[attnum - 1]->atttypid);
+	}
 
 	/*
 	 * If the attribute number is invalid, then we are supposed to return
diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index ca25774805b..a47e37d7e55 100644
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.21 2003/09/25 06:57:59 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.22 2003/09/25 23:02:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -35,7 +35,7 @@
 
 
 static TupleTableSlot *FunctionNext(FunctionScanState *node);
-static bool tupledesc_mismatch(TupleDesc tupdesc1, TupleDesc tupdesc2);
+static bool tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc);
 
 /* ----------------------------------------------------------------
  *						Scan Support
@@ -86,8 +86,7 @@ FunctionNext(FunctionScanState *node)
 		 * need to do this for functions returning RECORD, but might as
 		 * well do it always.
 		 */
-		if (funcTupdesc &&
-			tupledesc_mismatch(node->tupdesc, funcTupdesc))
+		if (funcTupdesc && !tupledesc_match(node->tupdesc, funcTupdesc))
 			ereport(ERROR,
 					(errcode(ERRCODE_DATATYPE_MISMATCH),
 					 errmsg("query-specified return row and actual function return row do not match")));
@@ -364,26 +363,36 @@ ExecFunctionReScan(FunctionScanState *node, ExprContext *exprCtxt)
 		tuplestore_rescan(node->tuplestorestate);
 }
 
-
+/*
+ * Check that function result tuple type (src_tupdesc) matches or can be
+ * considered to match what the query expects (dst_tupdesc).
+ *
+ * We really only care about number of attributes and data type.
+ * Also, we can ignore type mismatch on columns that are dropped in the
+ * destination type, so long as the physical storage matches.  This is
+ * helpful in some cases involving out-of-date cached plans.
+ */
 static bool
-tupledesc_mismatch(TupleDesc tupdesc1, TupleDesc tupdesc2)
+tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc)
 {
 	int			i;
 
-	if (tupdesc1->natts != tupdesc2->natts)
-		return true;
+	if (dst_tupdesc->natts != src_tupdesc->natts)
+		return false;
 
-	for (i = 0; i < tupdesc1->natts; i++)
+	for (i = 0; i < dst_tupdesc->natts; i++)
 	{
-		Form_pg_attribute attr1 = tupdesc1->attrs[i];
-		Form_pg_attribute attr2 = tupdesc2->attrs[i];
-
-		/*
-		 * We really only care about number of attributes and data type
-		 */
-		if (attr1->atttypid != attr2->atttypid)
-			return true;
+		Form_pg_attribute dattr = dst_tupdesc->attrs[i];
+		Form_pg_attribute sattr = src_tupdesc->attrs[i];
+
+		if (dattr->atttypid == sattr->atttypid)
+			continue;			/* no worries */
+		if (!dattr->attisdropped)
+			return false;
+		if (dattr->attlen != sattr->attlen ||
+			dattr->attalign != sattr->attalign)
+			return false;
 	}
 
-	return false;
+	return true;
 }