Code review for HeapTupleHeader changes. Add version number to page headers

(overlaying low byte of page size) and add HEAP_HASOID bit to t_infomask,
per earlier discussion.  Simplify scheme for overlaying fields in tuple
header (no need for cmax to live in more than one place).  Don't try to
clear infomask status bits in tqual.c --- not safe to do it there.  Don't
try to force output table of a SELECT INTO to have OIDs, either.  Get rid
of unnecessarily complex three-state scheme for TupleDesc.tdhasoids, which
has already caused one recent failure.  Improve documentation.

7 files changed, 52 insertions, 42 deletions

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6318d79d4eb..fd4431ce5f7 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -27,7 +27,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.176 2002/08/29 00:17:03 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.177 2002/09/02 01:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -713,21 +713,24 @@ InitPlan(CmdType operation, Query *parseTree, Plan *plan, EState *estate)
 								   get_namespace_name(namespaceId));
 
 				/*
-				 * new "INTO" table is created WITH OIDS
-				 */
-				tupType->tdhasoid = WITHOID;
-				/*
 				 * have to copy tupType to get rid of constraints
 				 */
 				tupdesc = CreateTupleDescCopy(tupType);
 
+				/*
+				 * Formerly we forced the output table to have OIDs, but
+				 * as of 7.3 it will not have OIDs, because it's too late
+				 * here to change the tupdescs of the already-initialized
+				 * plan tree.  (Perhaps we could recurse and change them
+				 * all, but it's not really worth the trouble IMHO...)
+				 */
+
 				intoRelationId =
 					heap_create_with_catalog(intoName,
 											 namespaceId,
 											 tupdesc,
 											 RELKIND_RELATION,
 											 false,
-											 true,
 											 allowSystemTableMods);
 
 				FreeTupleDesc(tupdesc);
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 30f5b0e378f..12b2089fd73 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.106 2002/08/31 22:10:43 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.107 2002/09/02 01:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -997,7 +997,7 @@ ExecMakeTableFunctionResult(Expr *funcexpr,
 					/*
 					 * Scalar type, so make a single-column descriptor
 					 */
-					tupdesc = CreateTemplateTupleDesc(1, WITHOUTOID);
+					tupdesc = CreateTemplateTupleDesc(1, false);
 					TupleDescInitEntry(tupdesc,
 									   (AttrNumber) 1,
 									   "column",
@@ -2006,10 +2006,7 @@ ExecTargetList(List *targetlist,
 	 * natts = 0 to deal with it.
 	 */
 	if (targettype == NULL)
-	{
 		targettype = &NullTupleDesc;
-		targettype->tdhasoid = WITHOUTOID;
-	}
 
 	/*
 	 * allocate an array of char's to hold the "null" information only if
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index afadcd3137b..e07fd8719ab 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execTuples.c,v 1.57 2002/08/29 00:17:03 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execTuples.c,v 1.58 2002/09/02 01:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -541,7 +541,6 @@ ExecInitNullTupleSlot(EState *estate, TupleDesc tupType)
 
 	ExecSetSlotDescriptor(slot, tupType, false);
 
-	NullTupleDesc.tdhasoid = WITHOUTOID;
 	nullTuple = heap_formtuple(&NullTupleDesc, values, nulls);
 
 	return ExecStoreTuple(nullTuple, slot, InvalidBuffer, true);
@@ -559,7 +558,7 @@ ExecInitNullTupleSlot(EState *estate, TupleDesc tupType)
  * ----------------------------------------------------------------
  */
 TupleDesc
-ExecTypeFromTL(List *targetList, hasoid_t withoid)
+ExecTypeFromTL(List *targetList, bool hasoid)
 {
 	List	   *tlitem;
 	TupleDesc	typeInfo;
@@ -578,7 +577,7 @@ ExecTypeFromTL(List *targetList, hasoid_t withoid)
 	/*
 	 * allocate a new typeInfo
 	 */
-	typeInfo = CreateTemplateTupleDesc(len, withoid);
+	typeInfo = CreateTemplateTupleDesc(len, hasoid);
 
 	/*
 	 * scan list, generate type info for each entry
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 24f232469b0..3e8cf203b11 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.88 2002/08/06 02:36:34 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.89 2002/09/02 01:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -291,22 +291,36 @@ void
 ExecAssignResultTypeFromTL(Plan *node, CommonState *commonstate)
 {
 	ResultRelInfo *ri;
-	Relation	rel;
-	hasoid_t	withoid;
+	bool		hasoid = false;
 	TupleDesc	tupDesc;
 
+	/*
+	 * This is pretty grotty: we need to ensure that result tuples have
+	 * space for an OID iff they are going to be stored into a relation
+	 * that has OIDs.  We assume that estate->es_result_relation_info
+	 * is already set up to describe the target relation.  One reason
+	 * this is ugly is that all plan nodes in the plan tree will emit
+	 * tuples with space for an OID, though we really only need the topmost
+	 * plan to do so.
+	 *
+	 * It would be better to have InitPlan adjust the topmost plan node's
+	 * output descriptor after plan tree initialization.  However, that
+	 * doesn't quite work because in an UPDATE that spans an inheritance
+	 * tree, some of the target relations may have OIDs and some not.
+	 * We have to make the decision on a per-relation basis as we initialize
+	 * each of the child plans of the topmost Append plan.  So, this is ugly
+	 * but it works, for now ...
+	 */
 	ri = node->state->es_result_relation_info;
 	if (ri != NULL)
-		rel = ri->ri_RelationDesc;
-	else
-		rel = node->state->es_into_relation_descriptor;
+	{
+		Relation	rel = ri->ri_RelationDesc;
 
-	if (rel != NULL)
-		withoid = BoolToHasOid(rel->rd_rel->relhasoids);
-	else
-		withoid = WITHOUTOID;
+		if (rel != NULL)
+			hasoid = rel->rd_rel->relhasoids;
+	}
 		
-	tupDesc = ExecTypeFromTL(node->targetlist, withoid);
+	tupDesc = ExecTypeFromTL(node->targetlist, hasoid);
 	ExecAssignResultType(commonstate, tupDesc, true);
 }
 
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 724f0c8ce89..8bc7136cb44 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeAppend.c,v 1.45 2002/06/20 20:29:28 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeAppend.c,v 1.46 2002/09/02 01:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -217,7 +217,9 @@ ExecInitAppend(Append *node, EState *estate, Plan *parent)
 
 	/*
 	 * call ExecInitNode on each of the plans to be executed and save the
-	 * results into the array "initialized"
+	 * results into the array "initialized".  Note we *must* set
+	 * estate->es_result_relation_info correctly while we initialize each
+	 * sub-plan; ExecAssignResultTypeFromTL depends on that!
 	 */
 	for (i = appendstate->as_firstplan; i <= appendstate->as_lastplan; i++)
 	{
diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index 89b5a544e5a..8a9ac848e7b 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.10 2002/08/31 19:09:27 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v 1.11 2002/09/02 01:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -209,7 +209,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, Plan *parent)
 		 */
 		char	   *attname = strVal(lfirst(rte->eref->colnames));
 
-		tupdesc = CreateTemplateTupleDesc(1, WITHOUTOID);
+		tupdesc = CreateTemplateTupleDesc(1, false);
 		TupleDescInitEntry(tupdesc,
 						   (AttrNumber) 1,
 						   attname,
@@ -223,10 +223,7 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, Plan *parent)
 		/*
 		 * Must be a pseudo type, i.e. record
 		 */
-		List *coldeflist = rte->coldeflist;
-
-		tupdesc = BuildDescForRelation(coldeflist);
-		tupdesc->tdhasoid = WITHOUTOID;
+		tupdesc = BuildDescForRelation(rte->coldeflist);
 	}
 	else
 		elog(ERROR, "Unknown kind of return type specified for function");
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 8535d875328..f0cc3fe17ae 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/spi.c,v 1.72 2002/07/20 05:16:58 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/spi.c,v 1.73 2002/09/02 01:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -392,7 +392,6 @@ SPI_modifytuple(Relation rel, HeapTuple tuple, int natts, int *attnum,
 	MemoryContext oldcxt = NULL;
 	HeapTuple	mtuple;
 	int			numberOfAttributes;
-	uint8		infomask;
 	Datum	   *v;
 	char	   *n;
 	bool		isnull;
@@ -434,14 +433,13 @@ SPI_modifytuple(Relation rel, HeapTuple tuple, int natts, int *attnum,
 	if (i == natts)				/* no errors in *attnum */
 	{
 		mtuple = heap_formtuple(rel->rd_att, v, n);
-		infomask = mtuple->t_data->t_infomask;
 		/*
-		 * copy t_xmin, t_cid, t_xmax, t_ctid, t_natts, t_infomask
+		 * copy the identification info of the old tuple: t_ctid, t_self,
+		 * and OID (if any)
 		 */
-		memmove((char *)mtuple->t_data, (char *)tuple->t_data,
-				offsetof(HeapTupleHeaderData, t_hoff));
-		mtuple->t_data->t_infomask = infomask;
-		mtuple->t_data->t_natts = numberOfAttributes;
+		mtuple->t_data->t_ctid = tuple->t_data->t_ctid;
+		mtuple->t_self = tuple->t_self;
+		mtuple->t_tableOid = tuple->t_tableOid;
 		if (rel->rd_rel->relhasoids)
 			HeapTupleSetOid(mtuple, HeapTupleGetOid(tuple));
 	}