Re: Last infomask bit - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: Last infomask bit
Date
Msg-id 200701092201.l09M15F17956@momjian.us
Whole thread Raw
In response to Last infomask bit  (Heikki Linnakangas <heikki@enterprisedb.com>)
Responses Re: Last infomask bit
List pgsql-patches
Patch applied.  Thanks.

I added a comment about the unused bits in the header file.

---------------------------------------------------------------------------



Heikki Linnakangas wrote:
> Hi,
>
> We're running out of infomask bits in the tuple header. I bumped into
> this as I tried to apply both the phantom command ids patch, and the HOT
> patch simultaneously. They both require one infomask bit, so they
> conflicted.
>
> This has been discussed before; I think the best approach is to use the
> extra bits available in t_natts field. Here's a patch that doesn't do
> anything interesting in itself, but it renames the t_natts field to
> t_infomask2, with 11 bits reserved for the number of attributes and the
> other 5 bits available for future use. All references to the old t_natts
> field are replaced with a HeapTupleHeaderGetNatts accessor macro.
>
> I believe it would actually be even better to combine the t_natts and
> t_infomask fields to a single 32-bit infomask field. I refrained from
> doing that for now because it would've required shifting all the
> existing infomask flags.
>
> Naturally, there's no point applying this before we actually need more
> infobits, but it's good to be prepared.
>
> --
>    Heikki Linnakangas
>    EnterpriseDB   http://www.enterprisedb.com

[ text/x-patch is unsupported, treating like TEXT/PLAIN ]

