Thread: Wrap access to Oid in HeapTupleHeader

Macros HeapTupleHeaderGetOid, HeapTupleHeaderSetOid, HeapTupleGetOid,
and HeapTupleSetOid are defined in htup.h, accesses to
HeapTupleHeader->t_oid are now wrapped by using these macros, wherever
I could find them.

This patch is not intended to change any behaviour, but in preparation
of changes I plan to post later this week it indroduces two
pre-conditions for accessing the oid:
.  the relation has oids
.  the heap tuple header is already formatted, i.e. t_hoff is valid

I have added a lot of assertions to the best of my knowledge; at first
too many of them :-)  After some tweaking all regression tests are
passed now.

This patch has been created against a snapshot from 2002-06-17 patched
with my previous patches.  If this causes problems, let me know and
I'll submit a new version.

Servus
 Manfred

diff -ru ../base/src/backend/access/common/heaptuple.c src/backend/access/common/heaptuple.c
--- ../base/src/backend/access/common/heaptuple.c    2002-06-17 10:11:31.000000000 +0200
+++ src/backend/access/common/heaptuple.c    2002-06-30 00:11:55.000000000 +0200
@@ -436,7 +436,7 @@
             result = PointerGetDatum(&(tup->t_self));
             break;
         case ObjectIdAttributeNumber:
-            result = ObjectIdGetDatum(tup->t_data->t_oid);
+            result = ObjectIdGetDatum(HeapTupleGetOid(tup));
             break;
         case MinTransactionIdAttributeNumber:
             result = TransactionIdGetDatum(HeapTupleHeaderGetXmin(tup->t_data));
@@ -698,14 +698,21 @@
      * t_infomask
      */
     infomask = newTuple->t_data->t_infomask;
-    memmove((char *) &newTuple->t_data->t_oid,    /* XXX */
-            (char *) &tuple->t_data->t_oid,
-            ((char *) &tuple->t_data->t_hoff -
-             (char *) &tuple->t_data->t_oid));    /* XXX */
+    /*
+     * PG72FORMAT:
+     * copy t_cmin, t_cmax, t_xmin, t_xmax, t_ctid, t_natts, t_infomask
+     * PG73FORMAT:
+     * copy t_xmin, t_cid, t_xmax, t_ctid, t_natts, t_infomask
+     */
+    memmove((char *) newTuple->t_data,    /* XXX */
+            (char *) tuple->t_data,
+            offsetof(HeapTupleHeaderData, t_hoff));    /* XXX */
     newTuple->t_data->t_infomask = infomask;
     newTuple->t_data->t_natts = numberOfAttributes;
     newTuple->t_self = tuple->t_self;
     newTuple->t_tableOid = tuple->t_tableOid;
+    if (relation->rd_rel->relhasoids)
+        HeapTupleSetOid(newTuple, HeapTupleGetOid(tuple));

     return newTuple;
 }
diff -ru ../base/src/backend/access/common/tupdesc.c src/backend/access/common/tupdesc.c
--- ../base/src/backend/access/common/tupdesc.c    2002-03-29 21:05:59.000000000 +0100
+++ src/backend/access/common/tupdesc.c    2002-06-30 00:15:22.000000000 +0200
@@ -389,7 +389,7 @@
      */
     typeForm = (Form_pg_type) GETSTRUCT(tuple);

-    att->atttypid = tuple->t_data->t_oid;
+    att->atttypid = HeapTupleGetOid(tuple);

     /*
      * There are a couple of cases where we must override the information
diff -ru ../base/src/backend/access/heap/heapam.c src/backend/access/heap/heapam.c
--- ../base/src/backend/access/heap/heapam.c    2002-06-19 16:57:07.000000000 +0200
+++ src/backend/access/heap/heapam.c    2002-06-30 03:31:06.000000000 +0200
@@ -1116,10 +1116,10 @@
          * to support a persistent object store (objects need to contain
          * pointers to one another).
          */
-        if (!OidIsValid(tup->t_data->t_oid))
-            tup->t_data->t_oid = newoid();
+        if (!OidIsValid(HeapTupleGetOid(tup)))
+            HeapTupleSetOid(tup, newoid());
         else
-            CheckMaxObjectId(tup->t_data->t_oid);
+            CheckMaxObjectId(HeapTupleGetOid(tup));
     }

     HeapTupleHeaderSetXmin(tup->t_data, GetCurrentTransactionId());
@@ -1166,7 +1166,10 @@
         rdata[0].len = SizeOfHeapInsert;
         rdata[0].next = &(rdata[1]);

-        xlhdr.t_oid = tup->t_data->t_oid;
+        if (relation->rd_rel->relhasoids)
+            xlhdr.t_oid = HeapTupleGetOid(tup);
+        else
+            xlhdr.t_oid = InvalidOid;
         xlhdr.t_natts = tup->t_data->t_natts;
         xlhdr.t_hoff = tup->t_data->t_hoff;
         xlhdr.mask = tup->t_data->t_infomask;
@@ -1206,7 +1209,10 @@
      */
     CacheInvalidateHeapTuple(relation, tup);

-    return tup->t_data->t_oid;
+    if (!relation->rd_rel->relhasoids)
+        return InvalidOid;
+
+    return HeapTupleGetOid(tup);
 }

 /*
@@ -1499,7 +1505,8 @@
     }

     /* Fill in OID and transaction status data for newtup */
-    newtup->t_data->t_oid = oldtup.t_data->t_oid;
+    if (relation->rd_rel->relhasoids)
+        HeapTupleSetOid(newtup, HeapTupleGetOid(&oldtup));
     newtup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
     newtup->t_data->t_infomask |= (HEAP_XMAX_INVALID | HEAP_UPDATED);
     HeapTupleHeaderSetXmin(newtup->t_data, GetCurrentTransactionId());
@@ -1972,7 +1979,10 @@
     rdata[1].len = 0;
     rdata[1].next = &(rdata[2]);

-    xlhdr.hdr.t_oid = newtup->t_data->t_oid;
+    if (reln->rd_rel->relhasoids)
+        xlhdr.hdr.t_oid = HeapTupleGetOid(newtup);
+    else
+        xlhdr.hdr.t_oid = InvalidOid;
     xlhdr.hdr.t_natts = newtup->t_data->t_natts;
     xlhdr.hdr.t_hoff = newtup->t_data->t_hoff;
     xlhdr.hdr.mask = newtup->t_data->t_infomask;
@@ -2198,7 +2208,6 @@
                newlen);
         newlen += offsetof(HeapTupleHeaderData, t_bits);
         htup = &tbuf.hdr;
-        htup->t_oid = xlhdr.t_oid;
         htup->t_natts = xlhdr.t_natts;
         htup->t_hoff = xlhdr.t_hoff;
         htup->t_infomask = HEAP_XMAX_INVALID | xlhdr.mask;
@@ -2206,6 +2215,8 @@
         HeapTupleHeaderSetCmin(htup, FirstCommandId);
         HeapTupleHeaderSetXmaxInvalid(htup);
         HeapTupleHeaderSetCmax(htup, FirstCommandId);
+        if (reln->rd_rel->relhasoids)
+            HeapTupleHeaderSetOid(htup, xlhdr.t_oid);

         offnum = PageAddItem(page, (Item) htup, newlen, offnum,
                              LP_USED | OverwritePageMode);
@@ -2367,9 +2378,10 @@
                newlen);
         newlen += offsetof(HeapTupleHeaderData, t_bits);
         htup = &tbuf.hdr;
-        htup->t_oid = xlhdr.t_oid;
         htup->t_natts = xlhdr.t_natts;
         htup->t_hoff = xlhdr.t_hoff;
+        if (reln->rd_rel->relhasoids)
+            HeapTupleHeaderSetOid(htup, xlhdr.t_oid);
         if (move)
         {
             TransactionId xmax;
diff -ru ../base/src/backend/bootstrap/bootstrap.c src/backend/bootstrap/bootstrap.c
--- ../base/src/backend/bootstrap/bootstrap.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/bootstrap/bootstrap.c    2002-07-01 00:41:49.000000000 +0200
@@ -495,7 +495,8 @@
         app = Typ;
         while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
         {
-            (*app)->am_oid = tup->t_data->t_oid;
+            AssertRelationHasOids(rel);
+            (*app)->am_oid = HeapTupleGetOid(tup);
             memcpy((char *) &(*app)->am_typ,
                    (char *) GETSTRUCT(tup),
                    sizeof((*app)->am_typ));
@@ -679,7 +680,10 @@
     pfree(tupDesc);                /* just free's tupDesc, not the attrtypes */

     if (objectid != (Oid) 0)
-        tuple->t_data->t_oid = objectid;
+    {
+        AssertRelationHasOids(boot_reldesc);
+        HeapTupleSetOid(tuple, objectid);
+    }
     simple_heap_insert(boot_reldesc, tuple);
     heap_freetuple(tuple);
     elog(DEBUG3, "row inserted");
@@ -871,7 +875,8 @@
         app = Typ;
         while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
         {
-            (*app)->am_oid = tup->t_data->t_oid;
+            AssertRelationHasOids(rel);
+            (*app)->am_oid = HeapTupleGetOid(tup);
             memmove((char *) &(*app++)->am_typ,
                     (char *) GETSTRUCT(tup),
                     sizeof((*app)->am_typ));
diff -ru ../base/src/backend/catalog/aclchk.c src/backend/catalog/aclchk.c
--- ../base/src/backend/catalog/aclchk.c    2002-05-21 11:54:12.000000000 +0200
+++ src/backend/catalog/aclchk.c    2002-06-30 20:44:51.000000000 +0200
@@ -591,7 +591,8 @@
             elog(ERROR, "namespace \"%s\" not found", nspname);
         pg_namespace_tuple = (Form_pg_namespace) GETSTRUCT(tuple);

-        if (!pg_namespace_ownercheck(tuple->t_data->t_oid, GetUserId()))
+        AssertRelationHasOids(relation);
+        if (!pg_namespace_ownercheck(HeapTupleGetOid(tuple), GetUserId()))
             aclcheck_error(ACLCHECK_NOT_OWNER, nspname);

         /*
diff -ru ../base/src/backend/catalog/heap.c src/backend/catalog/heap.c
--- ../base/src/backend/catalog/heap.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/catalog/heap.c    2002-06-30 20:46:23.000000000 +0200
@@ -583,7 +583,7 @@
                          (void *) new_rel_reltup);

     /* force tuple to have the desired OID */
-    tup->t_data->t_oid = new_rel_oid;
+    HeapTupleSetOid(tup, new_rel_oid);

     /*
      * finally insert the new tuple and free it.
@@ -1109,7 +1109,8 @@
      * the type of the relation we are deleteing then we have to disallow
      * the deletion.  should talk to stonebraker about this.  -cim 6/19/90
      */
-    typoid = tup->t_data->t_oid;
+    AssertRelationHasOids(pg_type_desc);
+    typoid = HeapTupleGetOid(tup);

     pg_attribute_desc = heap_openr(AttributeRelationName, RowExclusiveLock);

diff -ru ../base/src/backend/catalog/index.c src/backend/catalog/index.c
--- ../base/src/backend/catalog/index.c    2002-06-17 10:11:31.000000000 +0200
+++ src/backend/catalog/index.c    2002-06-30 00:53:45.000000000 +0200
@@ -326,7 +326,7 @@
      * the new tuple must have the oid already chosen for the index.
      * sure would be embarrassing to do this sort of thing in polite company.
      */
-    tuple->t_data->t_oid = RelationGetRelid(indexRelation);
+    HeapTupleSetOid(tuple, RelationGetRelid(indexRelation));
     simple_heap_insert(pg_class, tuple);

     /*
diff -ru ../base/src/backend/catalog/namespace.c src/backend/catalog/namespace.c
--- ../base/src/backend/catalog/namespace.c    2002-05-21 11:54:12.000000000 +0200
+++ src/backend/catalog/namespace.c    2002-06-30 00:57:56.000000000 +0200
@@ -584,7 +584,7 @@
                         continue; /* keep previous result */
                     /* replace previous result */
                     prevResult->pathpos = pathpos;
-                    prevResult->oid = proctup->t_data->t_oid;
+                    prevResult->oid = HeapTupleGetOid(proctup);
                     continue;    /* args are same, of course */
                 }
             }
@@ -597,7 +597,7 @@
             palloc(sizeof(struct _FuncCandidateList) - sizeof(Oid)
                    + nargs * sizeof(Oid));
         newResult->pathpos = pathpos;
-        newResult->oid = proctup->t_data->t_oid;
+        newResult->oid = HeapTupleGetOid(proctup);
         newResult->nargs = nargs;
         memcpy(newResult->args, procform->proargtypes, nargs * sizeof(Oid));

@@ -831,7 +831,7 @@
                         continue; /* keep previous result */
                     /* replace previous result */
                     prevResult->pathpos = pathpos;
-                    prevResult->oid = opertup->t_data->t_oid;
+                    prevResult->oid = HeapTupleGetOid(opertup);
                     continue;    /* args are same, of course */
                 }
             }
@@ -843,7 +843,7 @@
         newResult = (FuncCandidateList)
             palloc(sizeof(struct _FuncCandidateList) + sizeof(Oid));
         newResult->pathpos = pathpos;
-        newResult->oid = opertup->t_data->t_oid;
+        newResult->oid = HeapTupleGetOid(opertup);
         newResult->nargs = 2;
         newResult->args[0] = operform->oprleft;
         newResult->args[1] = operform->oprright;
@@ -1007,7 +1007,7 @@
                 /* replace previous result */
                 prevResult->opcname_tmp = NameStr(opcform->opcname);
                 prevResult->pathpos = pathpos;
-                prevResult->oid = opctup->t_data->t_oid;
+                prevResult->oid = HeapTupleGetOid(opctup);
                 prevResult->opcintype = opcform->opcintype;
                 prevResult->opcdefault = opcform->opcdefault;
                 prevResult->opckeytype = opcform->opckeytype;
@@ -1022,7 +1022,7 @@
             palloc(sizeof(struct _OpclassCandidateList));
         newResult->opcname_tmp = NameStr(opcform->opcname);
         newResult->pathpos = pathpos;
-        newResult->oid = opctup->t_data->t_oid;
+        newResult->oid = HeapTupleGetOid(opctup);
         newResult->opcintype = opcform->opcintype;
         newResult->opcdefault = opcform->opcdefault;
         newResult->opckeytype = opcform->opckeytype;
@@ -1597,7 +1597,7 @@
             case RELKIND_RELATION:
             case RELKIND_SEQUENCE:
             case RELKIND_VIEW:
-                tempRelList = lconsi(tuple->t_data->t_oid, tempRelList);
+                tempRelList = lconsi(HeapTupleGetOid(tuple), tempRelList);
                 break;
             default:
                 break;