> Index: src/backend/access/common/heaptuple.c
> ===================================================================
> RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/common/heaptuple.c,v
> retrieving revision 1.112
> diff -c -r1.112 heaptuple.c
> *** src/backend/access/common/heaptuple.c    23 Nov 2006 05:27:18 -0000    1.112
> --- src/backend/access/common/heaptuple.c    5 Jan 2007 13:11:10 -0000
> ***************
> *** 295,301 ****
>   bool
>   heap_attisnull(HeapTuple tup, int attnum)
>   {
> !     if (attnum > (int) tup->t_data->t_natts)
>           return true;
>
>       if (attnum > 0)
> --- 295,301 ----
>   bool
>   heap_attisnull(HeapTuple tup, int attnum)
>   {
> !     if (attnum > (int) HeapTupleHeaderGetNatts(tup->t_data))
>           return true;
>
>       if (attnum > 0)
> ***************
> *** 474,479 ****
> --- 474,480 ----
>       {
>           int            j = 1;
>           long        off;
> +         int            natts = HeapTupleHeaderGetNatts(tup);
>
>           /*
>            * need to set cache for some atts
> ***************
> *** 488,494 ****
>
>           for (; j <= attnum ||
>           /* Can we compute more?  We will probably need them */
> !              (j < tup->t_natts &&
>                 att[j]->attcacheoff == -1 &&
>                 (HeapTupleNoNulls(tuple) || !att_isnull(j, bp)) &&
>                 (HeapTupleAllFixed(tuple) || att[j]->attlen > 0)); j++)
> --- 489,495 ----
>
>           for (; j <= attnum ||
>           /* Can we compute more?  We will probably need them */
> !              (j < natts &&
>                 att[j]->attcacheoff == -1 &&
>                 (HeapTupleNoNulls(tuple) || !att_isnull(j, bp)) &&
>                 (HeapTupleAllFixed(tuple) || att[j]->attlen > 0)); j++)
> ***************
> *** 739,745 ****
>       HeapTupleHeaderSetTypeId(td, tupleDescriptor->tdtypeid);
>       HeapTupleHeaderSetTypMod(td, tupleDescriptor->tdtypmod);
>
> !     td->t_natts = numberOfAttributes;
>       td->t_hoff = hoff;
>
>       if (tupleDescriptor->tdhasoid)        /* else leave infomask = 0 */
> --- 740,746 ----
>       HeapTupleHeaderSetTypeId(td, tupleDescriptor->tdtypeid);
>       HeapTupleHeaderSetTypMod(td, tupleDescriptor->tdtypmod);
>
> !     HeapTupleHeaderSetNatts(td, numberOfAttributes);
>       td->t_hoff = hoff;
>
>       if (tupleDescriptor->tdhasoid)        /* else leave infomask = 0 */
> ***************
> *** 846,852 ****
>       HeapTupleHeaderSetTypeId(td, tupleDescriptor->tdtypeid);
>       HeapTupleHeaderSetTypMod(td, tupleDescriptor->tdtypmod);
>
> !     td->t_natts = numberOfAttributes;
>       td->t_hoff = hoff;
>
>       if (tupleDescriptor->tdhasoid)        /* else leave infomask = 0 */
> --- 847,853 ----
>       HeapTupleHeaderSetTypeId(td, tupleDescriptor->tdtypeid);
>       HeapTupleHeaderSetTypMod(td, tupleDescriptor->tdtypmod);
>
> !     HeapTupleHeaderSetNatts(td, numberOfAttributes);
>       td->t_hoff = hoff;
>
>       if (tupleDescriptor->tdhasoid)        /* else leave infomask = 0 */
> ***************
> *** 1035,1041 ****
>       bits8       *bp = tup->t_bits;        /* ptr to null bitmap in tuple */
>       bool        slow = false;    /* can we use/set attcacheoff? */
>
> !     natts = tup->t_natts;
>
>       /*
>        * In inheritance situations, it is possible that the given tuple actually
> --- 1036,1042 ----
>       bits8       *bp = tup->t_bits;        /* ptr to null bitmap in tuple */
>       bool        slow = false;    /* can we use/set attcacheoff? */
>
> !     natts = HeapTupleHeaderGetNatts(tup);
>
>       /*
>        * In inheritance situations, it is possible that the given tuple actually
> ***************
> *** 1128,1134 ****
>       bits8       *bp = tup->t_bits;        /* ptr to null bitmap in tuple */
>       bool        slow = false;    /* can we use/set attcacheoff? */
>
> !     natts = tup->t_natts;
>
>       /*
>        * In inheritance situations, it is possible that the given tuple actually
> --- 1129,1135 ----
>       bits8       *bp = tup->t_bits;        /* ptr to null bitmap in tuple */
>       bool        slow = false;    /* can we use/set attcacheoff? */
>
> !     natts = HeapTupleHeaderGetNatts(tup);
>
>       /*
>        * In inheritance situations, it is possible that the given tuple actually
> ***************
> *** 1335,1341 ****
>        * than the tupdesc.)
>        */
>       tup = tuple->t_data;
> !     if (attnum > tup->t_natts)
>       {
>           *isnull = true;
>           return (Datum) 0;
> --- 1336,1342 ----
>        * than the tupdesc.)
>        */
>       tup = tuple->t_data;
> !     if (attnum > HeapTupleHeaderGetNatts(tup))
>       {
>           *isnull = true;
>           return (Datum) 0;
> ***************
> *** 1401,1407 ****
>       /*
>        * load up any slots available from physical tuple
>        */
> !     attnum = tuple->t_data->t_natts;
>       attnum = Min(attnum, tdesc_natts);
>
>       slot_deform_tuple(slot, attnum);
> --- 1402,1408 ----
>       /*
>        * load up any slots available from physical tuple
>        */
> !     attnum = HeapTupleHeaderGetNatts(tuple->t_data);
>       attnum = Min(attnum, tdesc_natts);
>
>       slot_deform_tuple(slot, attnum);
> ***************
> *** 1448,1454 ****
>       /*
>        * load up any slots available from physical tuple
>        */
> !     attno = tuple->t_data->t_natts;
>       attno = Min(attno, attnum);
>
>       slot_deform_tuple(slot, attno);
> --- 1449,1455 ----
>       /*
>        * load up any slots available from physical tuple
>        */
> !     attno = HeapTupleHeaderGetNatts(tuple->t_data);
>       attno = Min(attno, attnum);
>
>       slot_deform_tuple(slot, attno);
> ***************
> *** 1601,1607 ****
>        * And fill in the information.
>        */
>       tuple->t_len = len;
> !     tuple->t_natts = numberOfAttributes;
>       tuple->t_hoff = hoff + MINIMAL_TUPLE_OFFSET;
>
>       if (tupleDescriptor->tdhasoid)        /* else leave infomask = 0 */
> --- 1602,1608 ----
>        * And fill in the information.
>        */
>       tuple->t_len = len;
> !     HeapTupleHeaderSetNatts(tuple, numberOfAttributes);
>       tuple->t_hoff = hoff + MINIMAL_TUPLE_OFFSET;
>
>       if (tupleDescriptor->tdhasoid)        /* else leave infomask = 0 */
> ***************
> *** 1663,1669 ****
>       result->t_tableOid = InvalidOid;
>       result->t_data = (HeapTupleHeader) ((char *) result + HEAPTUPLESIZE);
>       memcpy((char *) result->t_data + MINIMAL_TUPLE_OFFSET, mtup, mtup->t_len);
> !     memset(result->t_data, 0, offsetof(HeapTupleHeaderData, t_natts));
>       return result;
>   }
>
> --- 1664,1670 ----
>       result->t_tableOid = InvalidOid;
>       result->t_data = (HeapTupleHeader) ((char *) result + HEAPTUPLESIZE);
>       memcpy((char *) result->t_data + MINIMAL_TUPLE_OFFSET, mtup, mtup->t_len);
> !     memset(result->t_data, 0, offsetof(HeapTupleHeaderData, t_infomask2));
>       return result;
>   }
>
> ***************
> *** 1729,1735 ****
>
>       /* we don't bother to fill the Datum fields */
>
> !     td->t_natts = natts;
>       td->t_hoff = hoff;
>
>       if (withoid)                /* else leave infomask = 0 */
> --- 1730,1736 ----
>
>       /* we don't bother to fill the Datum fields */
>
> !     HeapTupleHeaderSetNatts(td, natts);
>       td->t_hoff = hoff;
>
>       if (withoid)                /* else leave infomask = 0 */
> Index: src/backend/access/heap/heapam.c
> ===================================================================
> RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/heap/heapam.c,v
> retrieving revision 1.222
> diff -c -r1.222 heapam.c
> *** src/backend/access/heap/heapam.c    17 Nov 2006 18:00:14 -0000    1.222
> --- src/backend/access/heap/heapam.c    5 Jan 2007 13:13:30 -0000
> ***************
> *** 1455,1461 ****
>           rdata[0].buffer = InvalidBuffer;
>           rdata[0].next = &(rdata[1]);
>
> !         xlhdr.t_natts = heaptup->t_data->t_natts;
>           xlhdr.t_infomask = heaptup->t_data->t_infomask;
>           xlhdr.t_hoff = heaptup->t_data->t_hoff;
>
> --- 1455,1461 ----
>           rdata[0].buffer = InvalidBuffer;
>           rdata[0].next = &(rdata[1]);
>
> !         xlhdr.t_infomask2 = heaptup->t_data->t_infomask2;
>           xlhdr.t_infomask = heaptup->t_data->t_infomask;
>           xlhdr.t_hoff = heaptup->t_data->t_hoff;
>
> ***************
> *** 3204,3210 ****
>       rdata[1].buffer_std = true;
>       rdata[1].next = &(rdata[2]);
>
> !     xlhdr.hdr.t_natts = newtup->t_data->t_natts;
>       xlhdr.hdr.t_infomask = newtup->t_data->t_infomask;
>       xlhdr.hdr.t_hoff = newtup->t_data->t_hoff;
>       if (move)                    /* remember xmax & xmin */
> --- 3204,3210 ----
>       rdata[1].buffer_std = true;
>       rdata[1].next = &(rdata[2]);
>
> !     xlhdr.hdr.t_infomask2 = newtup->t_data->t_infomask2;
>       xlhdr.hdr.t_infomask = newtup->t_data->t_infomask;
>       xlhdr.hdr.t_hoff = newtup->t_data->t_hoff;
>       if (move)                    /* remember xmax & xmin */
> ***************
> *** 3503,3509 ****
>              (char *) xlrec + SizeOfHeapInsert + SizeOfHeapHeader,
>              newlen);
>       newlen += offsetof(HeapTupleHeaderData, t_bits);
> !     htup->t_natts = xlhdr.t_natts;
>       htup->t_infomask = xlhdr.t_infomask;
>       htup->t_hoff = xlhdr.t_hoff;
>       HeapTupleHeaderSetXmin(htup, record->xl_xid);
> --- 3503,3509 ----
>              (char *) xlrec + SizeOfHeapInsert + SizeOfHeapHeader,
>              newlen);
>       newlen += offsetof(HeapTupleHeaderData, t_bits);
> !     htup->t_infomask2 = xlhdr.t_infomask2;
>       htup->t_infomask = xlhdr.t_infomask;
>       htup->t_hoff = xlhdr.t_hoff;
>       HeapTupleHeaderSetXmin(htup, record->xl_xid);
> ***************
> *** 3666,3672 ****
>              (char *) xlrec + hsize,
>              newlen);
>       newlen += offsetof(HeapTupleHeaderData, t_bits);
> !     htup->t_natts = xlhdr.t_natts;
>       htup->t_infomask = xlhdr.t_infomask;
>       htup->t_hoff = xlhdr.t_hoff;
>
> --- 3666,3672 ----
>              (char *) xlrec + hsize,
>              newlen);
>       newlen += offsetof(HeapTupleHeaderData, t_bits);
> !     htup->t_infomask2 = xlhdr.t_infomask2;
>       htup->t_infomask = xlhdr.t_infomask;
>       htup->t_hoff = xlhdr.t_hoff;
>
> Index: src/backend/executor/spi.c
> ===================================================================
> RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/executor/spi.c,v
> retrieving revision 1.165.2.2
> diff -c -r1.165.2.2 spi.c
> *** src/backend/executor/spi.c    26 Dec 2006 16:56:22 -0000    1.165.2.2
> --- src/backend/executor/spi.c    5 Jan 2007 13:16:42 -0000
> ***************
> *** 651,657 ****
>
>       SPI_result = 0;
>
> !     if (fnumber > tuple->t_data->t_natts || fnumber == 0 ||
>           fnumber <= FirstLowInvalidHeapAttributeNumber)
>       {
>           SPI_result = SPI_ERROR_NOATTRIBUTE;
> --- 651,657 ----
>
>       SPI_result = 0;
>
> !     if (fnumber > HeapTupleHeaderGetNatts(tuple->t_data) || fnumber == 0 ||
>           fnumber <= FirstLowInvalidHeapAttributeNumber)
>       {
>           SPI_result = SPI_ERROR_NOATTRIBUTE;
> ***************
> *** 692,698 ****
>   {
>       SPI_result = 0;
>
> !     if (fnumber > tuple->t_data->t_natts || fnumber == 0 ||
>           fnumber <= FirstLowInvalidHeapAttributeNumber)
>       {
>           SPI_result = SPI_ERROR_NOATTRIBUTE;
> --- 692,698 ----
>   {
>       SPI_result = 0;
>
> !     if (fnumber > HeapTupleHeaderGetNatts(tuple->t_data) || fnumber == 0 ||
>           fnumber <= FirstLowInvalidHeapAttributeNumber)
>       {
>           SPI_result = SPI_ERROR_NOATTRIBUTE;
> Index: src/include/access/heapam.h
> ===================================================================
> RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/include/access/heapam.h,v
> retrieving revision 1.117
> diff -c -r1.117 heapam.h
> *** src/include/access/heapam.h    5 Nov 2006 22:42:10 -0000    1.117
> --- src/include/access/heapam.h    5 Jan 2007 13:15:10 -0000
> ***************
> *** 98,104 ****
>       ( \
>           ((attnum) > 0) ? \
>           ( \
> !             ((attnum) > (int) (tup)->t_data->t_natts) ? \
>               ( \
>                   (((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \
>                   (Datum)NULL \
> --- 98,104 ----
>       ( \
>           ((attnum) > 0) ? \
>           ( \
> !             ((attnum) > (int) HeapTupleHeaderGetNatts((tup)->t_data)) ? \
>               ( \
>                   (((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \
>                   (Datum)NULL \
> Index: src/include/access/htup.h
> ===================================================================
> RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/include/access/htup.h,v
> retrieving revision 1.87
> diff -c -r1.87 htup.h
> *** src/include/access/htup.h    5 Nov 2006 22:42:10 -0000    1.87
> --- src/include/access/htup.h    5 Jan 2007 13:08:41 -0000
> ***************
> *** 139,145 ****
>
>       /* Fields below here must match MinimalTupleData! */
>
> !     int16        t_natts;        /* number of attributes */
>
>       uint16        t_infomask;        /* various flag bits, see below */
>
> --- 139,145 ----
>
>       /* Fields below here must match MinimalTupleData! */
>
> !     uint16        t_infomask2;    /* number of attributes + various flags */
>
>       uint16        t_infomask;        /* various flag bits, see below */
>
> ***************
> *** 182,187 ****
> --- 182,195 ----
>
>   #define HEAP_XACT_MASK            0xFFC0    /* visibility-related bits */
>
> + /* information stored in t_infomask2, and accessor macros */
> + #define HEAP_NATTS_MASK            0X7FF    /* 11 bits for number of attributes */
> +
> + #define HeapTupleHeaderGetNatts(tup) ((tup)->t_infomask2 & HEAP_NATTS_MASK)
> + #define HeapTupleHeaderSetNatts(tup, natts) \
> + ( \
> +     (tup)->t_infomask2 = ((tup)->t_infomask2 & ~HEAP_NATTS_MASK) | (natts) \
> + )
>
>   /*
>    * HeapTupleHeader accessor macros
> ***************
> *** 367,374 ****
>    * and thereby prevent accidental use of the nonexistent fields.
>    *
>    * MinimalTupleData contains a length word, some padding, and fields matching
> !  * HeapTupleHeaderData beginning with t_natts.    The padding is chosen so that
> !  * offsetof(t_natts) is the same modulo MAXIMUM_ALIGNOF in both structs.
>    * This makes data alignment rules equivalent in both cases.
>    *
>    * When a minimal tuple is accessed via a HeapTupleData pointer, t_data is
> --- 375,382 ----
>    * and thereby prevent accidental use of the nonexistent fields.
>    *
>    * MinimalTupleData contains a length word, some padding, and fields matching
> !  * HeapTupleHeaderData beginning with t_infomask2. The padding is chosen so that
> !  * offsetof(t_infomask2) is the same modulo MAXIMUM_ALIGNOF in both structs.
>    * This makes data alignment rules equivalent in both cases.
>    *
>    * When a minimal tuple is accessed via a HeapTupleData pointer, t_data is
> ***************
> *** 380,388 ****
>    * the MINIMAL_TUPLE_OFFSET distance.  t_len does not include that, however.
>    */
>   #define MINIMAL_TUPLE_OFFSET \
> !     ((offsetof(HeapTupleHeaderData, t_natts) - sizeof(uint32)) / MAXIMUM_ALIGNOF * MAXIMUM_ALIGNOF)
>   #define MINIMAL_TUPLE_PADDING \
> !     ((offsetof(HeapTupleHeaderData, t_natts) - sizeof(uint32)) % MAXIMUM_ALIGNOF)
>
>   typedef struct MinimalTupleData
>   {
> --- 388,396 ----
>    * the MINIMAL_TUPLE_OFFSET distance.  t_len does not include that, however.
>    */
>   #define MINIMAL_TUPLE_OFFSET \
> !     ((offsetof(HeapTupleHeaderData, t_infomask2) - sizeof(uint32)) / MAXIMUM_ALIGNOF * MAXIMUM_ALIGNOF)
>   #define MINIMAL_TUPLE_PADDING \
> !     ((offsetof(HeapTupleHeaderData, t_infomask2) - sizeof(uint32)) % MAXIMUM_ALIGNOF)
>
>   typedef struct MinimalTupleData
>   {
> ***************
> *** 392,398 ****
>
>       /* Fields below here must match HeapTupleHeaderData! */
>
> !     int16        t_natts;        /* number of attributes */
>
>       uint16        t_infomask;        /* various flag bits, see below */
>
> --- 400,406 ----
>
>       /* Fields below here must match HeapTupleHeaderData! */
>
> !     uint16        t_infomask2;    /* number of attributes + various flags */
>
>       uint16        t_infomask;        /* various flag bits, see below */
>
> ***************
> *** 552,558 ****
>    */
>   typedef struct xl_heap_header
>   {
> !     int16        t_natts;
>       uint16        t_infomask;
>       uint8        t_hoff;
>   } xl_heap_header;
> --- 560,566 ----
>    */
>   typedef struct xl_heap_header
>   {
> !     uint16        t_infomask2;
>       uint16        t_infomask;
>       uint8        t_hoff;
>   } xl_heap_header;
> Index: src/pl/plpgsql/src/pl_exec.c
> ===================================================================
> RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/pl/plpgsql/src/pl_exec.c,v
> retrieving revision 1.180
> diff -c -r1.180 pl_exec.c
> *** src/pl/plpgsql/src/pl_exec.c    4 Oct 2006 00:30:13 -0000    1.180
> --- src/pl/plpgsql/src/pl_exec.c    5 Jan 2007 13:19:14 -0000
> ***************
> *** 4092,4098 ****
>        * Row is a bit more complicated in that we assign the individual
>        * attributes of the tuple to the variables the row points to.
>        *
> !      * NOTE: this code used to demand row->nfields == tup->t_data->t_natts,
>        * but that's wrong.  The tuple might have more fields than we expected if
>        * it's from an inheritance-child table of the current table, or it might
>        * have fewer if the table has had columns added by ALTER TABLE. Ignore
> --- 4092,4098 ----
>        * Row is a bit more complicated in that we assign the individual
>        * attributes of the tuple to the variables the row points to.
>        *
> !      * NOTE: this code used to demand row->nfields == HeapTupleHeaderGetNatts(tup->t_data,
>        * but that's wrong.  The tuple might have more fields than we expected if
>        * it's from an inheritance-child table of the current table, or it might
>        * have fewer if the table has had columns added by ALTER TABLE. Ignore
> ***************
> *** 4110,4116 ****
>           int            anum;
>
>           if (HeapTupleIsValid(tup))
> !             t_natts = tup->t_data->t_natts;
>           else
>               t_natts = 0;
>
> --- 4110,4116 ----
>           int            anum;
>
>           if (HeapTupleIsValid(tup))
> !             t_natts = HeapTupleHeaderGetNatts(tup->t_data);
>           else
>               t_natts = 0;
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS] Patch to log usage of temporary files
Next
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Patch to log usage of temporary files