diff -ru ../base/src/backend/catalog/pg_operator.c src/backend/catalog/pg_operator.c
--- ../base/src/backend/catalog/pg_operator.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/catalog/pg_operator.c    2002-06-30 00:58:58.000000000 +0200
@@ -141,7 +141,7 @@
     {
         RegProcedure oprcode = ((Form_pg_operator) GETSTRUCT(tup))->oprcode;

-        operatorObjectId = tup->t_data->t_oid;
+        operatorObjectId = HeapTupleGetOid(tup);
         *defined = RegProcedureIsValid(oprcode);
         ReleaseSysCache(tup);
     }
diff -ru ../base/src/backend/catalog/pg_proc.c src/backend/catalog/pg_proc.c
--- ../base/src/backend/catalog/pg_proc.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/catalog/pg_proc.c    2002-06-30 20:47:27.000000000 +0200
@@ -249,7 +249,8 @@
         CatalogCloseIndices(Num_pg_proc_indices, idescs);
     }

-    retval = tup->t_data->t_oid;
+    AssertRelationHasOids(rel);
+    retval = HeapTupleGetOid(tup);
     heap_freetuple(tup);

     heap_close(rel, RowExclusiveLock);
diff -ru ../base/src/backend/catalog/pg_type.c src/backend/catalog/pg_type.c
--- ../base/src/backend/catalog/pg_type.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/catalog/pg_type.c    2002-06-30 20:48:06.000000000 +0200
@@ -272,7 +272,8 @@

         simple_heap_update(pg_type_desc, &tup->t_self, tup);

-        typeObjectId = tup->t_data->t_oid;
+        AssertRelationHasOids(pg_type_desc);
+        typeObjectId = HeapTupleGetOid(tup);
     }
     else
     {
@@ -283,7 +284,8 @@
                              nulls);

         /* preassign tuple Oid, if one was given */
-        tup->t_data->t_oid = assignedTypeOid;
+        AssertRelationHasOids(pg_type_desc);
+        HeapTupleSetOid(tup, assignedTypeOid);

         typeObjectId = simple_heap_insert(pg_type_desc, tup);
     }
diff -ru ../base/src/backend/commands/comment.c src/backend/commands/comment.c
--- ../base/src/backend/commands/comment.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/commands/comment.c    2002-06-30 20:49:28.000000000 +0200
@@ -424,7 +424,8 @@

     if (!HeapTupleIsValid(dbtuple))
         elog(ERROR, "database \"%s\" does not exist", database);
-    oid = dbtuple->t_data->t_oid;
+    AssertRelationHasOids(pg_database);
+    oid = HeapTupleGetOid(dbtuple);

     /* Allow if the user matches the database dba or is a superuser */

@@ -470,7 +471,8 @@
         elog(ERROR, "CommentSchema: Schema \"%s\" could not be found",
              namespace);

-    oid = tp->t_data->t_oid;
+    /* no Relation here to Assert(...->relhasoids); */
+    oid = HeapTupleGetOid(tp);

     /* Check object security */
     if (!pg_namespace_ownercheck(oid, GetUserId()))
@@ -541,7 +543,8 @@
         if (HeapTupleIsValid(tuple))
         {
             reloid = ((Form_pg_rewrite) GETSTRUCT(tuple))->ev_class;
-            ruleoid = tuple->t_data->t_oid;
+            AssertRelationHasOids(RewriteRelation);
+            ruleoid = HeapTupleGetOid(tuple);
         }
         else
         {
@@ -581,7 +584,8 @@
         if (!HeapTupleIsValid(tuple))
             elog(ERROR, "rule \"%s\" does not exist", rulename);
         Assert(reloid == ((Form_pg_rewrite) GETSTRUCT(tuple))->ev_class);
-        ruleoid = tuple->t_data->t_oid;
+        AssertRelationHasOids(relation);
+        ruleoid = HeapTupleGetOid(tuple);
         ReleaseSysCache(tuple);
     }

@@ -794,7 +798,8 @@
         elog(ERROR, "trigger \"%s\" for relation \"%s\" does not exist",
              trigname, RelationGetRelationName(relation));

-    oid = triggertuple->t_data->t_oid;
+    AssertRelationHasOids(pg_trigger);
+    oid = HeapTupleGetOid(triggertuple);

     systable_endscan(scan);

diff -ru ../base/src/backend/commands/copy.c src/backend/commands/copy.c
--- ../base/src/backend/commands/copy.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/commands/copy.c    2002-06-30 20:50:17.000000000 +0200
@@ -514,9 +514,13 @@
             /* Send OID if wanted --- note fld_count doesn't include it */
             if (oids)
             {
+                Oid oid;
+
+                AssertRelationHasOids(rel);
+                oid = HeapTupleGetOid(tuple);
                 fld_size = sizeof(Oid);
                 CopySendData(&fld_size, sizeof(int16), fp);
-                CopySendData(&tuple->t_data->t_oid, sizeof(Oid), fp);
+                CopySendData(&oid, sizeof(Oid), fp);
             }
         }
         else
@@ -524,8 +528,9 @@
             /* Text format has no per-tuple header, but send OID if wanted */
             if (oids)
             {
+                AssertRelationHasOids(rel);
                 string = DatumGetCString(DirectFunctionCall1(oidout,
-                                ObjectIdGetDatum(tuple->t_data->t_oid)));
+                                ObjectIdGetDatum(HeapTupleGetOid(tuple))));
                 CopySendString(string, fp);
                 pfree(string);
                 need_delim = true;
@@ -916,7 +921,7 @@
         tuple = heap_formtuple(tupDesc, values, nulls);

         if (oids && file_has_oids)
-            tuple->t_data->t_oid = loaded_oid;
+            HeapTupleSetOid(tuple, loaded_oid);

         skip_tuple = false;

diff -ru ../base/src/backend/commands/dbcommands.c src/backend/commands/dbcommands.c
--- ../base/src/backend/commands/dbcommands.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/commands/dbcommands.c    2002-06-30 20:51:05.000000000 +0200
@@ -290,7 +290,8 @@

     tuple = heap_formtuple(pg_database_dsc, new_record, new_record_nulls);

-    tuple->t_data->t_oid = dboid;        /* override heap_insert's OID
+    AssertRelationHasOids(pg_database_rel);
+    HeapTupleSetOid(tuple, dboid);        /* override heap_insert's OID
                                          * selection */

     simple_heap_insert(pg_database_rel, tuple);
@@ -560,7 +561,10 @@

         /* oid of the database */
         if (dbIdP)
-            *dbIdP = tuple->t_data->t_oid;
+        {
+            AssertRelationHasOids(relation);
+            *dbIdP = HeapTupleGetOid(tuple);
+        }
         /* sysid of the owner */
         if (ownerIdP)
             *ownerIdP = dbform->datdba;
diff -ru ../base/src/backend/commands/functioncmds.c src/backend/commands/functioncmds.c
--- ../base/src/backend/commands/functioncmds.c    2002-05-27 16:29:37.000000000 +0200
+++ src/backend/commands/functioncmds.c    2002-06-30 17:26:31.000000000 +0200
@@ -439,7 +439,8 @@
     if (!HeapTupleIsValid(languageTuple))
         elog(ERROR, "language \"%s\" does not exist", languageName);

-    languageOid = languageTuple->t_data->t_oid;
+    /* no Relation here to Assert(...->relhasoids) */
+    languageOid = HeapTupleGetOid(languageTuple);
     languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);

     if (languageStruct->lanpltrusted)
diff -ru ../base/src/backend/commands/indexcmds.c src/backend/commands/indexcmds.c
--- ../base/src/backend/commands/indexcmds.c    2002-05-21 11:54:12.000000000 +0200
+++ src/backend/commands/indexcmds.c    2002-06-30 20:52:10.000000000 +0200
@@ -139,7 +139,8 @@
     if (!HeapTupleIsValid(tuple))
         elog(ERROR, "DefineIndex: access method \"%s\" not found",
              accessMethodName);
-    accessMethodId = tuple->t_data->t_oid;
+    /* AssertRelationHasOids(rel);  rel is already closed */
+    accessMethodId = HeapTupleGetOid(tuple);
     accessMethodForm = (Form_pg_am) GETSTRUCT(tuple);

     if (unique && !accessMethodForm->amcanunique)
@@ -494,7 +495,8 @@
      * Verify that the index operator class accepts this
      * datatype.  Note we will accept binary compatibility.
      */
-    opClassId = tuple->t_data->t_oid;
+    /* no Relation here to AssertRelationHasOids(relation); */
+    opClassId = HeapTupleGetOid(tuple);
     opInputType = ((Form_pg_opclass) GETSTRUCT(tuple))->opcintype;

     if (!IsBinaryCompatible(attrType, opInputType))
@@ -759,7 +761,8 @@
                 relids = repalloc(relids, sizeof(Oid) * relalc);
             }
             MemoryContextSwitchTo(old);
-            relids[relcnt] = tuple->t_data->t_oid;
+            AssertRelationHasOids(relationRelation);
+            relids[relcnt] = HeapTupleGetOid(tuple);
             relcnt++;
         }
     }
diff -ru ../base/src/backend/commands/tablecmds.c src/backend/commands/tablecmds.c
--- ../base/src/backend/commands/tablecmds.c    2002-06-17 20:47:27.000000000 +0200
+++ src/backend/commands/tablecmds.c    2002-06-30 01:17:28.000000000 +0200
@@ -1656,7 +1656,7 @@

     attribute->attrelid = myrelid;
     namestrcpy(&(attribute->attname), colDef->colname);
-    attribute->atttypid = typeTuple->t_data->t_oid;
+    attribute->atttypid = HeapTupleGetOid(typeTuple);
     attribute->attstattarget = DEFAULT_ATTSTATTARGET;
     attribute->attlen = tform->typlen;
     attribute->attcacheoff = -1;
diff -ru ../base/src/backend/commands/trigger.c src/backend/commands/trigger.c
--- ../base/src/backend/commands/trigger.c    2002-05-27 16:29:38.000000000 +0200
+++ src/backend/commands/trigger.c    2002-06-30 20:53:00.000000000 +0200
@@ -364,7 +364,8 @@
         if (namestrcmp(&(pg_trigger->tgname), trigname) == 0)
         {
             /* Delete any comments associated with this trigger */
-            DeleteComments(tuple->t_data->t_oid, RelationGetRelid(tgrel));
+            AssertRelationHasOids(tgrel);
+            DeleteComments(HeapTupleGetOid(tuple), RelationGetRelid(tgrel));

             simple_heap_delete(tgrel, &tuple->t_self);
             found++;
@@ -430,7 +431,8 @@
     while (HeapTupleIsValid(tup = systable_getnext(tgscan)))
     {
         /* Delete any comments associated with this trigger */
-        DeleteComments(tup->t_data->t_oid, RelationGetRelid(tgrel));
+        AssertRelationHasOids(tgrel);
+        DeleteComments(HeapTupleGetOid(tup), RelationGetRelid(tgrel));

         simple_heap_delete(tgrel, &tup->t_self);

@@ -667,7 +669,8 @@
                  RelationGetRelationName(relation));
         build = &(triggers[found]);

-        build->tgoid = htup->t_data->t_oid;
+        AssertRelationHasOids(tgrel);
+        build->tgoid = HeapTupleGetOid(htup);
         build->tgname = MemoryContextStrdup(CacheMemoryContext,
                              DatumGetCString(DirectFunctionCall1(nameout,
                                     NameGetDatum(&pg_trigger->tgname))));
@@ -1928,7 +1931,8 @@
                 elog(ERROR, "Constraint '%s' is not deferrable",
                      cname);

-            constr_oid = htup->t_data->t_oid;
+            AssertRelationHasOids(tgrel);
+            constr_oid = HeapTupleGetOid(htup);
             loid = lappendi(loid, constr_oid);
             found = true;
         }
diff -ru ../base/src/backend/commands/typecmds.c src/backend/commands/typecmds.c
--- ../base/src/backend/commands/typecmds.c    2002-05-03 02:32:16.000000000 +0200
+++ src/backend/commands/typecmds.c    2002-06-30 01:22:36.000000000 +0200
@@ -475,7 +475,7 @@
                  * Note: Name is strictly for error message
                  */
                 expr = cookDefault(pstate, colDef->raw_expr,
-                                   typeTup->t_data->t_oid,
+                                   HeapTupleGetOid(typeTup),
                                    stmt->typename->typmod,
                                    domainName);
                 /*
@@ -552,7 +552,7 @@
                receiveProcedure,    /* receive procedure */
                sendProcedure,        /* send procedure */
                basetypelem,            /* element type ID */
-               typeTup->t_data->t_oid,    /* base type ID */
+               HeapTupleGetOid(typeTup),    /* base type ID */
                defaultValue,        /* default type value (text) */
                defaultValueBin,        /* default type value (binary) */
                byValue,                /* passed by value */
diff -ru ../base/src/backend/commands/vacuum.c src/backend/commands/vacuum.c
--- ../base/src/backend/commands/vacuum.c    2002-06-17 10:11:31.000000000 +0200
+++ src/backend/commands/vacuum.c    2002-06-30 20:55:09.000000000 +0200
@@ -392,7 +392,8 @@
         {
             /* Make a relation list entry for this guy */
             oldcontext = MemoryContextSwitchTo(vac_context);
-            vrl = lappendi(vrl, tuple->t_data->t_oid);
+            AssertRelationHasOids(pgclass);
+            vrl = lappendi(vrl, HeapTupleGetOid(tuple));
             MemoryContextSwitchTo(oldcontext);
         }

@@ -1172,8 +1173,8 @@
             /*
              * Other checks...
              */
-            if (!OidIsValid(tuple.t_data->t_oid) &&
-                onerel->rd_rel->relhasoids)
+            if (onerel->rd_rel->relhasoids &&
+                !OidIsValid(HeapTupleGetOid(&tuple)))
                 elog(WARNING, "Rel %s: TID %u/%u: OID IS INVALID. TUPGONE %d.",
                      relname, blkno, offnum, (int) tupgone);

diff -ru ../base/src/backend/commands/vacuumlazy.c src/backend/commands/vacuumlazy.c
--- ../base/src/backend/commands/vacuumlazy.c    2002-06-17 10:11:31.000000000 +0200
+++ src/backend/commands/vacuumlazy.c    2002-06-30 01:29:17.000000000 +0200
@@ -368,8 +368,8 @@
             /*
              * Other checks...
              */
-            if (!OidIsValid(tuple.t_data->t_oid) &&
-                onerel->rd_rel->relhasoids)
+            if (onerel->rd_rel->relhasoids &&
+                !OidIsValid(HeapTupleGetOid(&tuple)))
                 elog(WARNING, "Rel %s: TID %u/%u: OID IS INVALID. TUPGONE %d.",
                      relname, blkno, offnum, (int) tupgone);

diff -ru ../base/src/backend/executor/spi.c src/backend/executor/spi.c
--- ../base/src/backend/executor/spi.c    2002-05-27 16:29:38.000000000 +0200
+++ src/backend/executor/spi.c    2002-06-30 03:06:56.000000000 +0200
@@ -435,11 +435,18 @@
     {
         mtuple = heap_formtuple(rel->rd_att, v, n);
         infomask = mtuple->t_data->t_infomask;
-        memmove(&(mtuple->t_data->t_oid), &(tuple->t_data->t_oid),
-                ((char *) &(tuple->t_data->t_hoff) -
-                 (char *) &(tuple->t_data->t_oid)));
+        /*
+         * PG72FORMAT:
+         * copy t_cmin, t_cmax, t_xmin, t_xmax, t_ctid, t_natts, t_infomask
+         * PG73FORMAT:
+         * copy t_xmin, t_cid, t_xmax, t_ctid, t_natts, t_infomask
+         */
+        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;
+        if (rel->rd_rel->relhasoids)
+            HeapTupleSetOid(mtuple, HeapTupleGetOid(tuple));
     }
     else
     {
diff -ru ../base/src/backend/optimizer/util/clauses.c src/backend/optimizer/util/clauses.c
--- ../base/src/backend/optimizer/util/clauses.c    2002-05-21 11:54:15.000000000 +0200
+++ src/backend/optimizer/util/clauses.c    2002-07-01 01:55:32.000000000 +0200
@@ -1066,7 +1066,7 @@

     commuTup = (Form_pg_operator) GETSTRUCT(optup);

-    commu = makeOper(optup->t_data->t_oid,
+    commu = makeOper(HeapTupleGetOid(optup),
                      commuTup->oprcode,
                      commuTup->oprresult,
                      ((Oper *) clause->oper)->opretset);
diff -ru ../base/src/backend/parser/parse_coerce.c src/backend/parser/parse_coerce.c
--- ../base/src/backend/parser/parse_coerce.c    2002-05-21 11:54:16.000000000 +0200
+++ src/backend/parser/parse_coerce.c    2002-06-30 17:47:47.000000000 +0200
@@ -770,7 +770,8 @@
             if (isExplicit || pform->proimplicit)
             {
                 /* Okay to use it */
-                funcid = ftup->t_data->t_oid;
+                /* Assert(ftup has oid) */
+                funcid = HeapTupleGetOid(ftup);
             }
         }
         ReleaseSysCache(ftup);
diff -ru ../base/src/backend/parser/parse_oper.c src/backend/parser/parse_oper.c
--- ../base/src/backend/parser/parse_oper.c    2002-05-01 21:26:07.000000000 +0200
+++ src/backend/parser/parse_oper.c    2002-06-30 17:49:03.000000000 +0200
@@ -150,7 +150,8 @@
 Oid
 oprid(Operator op)
 {
-    return op->t_data->t_oid;
+    /* Assert(op has oid) */
+    return HeapTupleGetOid(op);
 }

 /* given operator tuple, return the underlying function's OID */
diff -ru ../base/src/backend/parser/parse_type.c src/backend/parser/parse_type.c
--- ../base/src/backend/parser/parse_type.c    2002-05-21 11:54:17.000000000 +0200
+++ src/backend/parser/parse_type.c    2002-06-30 17:50:12.000000000 +0200
@@ -283,7 +283,8 @@
 {
     if (tp == NULL)
         elog(ERROR, "typeTypeId() called with NULL type struct");
-    return tp->t_data->t_oid;
+    /* Assert(tp has oid); */
+    return HeapTupleGetOid(tp);
 }

 /* given type (as type struct), return the length of type */
diff -ru ../base/src/backend/postmaster/pgstat.c src/backend/postmaster/pgstat.c
--- ../base/src/backend/postmaster/pgstat.c    2002-05-21 11:54:17.000000000 +0200
+++ src/backend/postmaster/pgstat.c    2002-06-30 21:03:27.000000000 +0200
@@ -628,7 +628,8 @@
             dbidlist = (Oid *) repalloc((char *) dbidlist,
                                         sizeof(Oid) * dbidalloc);
         }
-        dbidlist[dbidused++] = dbtup->t_data->t_oid;
+        AssertRelationHasOids(dbrel);
+        dbidlist[dbidused++] = HeapTupleGetOid(dbtup);
     }
     heap_endscan(dbscan);
     heap_close(dbrel, AccessShareLock);
diff -ru ../base/src/backend/rewrite/rewriteRemove.c src/backend/rewrite/rewriteRemove.c
--- ../base/src/backend/rewrite/rewriteRemove.c    2002-04-27 05:45:03.000000000 +0200
+++ src/backend/rewrite/rewriteRemove.c    2002-06-30 21:04:13.000000000 +0200
@@ -67,7 +67,8 @@
      * Save the OID of the rule (i.e. the tuple's OID) and the event
      * relation's OID
      */
-    ruleId = tuple->t_data->t_oid;
+    AssertRelationHasOids(RewriteRelation);
+    ruleId = HeapTupleGetOid(tuple);
     eventRelationOid = ((Form_pg_rewrite) GETSTRUCT(tuple))->ev_class;
     Assert(eventRelationOid == owningRel);

@@ -157,7 +158,8 @@
     while (HeapTupleIsValid(tuple = systable_getnext(scanDesc)))
     {
         /* Delete any comments associated with this rule */
-        DeleteComments(tuple->t_data->t_oid, RelationGetRelid(RewriteRelation));
+        AssertRelationHasOids(RewriteRelation);
+        DeleteComments(HeapTupleGetOid(tuple), RelationGetRelid(RewriteRelation));

         simple_heap_delete(RewriteRelation, &tuple->t_self);
     }
diff -ru ../base/src/backend/utils/adt/regproc.c src/backend/utils/adt/regproc.c
--- ../base/src/backend/utils/adt/regproc.c    2002-05-12 01:27:14.000000000 +0200
+++ src/backend/utils/adt/regproc.c    2002-06-30 21:05:13.000000000 +0200
@@ -112,7 +112,8 @@

         while (HeapTupleIsValid(tuple = systable_getnext(sysscan)))
         {
-            result = (RegProcedure) tuple->t_data->t_oid;
+            AssertRelationHasOids(hdesc);
+            result = (RegProcedure) HeapTupleGetOid(tuple);
             if (++matches > 1)
                 break;
         }
@@ -416,7 +417,8 @@

         while (HeapTupleIsValid(tuple = systable_getnext(sysscan)))
         {
-            result = tuple->t_data->t_oid;
+            AssertRelationHasOids(hdesc);
+            result = HeapTupleGetOid(tuple);
             if (++matches > 1)
                 break;
         }
@@ -733,7 +735,10 @@
                                      SnapshotNow, 1, skey);

         if (HeapTupleIsValid(tuple = systable_getnext(sysscan)))
-            result = tuple->t_data->t_oid;
+        {
+            AssertRelationHasOids(hdesc);
+            result = HeapTupleGetOid(tuple);
+        }
         else
             elog(ERROR, "No class with name %s", class_name_or_oid);

@@ -886,7 +891,10 @@
                                      SnapshotNow, 1, skey);

         if (HeapTupleIsValid(tuple = systable_getnext(sysscan)))
-            result = tuple->t_data->t_oid;
+        {
+            AssertRelationHasOids(hdesc);
+            result = HeapTupleGetOid(tuple);
+        }
         else
             elog(ERROR, "No type with name %s", typ_name_or_oid);

diff -ru ../base/src/backend/utils/adt/sets.c src/backend/utils/adt/sets.c
--- ../base/src/backend/utils/adt/sets.c    2002-05-27 16:29:39.000000000 +0200
+++ src/backend/utils/adt/sets.c    2002-06-30 21:05:49.000000000 +0200
@@ -120,7 +120,8 @@

         simple_heap_update(procrel, &newtup->t_self, newtup);

-        setoid = newtup->t_data->t_oid;
+        AssertRelationHasOids(procrel);
+        setoid = HeapTupleGetOid(newtup);

         if (RelationGetForm(procrel)->relhasindex)
         {
diff -ru ../base/src/backend/utils/cache/catcache.c src/backend/utils/cache/catcache.c
--- ../base/src/backend/utils/cache/catcache.c    2002-04-25 04:56:56.000000000 +0200
+++ src/backend/utils/cache/catcache.c    2002-06-30 01:50:50.000000000 +0200
@@ -218,7 +218,7 @@
         case 4:
             cur_skey[3].sk_argument =
                 (cache->cc_key[3] == ObjectIdAttributeNumber)
-                ? ObjectIdGetDatum(tuple->t_data->t_oid)
+                ? ObjectIdGetDatum(HeapTupleGetOid(tuple))
                 : fastgetattr(tuple,
                               cache->cc_key[3],
                               cache->cc_tupdesc,
@@ -228,7 +228,7 @@
         case 3:
             cur_skey[2].sk_argument =
                 (cache->cc_key[2] == ObjectIdAttributeNumber)
-                ? ObjectIdGetDatum(tuple->t_data->t_oid)
+                ? ObjectIdGetDatum(HeapTupleGetOid(tuple))
                 : fastgetattr(tuple,
                               cache->cc_key[2],
                               cache->cc_tupdesc,
@@ -238,7 +238,7 @@
         case 2:
             cur_skey[1].sk_argument =
                 (cache->cc_key[1] == ObjectIdAttributeNumber)
-                ? ObjectIdGetDatum(tuple->t_data->t_oid)
+                ? ObjectIdGetDatum(HeapTupleGetOid(tuple))
                 : fastgetattr(tuple,
                               cache->cc_key[1],
                               cache->cc_tupdesc,
@@ -248,7 +248,7 @@
         case 1:
             cur_skey[0].sk_argument =
                 (cache->cc_key[0] == ObjectIdAttributeNumber)
-                ? ObjectIdGetDatum(tuple->t_data->t_oid)
+                ? ObjectIdGetDatum(HeapTupleGetOid(tuple))
                 : fastgetattr(tuple,
                               cache->cc_key[0],
                               cache->cc_tupdesc,
@@ -572,7 +572,7 @@
             if (isCommit)
                 elog(WARNING, "Cache reference leak: cache %s (%d), tuple %u has count %d",
                      ct->my_cache->cc_relname, ct->my_cache->id,
-                     ct->tuple.t_data->t_oid,
+                     HeapTupleGetOid(&ct->tuple),
                      ct->refcount);
             ct->refcount = 0;
         }
@@ -717,7 +717,7 @@
                     continue;

                 if (cache->cc_reloidattr == ObjectIdAttributeNumber)
-                    tupRelid = ct->tuple.t_data->t_oid;
+                    tupRelid = HeapTupleGetOid(&ct->tuple);
                 else
                 {
                     bool        isNull;
@@ -1685,7 +1685,7 @@
     }

     ntp = heap_formtuple(tupDesc, values, nulls);
-    ntp->t_data->t_oid = tupOid;
+    HeapTupleSetOid(ntp, tupOid);

     pfree(values);
     pfree(nulls);
diff -ru ../base/src/backend/utils/cache/inval.c src/backend/utils/cache/inval.c
--- ../base/src/backend/utils/cache/inval.c    2002-04-30 00:14:34.000000000 +0200
+++ src/backend/utils/cache/inval.c    2002-07-01 00:47:21.000000000 +0200
@@ -525,7 +525,10 @@
     tupleRelId = RelationGetRelid(relation);

     if (tupleRelId == RelOid_pg_class)
-        relationId = tuple->t_data->t_oid;
+    {
+        AssertRelationHasOids(relation);
+        relationId = HeapTupleGetOid(tuple);
+    }
     else if (tupleRelId == RelOid_pg_attribute)
         relationId = ((Form_pg_attribute) GETSTRUCT(tuple))->attrelid;
     else
diff -ru ../base/src/backend/utils/cache/relcache.c src/backend/utils/cache/relcache.c
--- ../base/src/backend/utils/cache/relcache.c    2002-05-27 16:29:39.000000000 +0200
+++ src/backend/utils/cache/relcache.c    2002-06-30 21:06:53.000000000 +0200
@@ -701,7 +701,8 @@
         rule = (RewriteRule *) MemoryContextAlloc(rulescxt,
                                                   sizeof(RewriteRule));

-        rule->ruleId = rewrite_tuple->t_data->t_oid;
+        AssertRelationHasOids(rewrite_desc);
+        rule->ruleId = HeapTupleGetOid(rewrite_tuple);

         rule->event = rewrite_form->ev_type - '0';
         rule->attrno = rewrite_form->ev_attr;
@@ -839,7 +840,8 @@
     /*
      * get information from the pg_class_tuple
      */
-    relid = pg_class_tuple->t_data->t_oid;
+    /* Assert(pg_class_tuple has oid); */
+    relid = HeapTupleGetOid(pg_class_tuple);
     relp = (Form_pg_class) GETSTRUCT(pg_class_tuple);

     /*
diff -ru ../base/src/backend/utils/cache/syscache.c src/backend/utils/cache/syscache.c
--- ../base/src/backend/utils/cache/syscache.c    2002-04-18 22:01:10.000000000 +0200
+++ src/backend/utils/cache/syscache.c    2002-06-30 18:07:38.000000000 +0200
@@ -574,7 +574,8 @@
     tuple = SearchSysCache(cacheId, key1, key2, key3, key4);
     if (!HeapTupleIsValid(tuple))
         return InvalidOid;
-    result = tuple->t_data->t_oid;
+    /* Assert(tuple has oid); */
+    result = HeapTupleGetOid(tuple);
     ReleaseSysCache(tuple);
     return result;
 }
diff -ru ../base/src/backend/utils/init/postinit.c src/backend/utils/init/postinit.c
--- ../base/src/backend/utils/init/postinit.c    2002-06-12 21:13:33.000000000 +0200
+++ src/backend/utils/init/postinit.c    2002-06-30 21:08:12.000000000 +0200
@@ -98,8 +98,9 @@
     pgdbscan = heap_beginscan(pgdbrel, SnapshotNow, 1, &key);

     tup = heap_getnext(pgdbscan, ForwardScanDirection);
+    AssertRelationHasOids(pgdbrel);
     if (!HeapTupleIsValid(tup) ||
-        tup->t_data->t_oid != MyDatabaseId)
+        HeapTupleGetOid(tup) != MyDatabaseId)
     {
         /* OOPS */
         heap_close(pgdbrel, AccessShareLock);
diff -ru ../base/src/backend/utils/misc/database.c src/backend/utils/misc/database.c
--- ../base/src/backend/utils/misc/database.c    2002-05-05 19:50:04.000000000 +0200
+++ src/backend/utils/misc/database.c    2002-06-30 18:11:38.000000000 +0200
@@ -220,7 +220,8 @@
             if (strcmp(name, NameStr(tup_db->datname)) == 0)
             {
                 /* Found it; extract the OID and the database path. */
-                *db_id = tup.t_data->t_oid;
+                /* Assert(&tup has oid); */
+                *db_id = HeapTupleGetOid(&tup);
                 pathlen = VARSIZE(&(tup_db->datpath)) - VARHDRSZ;
                 if (pathlen < 0)
                     pathlen = 0;                /* pure paranoia */
diff -ru ../base/src/include/access/htup.h src/include/access/htup.h
--- ../base/src/include/access/htup.h    2002-06-25 16:14:37.000000000 +0200
+++ src/include/access/htup.h    2002-06-30 11:37:16.000000000 +0200
@@ -136,6 +136,19 @@

 /* HeapTupleHeader accessor macros */

+#define HeapTupleHeaderGetOid(tup) \
+( \
+    AssertMacro((tup)->t_hoff >= offsetof(HeapTupleHeaderData, t_bits)), \
+    (tup)->t_oid \
+)
+
+#define HeapTupleHeaderSetOid(tup, oid) \
+( \
+    AssertMacro((tup)->t_hoff >= offsetof(HeapTupleHeaderData, t_bits)), \
+    (tup)->t_oid = (oid) \
+)
+
+
 #define HeapTupleHeaderGetXmin(tup) \
 ( \
     (tup)->t_xmin \
@@ -463,4 +476,10 @@
 #define HeapTupleHasExtended(tuple) \
         ((((HeapTuple)(tuple))->t_data->t_infomask & HEAP_HASEXTENDED) != 0)

+#define HeapTupleGetOid(tuple) \
+        HeapTupleHeaderGetOid(((HeapTuple)(tuple))->t_data)
+
+#define HeapTupleSetOid(tuple, oid) \
+        HeapTupleHeaderSetOid(((HeapTuple)(tuple))->t_data, (oid))
+
 #endif   /* HTUP_H */
diff -ru ../base/src/include/utils/rel.h src/include/utils/rel.h
--- ../base/src/include/utils/rel.h    2002-04-02 00:36:13.000000000 +0200
+++ src/include/utils/rel.h    2002-07-01 01:11:38.000000000 +0200
@@ -258,4 +258,11 @@
 #define RelationGetNamespace(relation) \
     ((relation)->rd_rel->relnamespace)

+#define AssertRelationHasOids(rel) \
+do { \
+    Assert(RelationIsValid(rel)); \
+    Assert(PointerIsValid((rel)->rd_rel)); \
+    Assert(rel->rd_rel->relhasoids); \
+} while (0)
+
 #endif   /* REL_H */
diff -ru ../base/src/pl/plpython/plpython.c src/pl/plpython/plpython.c
--- ../base/src/pl/plpython/plpython.c    2002-06-17 10:11:34.000000000 +0200
+++ src/pl/plpython/plpython.c    2002-06-30 02:00:44.000000000 +0200
@@ -2091,7 +2091,7 @@
                 Py_DECREF(optr);
                 optr = NULL;    /* this is important */

-                plan->types[i] = typeTup->t_data->t_oid;
+                plan->types[i] = HeapTupleGetOid(typeTup);
                 typeStruct = (Form_pg_type) GETSTRUCT(typeTup);
                 if (typeStruct->typrelid == InvalidOid)
                     PLy_output_datum_func(&plan->args[i], typeStruct);
diff -ru ../base/src/pl/tcl/pltcl.c src/pl/tcl/pltcl.c
--- ../base/src/pl/tcl/pltcl.c    2002-06-17 10:11:34.000000000 +0200
+++ src/pl/tcl/pltcl.c    2002-06-30 02:02:06.000000000 +0200
@@ -1755,7 +1755,7 @@
     {
         /* XXX should extend this to allow qualified type names */
         typeTup = typenameType(makeTypeName(args[i]));
-        qdesc->argtypes[i] = typeTup->t_data->t_oid;
+        qdesc->argtypes[i] = HeapTupleGetOid(typeTup);
         perm_fmgr_info(((Form_pg_type) GETSTRUCT(typeTup))->typinput,
                        &(qdesc->arginfuncs[i]));
         qdesc->argtypelems[i] = ((Form_pg_type) GETSTRUCT(typeTup))->typelem;
diff -ru ../base/contrib/dbsize/dbsize.c contrib/dbsize/dbsize.c
--- ../base/contrib/dbsize/dbsize.c    2002-05-21 11:54:08.000000000 +0200
+++ contrib/dbsize/dbsize.c    2002-06-30 21:10:47.000000000 +0200
@@ -65,7 +65,8 @@
     if (!HeapTupleIsValid(tuple))
         elog(ERROR, "database %s does not exist", NameStr(*dbname));

-    dbid = tuple->t_data->t_oid;
+    AssertRelationHasOids(relation);
+    dbid = HeapTupleGetOid(tuple);
     if (dbid == InvalidOid)
         elog(ERROR, "invalid database id");

diff -ru ../base/contrib/fulltextindex/fti.c contrib/fulltextindex/fti.c
--- ../base/contrib/fulltextindex/fti.c    2001-11-05 19:46:23.000000000 +0100
+++ contrib/fulltextindex/fti.c    2002-06-30 16:07:38.000000000 +0200
@@ -190,7 +190,7 @@
     tupdesc = rel->rd_att;        /* what the tuple looks like (?) */

     /* get oid of current tuple, needed by all, so place here */
-    oid = rettuple->t_data->t_oid;
+    oid = rel->rd_rel->relhasoids ? HeapTupleGetOid(rettuple) : InvalidOid;
     if (!OidIsValid(oid))
         elog(ERROR, "Full Text Indexing: Oid of current tuple is invalid");

diff -ru ../base/contrib/rserv/rserv.c contrib/rserv/rserv.c
--- ../base/contrib/rserv/rserv.c    2002-03-06 08:09:11.000000000 +0100
+++ contrib/rserv/rserv.c    2002-06-30 16:06:20.000000000 +0200
@@ -102,7 +102,9 @@

     if (keynum == ObjectIdAttributeNumber)
     {
-        sprintf(oidbuf, "%u", tuple->t_data->t_oid);
+        sprintf(oidbuf, "%u", rel->rd_rel->relhasoids
+                              ? HeapTupleGetOid(tuple)
+                              : InvalidOid);
         key = oidbuf;
     }
     else

Manfred Koizar <mkoi-pg@aon.at> writes:
> it indroduces two
> pre-conditions for accessing the oid:
> .  the relation has oids
> .  the heap tuple header is already formatted, i.e. t_hoff is valid

I think the latter test is unnecessary and potentially dangerous;
it could break code that tries to access OID in a
not-yet-completely-built header.  The test that insists t_hoff is valid
before one can *set* OID is even more likely to cause trouble.
I do not see the point of this in any case.

            regards, tom lane

On Mon, 01 Jul 2002 09:46:41 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:
>Manfred Koizar <mkoi-pg@aon.at> writes:
>> .  the heap tuple header is already formatted, i.e. t_hoff is valid
>
>I think the latter test is unnecessary

Yes, currently.

> and potentially dangerous;

That's the reason, I explicitly mentioned it.

>it could break code that tries to access OID in a
>not-yet-completely-built header.  The test that insists t_hoff is valid
>before one can *set* OID is even more likely to cause trouble.

This was actually the case in one or two places;  I had to change the
order of assignments there.  The code does not contain too many places
where a heap tuple header is built.  If the assertions break the code,
we (or in this case: I) have to fix it.  I need the assertions to find
those spots, if there are any left.

>I do not see the point of this in any case.

I see you have read my proposal on -hackers in the meantime.  Does
that make it clearer?

>            regards, tom lane
Thanks for reviewing my patch!

Servus
 Manfred

Manfred Koizar <mkoi-pg@aon.at> writes:
>> I do not see the point of this in any case.

> I see you have read my proposal on -hackers in the meantime.  Does
> that make it clearer?

Yes, but I still think the Assert is useless.  The only thing it can
catch is failure to set t_hoff before touching the OID, but in practice
if t_hoff has not been set then it will be uninitialized garbage; the
odds that the Assert will actually fire are only about 1 in 10.

            regards, tom lane

On Mon, 01 Jul 2002 11:10:20 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:
>Yes, but I still think the Assert is useless.  The only thing it can
>catch is failure to set t_hoff before touching the OID, but in practice
>if t_hoff has not been set then it will be uninitialized garbage; the
>odds that the Assert will actually fire are only about 1 in 10.

Sad, but true.  Thanks for pointing that out.

AssertMacro((tup)->t_hoff >= offsetof(HeapTupleHeaderData, t_bits))

should be

AssertMacro((tup)->t_hoff == ExpectedLen(tup))

where

#define ExpectedLen(tup) MAXALIGN( \
               offsetof(HeapTupleHeaderData, t_bits) + \
               (((tup)->t_infomask | HEAP_HASNULL) \
                ? BITMAPLEN((tup)->t_natts) : 0) \
                                 )
Sure, this is even more expensive, but it catches 255 out of 256
errors.

On Mon, 01 Jul 2002 10:40:34 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote
on -hackers:
>Also, you could be a little more conservative about
>adding Asserts --- those are not free, at least not from a development
>point of view, so I object to adding multiple redundant Asserts in
>hotspot routines.

Messing around with tuple headers is delicate stuff IMHO, and I don't
want to be remembered as the man who broke the best open source
database.  So I cowardly put in those Asserts ... If you are concerned
about performance in development versions, is there any chance you
could be convinced of a configurable paranoia level?  Then I could
write
    Assert3(...)

which would expand to
    if (PARANOIA_LEVEL >= 3) Assert(...)

Servus
 Manfred

Manfred Koizar <mkoi-pg@aon.at> writes:
> AssertMacro((tup)->t_hoff == ExpectedLen(tup))

> where

> #define ExpectedLen(tup) MAXALIGN( \
>                offsetof(HeapTupleHeaderData, t_bits) + \
>                (((tup)->t_infomask | HEAP_HASNULL) \
>                 ? BITMAPLEN((tup)->t_natts) : 0) \
>                                  )
> Sure, this is even more expensive, but it catches 255 out of 256
> errors.

Yikes.  Once per routine would be one thing, but once per macro call?

> On Mon, 01 Jul 2002 10:40:34 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote
> on -hackers:
>> Also, you could be a little more conservative about
>> adding Asserts --- those are not free, at least not from a development
>> point of view, so I object to adding multiple redundant Asserts in
>> hotspot routines.

> Messing around with tuple headers is delicate stuff IMHO, and I don't
> want to be remembered as the man who broke the best open source
> database.  So I cowardly put in those Asserts ...

The other side of the coin is that if you break anything, it will be
*extremely* obvious.  I'm not particularly worried.

You might want to think about the fact that all the FooIsValid(fooptr)
macros expand to just "fooptr != NULL", and not to some amazingly
complete test of validity of the pointed-to object.  There's a tradeoff
that has to be made here between development cycles and the probability
of actually catching any bugs.  I think the asserts you've been
submitting are well over into the diminishing-returns end of that
spectrum.

> If you are concerned
> about performance in development versions, is there any chance you
> could be convinced of a configurable paranoia level?

The way we usually handle that is #define symbols that increase the
debuggability of a particular subsystem.  Perhaps you could provide
a symbol named something like DEBUG_TUPLE_ACCESS that causes
more-paranoid versions of the tuple access macros to be chosen.

            regards, tom lane

On Wed, 03 Jul 2002 10:21:40 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:
>Yikes.  Once per routine would be one thing, but once per macro call?

There's not much of a difference, because most routines read or set
the oid only once.  The most notable exception to this is heap_insert
with four oid accesses.  A local variable might help here.

By having the assertion in the macro we can be sure it is called, when
it is needed, (and not called if not needed) and once we trust the new
code we can remove it easily.

>You might want to think about the fact that all the FooIsValid(fooptr)
>macros expand to just "fooptr != NULL", and not to some amazingly
>complete test of validity of the pointed-to object.

Ok, so I'll reduce the definition of AssertRelationHasOids(rel) to
just Assert(rel->rd_rel->relhasoids) and let the backend crash, if
either rel or rel->rd_rel is not a valid pointer.  This seems good
enough for testing.

I'll let the AssertRelationHasOid calls in for now.  They will go
away, when hasoids goes into TupleDesc.

>The way we usually handle that is #define symbols that increase the
>debuggability of a particular subsystem.  Perhaps you could provide
>a symbol named something like DEBUG_TUPLE_ACCESS that causes
>more-paranoid versions of the tuple access macros to be chosen.

#ifdef DEBUG_TUPLE_ACCESS
#define AssertHoffIsValid(tup) \
    AssertMacro((tup)->t_hoff == ExpectedLen(tup))
#else
#define AssertHoffIsValid(tup)
#endif

Where should DEBUG_TUPLE_ACCESS be defined or undef'd?

As regards your complaint about the patch making oid access dependent
on Relation, my POV is, that I did not introduce this dependence;
currently whether a tuple header is supposed to contain a valid oid or
not (apart from the fact, that it has a t_oid field anyway) *is*
dependent on Relation-...->hasoids, we have nothing better to look at.
If this dependence is unwanted, it shall be subject to another patch.

If there are no objections, my next steps will be
.  submit a new version of this patch with the changes mentioned above
.  make one or more patches that make the code compatible with both
   the old and the new format (e.g. putting hasoid into TupleDesc),
   attacking one issue at a time and keeping the code working after
   each step
.  and finally make a small patch that only changes htup.h

Servus
 Manfred

Manfred Koizar <mkoi-pg@aon.at> writes:
> Where should DEBUG_TUPLE_ACCESS be defined or undef'd?

It doesn't need to be anywhere, but you could add it to pg_config.h.in
near line 308, where there are other similar symbols, eg
    /* #define ACLDEBUG */

> As regards your complaint about the patch making oid access dependent
> on Relation, my POV is, that I did not introduce this dependence;
> currently whether a tuple header is supposed to contain a valid oid or
> not (apart from the fact, that it has a t_oid field anyway) *is*
> dependent on Relation-...->hasoids, we have nothing better to look at.

My point is that in the existing code, you can fetch t_oid and be sure
you will get 0 (and not either garbage or a crash) if no valid OID has
been assigned; this is relied on in several places that do not have
access to a Relation.  To do the same in your proposed change, those
places will need more knowledge.  They can't just blindly fetch from
tuple + t_hoff - sizeof(Oid).

            regards, tom lane

On Wed, 03 Jul 2002 13:50:45 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:
>> Where should DEBUG_TUPLE_ACCESS be defined or undef'd?
>
>It doesn't need to be anywhere, but you could add it to pg_config.h.in
>near line 308, where there are other similar symbols, eg
>    /* #define ACLDEBUG */

Ok, initially I'll define it there so that all the fellow hackers can
help debug my patches ;-)

>To do the same in your proposed change, those
>places will need more knowledge.  They can't just blindly fetch from
>tuple + t_hoff - sizeof(Oid).

Yes, that's why I have planned some intermediate steps before finally
switching to the new format.  As you have guessed by now I'm not a
friend of huge "do it all at once" patches.  I prefer to do a number
of small steps.  This first patch does not break the code and takes us
one step in the right direction.  More patches will follow.

As nobody else commented, it seems that it depends on your approval,
before a revised version of the patch will be applied.

Servus
 Manfred