Обсуждение: Fixes to index pages

Tom, here are the changes I was thinking about to clean up a few areas
in index pages tables.  I will hold the patch until 7.2.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/access/common/indextuple.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/common/indextuple.c,v
retrieving revision 1.51
diff -c -w -r1.51 indextuple.c
*** src/backend/access/common/indextuple.c    2001/02/15 20:57:01    1.51
--- src/backend/access/common/indextuple.c    2001/02/21 23:23:27
***************
*** 230,236 ****

      attnum--;

!     if (IndexTupleNoNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
--- 230,236 ----

      attnum--;

!     if (!IndexTupleHasNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
***************
*** 301,307 ****
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (!IndexTupleAllFixed(tup))
          {
              int            j;

--- 301,307 ----
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (IndexTupleHasVars(tup))
          {
              int            j;

***************
*** 365,371 ****

          for (i = 0; i < attnum; i++)
          {
!             if (!IndexTupleNoNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
--- 365,371 ----

          for (i = 0; i < attnum; i++)
          {
!             if (IndexTupleHasNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
Index: src/backend/access/gist/gist.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/gist/gist.c,v
retrieving revision 1.69
diff -c -w -r1.69 gist.c
*** src/backend/access/gist/gist.c    2001/01/29 00:39:12    1.69
--- src/backend/access/gist/gist.c    2001/02/21 23:23:27
***************
*** 1102,1108 ****
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= 0xe000;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

--- 1102,1108 ----
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= ~INDEX_SIZE_MASK;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

Index: src/backend/access/hash/hash.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hash.c,v
retrieving revision 1.48
diff -c -w -r1.48 hash.c
*** src/backend/access/hash/hash.c    2001/01/29 00:39:13    1.48
--- src/backend/access/hash/hash.c    2001/02/21 23:23:28
***************
*** 170,176 ****
           * of the way nulls are handled here.
           */

!         if (itup->t_info & INDEX_NULL_MASK)
          {
              pfree(itup);
              continue;
--- 170,176 ----
           * of the way nulls are handled here.
           */

!         if (IndexTupleHasNulls(itup))
          {
              pfree(itup);
              continue;
***************
*** 256,262 ****
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (itup->t_info & INDEX_NULL_MASK)
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
--- 256,262 ----
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (IndexTupleHasNulls(itup))
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.25
diff -c -w -r1.25 hashutil.c
*** src/backend/access/hash/hashutil.c    2001/01/24 19:42:47    1.25
--- src/backend/access/hash/hashutil.c    2001/02/21 23:23:28
***************
*** 72,78 ****
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (itup->t_info & INDEX_NULL_MASK)
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
--- 72,78 ----
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (IndexTupleHasNulls(itup))
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
Index: src/include/access/itup.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/itup.h,v
retrieving revision 1.29
diff -c -w -r1.29 itup.h
*** src/include/access/itup.h    2001/02/21 19:07:04    1.29
--- src/include/access/itup.h    2001/02/21 23:23:34
***************
*** 27,35 ****
      /*
       * t_info is layed out in the following fashion:
       *
!      * 15th (leftmost) bit: "has nulls" bit 14th bit: "has varlenas" bit 13th
!      * bit: "has rules" bit - (removed ay 11/94) bits 12-0 bit: size of
!      * tuple.
       */

      unsigned short t_info;        /* various info about tuple */
--- 27,35 ----
      /*
       * t_info is layed out in the following fashion:
       *
!      * 15th (leftmost) bit: has nulls
!      * 14th bit: has varlenas
!      * 13-0 bit: size of tuple
       */

      unsigned short t_info;        /* various info about tuple */
***************
*** 66,81 ****
   * ----------------
   */

! #define INDEX_SIZE_MASK 0x1FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)    ((Size) (((IndexTuple) (itup))->t_info & 0x1FFF))
! #define IndexTupleDSize(itup)                  ((Size) ((itup).t_info & 0x1FFF))
! #define IndexTupleNoNulls(itup)  (!(((IndexTuple) (itup))->t_info & 0x8000))
! #define IndexTupleAllFixed(itup) (!(((IndexTuple) (itup))->t_info & 0x4000))

! #define IndexTupleHasMinHeader(itup) (IndexTupleNoNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
--- 66,81 ----
   * ----------------
   */

! #define INDEX_SIZE_MASK 0x3FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)        ((Size) (((IndexTuple) (itup))->t_info & INDEX_SIZE_MASK))
! #define IndexTupleDSize(itup)        ((Size) ((itup).t_info & INDEX_SIZE_MASK))
! #define IndexTupleHasNulls(itup)     ((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK))
! #define IndexTupleHasVars(itup)        ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))

! #define IndexTupleHasMinHeader(itup) (!IndexTupleHasNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
***************
*** 107,113 ****
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     IndexTupleNoNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
--- 107,113 ----
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     !IndexTupleHasNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
Index: src/include/access/nbtree.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/nbtree.h,v
retrieving revision 1.52
diff -c -w -r1.52 nbtree.h
*** src/include/access/nbtree.h    2001/02/21 19:07:04    1.52
--- src/include/access/nbtree.h    2001/02/21 23:23:34
***************
*** 50,56 ****


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \
--- 50,56 ----


  #define BTREE_METAPAGE    0            /* first page is meta */
! #define BTREE_MAGIC        0x053162    /* magic number of btree pages */

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom, here are the changes I was thinking about to clean up a few areas
> in index pages tables.  I will hold the patch until 7.2.

What happened to our discussion about keeping t_info bit 13 unused??

Still don't like the name "IndexTupleHasVars" ... sounds to me like that
means it has variables in it, which is not very sensical.  Maybe
"IndexTupleHasVarlenas"?

Also, if you don't put some dashes around the comment in itup.h line
27ff, pgindent will munge it for you, just like it did for the last guy.
(Have I mentioned that I really hate pgindent's handling of comment
blocks?)

            regards, tom lane

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom, here are the changes I was thinking about to clean up a few areas
> > in index pages tables.  I will hold the patch until 7.2.
>
> What happened to our discussion about keeping t_info bit 13 unused??

I wasn't going to reserve it in the patch. I figured I would make all
the items/flags match, and if someone wants to reserve it, it is easy to
do in one place.  I imagine 7.2 is going to be dump/reload anyway so the
decision can be made during development cycle.  I basically didn't want
to leave a bit gap and leave it unnamed because it could cause
confusion.


> Still don't like the name "IndexTupleHasVars" ... sounds to me like that
> means it has variables in it, which is not very sensical.  Maybe
> "IndexTupleHasVarlenas"?

Done.  Patch attached.

> Also, if you don't put some dashes around the comment in itup.h line
> 27ff, pgindent will munge it for you, just like it did for the last guy.
> (Have I mentioned that I really hate pgindent's handling of comment
> blocks?)

I see.  Done.  That comment wrapping is a _feature_ of BSD indent.  I
can easily disable it if people don't want it.  I have seen it clean up
some pretty ugly comments, but I have seen it mess a few too.  I think
it does better good than harm, but others may disagree.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/access/common/indextuple.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/common/indextuple.c,v
retrieving revision 1.51
diff -c -w -r1.51 indextuple.c
*** src/backend/access/common/indextuple.c    2001/02/15 20:57:01    1.51
--- src/backend/access/common/indextuple.c    2001/02/22 00:27:56
***************
*** 230,236 ****

      attnum--;

!     if (IndexTupleNoNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
--- 230,236 ----

      attnum--;

!     if (!IndexTupleHasNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
***************
*** 301,307 ****
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (!IndexTupleAllFixed(tup))
          {
              int            j;

--- 301,307 ----
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (IndexTupleHasVarlenas(tup))
          {
              int            j;

***************
*** 365,371 ****

          for (i = 0; i < attnum; i++)
          {
!             if (!IndexTupleNoNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
--- 365,371 ----

          for (i = 0; i < attnum; i++)
          {
!             if (IndexTupleHasNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
Index: src/backend/access/gist/gist.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/gist/gist.c,v
retrieving revision 1.69
diff -c -w -r1.69 gist.c
*** src/backend/access/gist/gist.c    2001/01/29 00:39:12    1.69
--- src/backend/access/gist/gist.c    2001/02/22 00:27:57
***************
*** 1102,1108 ****
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= 0xe000;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

--- 1102,1108 ----
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= ~INDEX_SIZE_MASK;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

Index: src/backend/access/hash/hash.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hash.c,v
retrieving revision 1.48
diff -c -w -r1.48 hash.c
*** src/backend/access/hash/hash.c    2001/01/29 00:39:13    1.48
--- src/backend/access/hash/hash.c    2001/02/22 00:27:57
***************
*** 170,176 ****
           * of the way nulls are handled here.
           */

!         if (itup->t_info & INDEX_NULL_MASK)
          {
              pfree(itup);
              continue;
--- 170,176 ----
           * of the way nulls are handled here.
           */

!         if (IndexTupleHasNulls(itup))
          {
              pfree(itup);
              continue;
***************
*** 256,262 ****
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (itup->t_info & INDEX_NULL_MASK)
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
--- 256,262 ----
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (IndexTupleHasNulls(itup))
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.25
diff -c -w -r1.25 hashutil.c
*** src/backend/access/hash/hashutil.c    2001/01/24 19:42:47    1.25
--- src/backend/access/hash/hashutil.c    2001/02/22 00:27:57
***************
*** 72,78 ****
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (itup->t_info & INDEX_NULL_MASK)
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
--- 72,78 ----
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (IndexTupleHasNulls(itup))
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
Index: src/include/access/itup.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/itup.h,v
retrieving revision 1.29
diff -c -w -r1.29 itup.h
*** src/include/access/itup.h    2001/02/21 19:07:04    1.29
--- src/include/access/itup.h    2001/02/22 00:27:59
***************
*** 24,35 ****
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /*
       * t_info is layed out in the following fashion:
       *
!      * 15th (leftmost) bit: "has nulls" bit 14th bit: "has varlenas" bit 13th
!      * bit: "has rules" bit - (removed ay 11/94) bits 12-0 bit: size of
!      * tuple.
       */

      unsigned short t_info;        /* various info about tuple */
--- 24,36 ----
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /* ---------------
       * t_info is layed out in the following fashion:
       *
!      * 15th (high) bit: has nulls
!      * 14th bit: has varlenas
!      * 13-0 bit: size of tuple
!      * ---------------
       */

      unsigned short t_info;        /* various info about tuple */
***************
*** 66,81 ****
   * ----------------
   */

! #define INDEX_SIZE_MASK 0x1FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)    ((Size) (((IndexTuple) (itup))->t_info & 0x1FFF))
! #define IndexTupleDSize(itup)                  ((Size) ((itup).t_info & 0x1FFF))
! #define IndexTupleNoNulls(itup)  (!(((IndexTuple) (itup))->t_info & 0x8000))
! #define IndexTupleAllFixed(itup) (!(((IndexTuple) (itup))->t_info & 0x4000))

! #define IndexTupleHasMinHeader(itup) (IndexTupleNoNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
--- 67,82 ----
   * ----------------
   */

! #define INDEX_SIZE_MASK 0x3FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)        ((Size) (((IndexTuple) (itup))->t_info & INDEX_SIZE_MASK))
! #define IndexTupleDSize(itup)        ((Size) ((itup).t_info & INDEX_SIZE_MASK))
! #define IndexTupleHasNulls(itup)     ((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK))
! #define IndexTupleHasVarlenas(itup)        ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))

! #define IndexTupleHasMinHeader(itup) (!IndexTupleHasNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
***************
*** 107,113 ****
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     IndexTupleNoNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
--- 108,114 ----
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     !IndexTupleHasNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
Index: src/include/access/nbtree.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/nbtree.h,v
retrieving revision 1.52
diff -c -w -r1.52 nbtree.h
*** src/include/access/nbtree.h    2001/02/21 19:07:04    1.52
--- src/include/access/nbtree.h    2001/02/22 00:27:59
***************
*** 50,56 ****


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \
--- 50,56 ----


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162    /* magic number of btree pages */

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \

Bruce Momjian wrote:
>
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Tom, here are the changes I was thinking about to clean up a few areas
> > > in index pages tables.  I will hold the patch until 7.2.
> >
> > What happened to our discussion about keeping t_info bit 13 unused??
>
> I wasn't going to reserve it in the patch. I figured I would make all
> the items/flags match, and if someone wants to reserve it, it is easy to
> do in one place.  I imagine 7.2 is going to be dump/reload anyway so the
> decision can be made during development cycle.  I basically didn't want
> to leave a bit gap and leave it unnamed because it could cause
> confusion.
>

You have added the following TODO recently.
 * Add deleted bit to index tuples to reduce heap access

Where would you have the deleted bit in IndexTupleData ?

Regards,
Hiroshi Inoue

> Bruce Momjian wrote:
> >
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > Tom, here are the changes I was thinking about to clean up a few areas
> > > > in index pages tables.  I will hold the patch until 7.2.
> > >
> > > What happened to our discussion about keeping t_info bit 13 unused??
> >
> > I wasn't going to reserve it in the patch. I figured I would make all
> > the items/flags match, and if someone wants to reserve it, it is easy to
> > do in one place.  I imagine 7.2 is going to be dump/reload anyway so the
> > decision can be made during development cycle.  I basically didn't want
> > to leave a bit gap and leave it unnamed because it could cause
> > confusion.
> >
>
> You have added the following TODO recently.
>  * Add deleted bit to index tuples to reduce heap access
>
> Where would you have the deleted bit in IndexTupleData ?

Wow, seems like everyone liked the deleted bit idea.  :-)

I would put it in bit 13.  I would adjust the bit masks in the itup.h
file.  I assume you are asking why I don't do it in the patch, and the
reason is because I have no code to update the bit field.   I think the
bit should be reserved at the time the code is added.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian wrote:
>
> > Bruce Momjian wrote:
> > >
> > > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > > Tom, here are the changes I was thinking about to clean up a few areas
> > > > > in index pages tables.  I will hold the patch until 7.2.
> > > >
> > > > What happened to our discussion about keeping t_info bit 13 unused??
> > >
> > > I wasn't going to reserve it in the patch. I figured I would make all
> > > the items/flags match, and if someone wants to reserve it, it is easy to
> > > do in one place.  I imagine 7.2 is going to be dump/reload anyway so the
> > > decision can be made during development cycle.  I basically didn't want
> > > to leave a bit gap and leave it unnamed because it could cause
> > > confusion.
> > >
> >
> > You have added the following TODO recently.
> >  * Add deleted bit to index tuples to reduce heap access
> >
> > Where would you have the deleted bit in IndexTupleData ?
>
> Wow, seems like everyone liked the deleted bit idea.  :-)
>
> I would put it in bit 13.  I would adjust the bit masks in the itup.h
> file.  I assume you are asking why I don't do it in the patch,

I don't think it's a good idea to fill bit 13 by force.
There's only 1 bit unused. IMHO there must be a discussion
about how to use the bit.

Regards,
Hiroshi Inoue

> > > > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > > > Tom, here are the changes I was thinking about to clean up a few areas
> > > > > > in index pages tables.  I will hold the patch until 7.2.
> > > > >
> > > > > What happened to our discussion about keeping t_info bit 13 unused??
> > > >
> > > > I wasn't going to reserve it in the patch. I figured I would make all
> > > > the items/flags match, and if someone wants to reserve it, it is easy to
> > > > do in one place.  I imagine 7.2 is going to be dump/reload anyway so the
> > > > decision can be made during development cycle.  I basically didn't want
> > > > to leave a bit gap and leave it unnamed because it could cause
> > > > confusion.
> > > >
> > >
> > > You have added the following TODO recently.
> > >  * Add deleted bit to index tuples to reduce heap access
> > >
> > > Where would you have the deleted bit in IndexTupleData ?
> >
> > Wow, seems like everyone liked the deleted bit idea.  :-)
> >
> > I would put it in bit 13.  I would adjust the bit masks in the itup.h
> > file.  I assume you are asking why I don't do it in the patch,
>
> I don't think it's a good idea to fill bit 13 by force.
> There's only 1 bit unused. IMHO there must be a discussion
> about how to use the bit.

I am not doing anything to 7.1, just 7.2.  My patch is just an attempt
to make the source accurate.  If you prefer, I will document the bit as
"unused" and let someone else change it later.

If I document the bit as "unused" I can apply the patch to 7.1 because
then the patch has no affect except to clean up the source labels.
Comments?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/access/common/indextuple.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/common/indextuple.c,v
retrieving revision 1.51
diff -c -w -r1.51 indextuple.c
*** src/backend/access/common/indextuple.c    2001/02/15 20:57:01    1.51
--- src/backend/access/common/indextuple.c    2001/02/22 00:27:56
***************
*** 230,236 ****

      attnum--;

!     if (IndexTupleNoNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
--- 230,236 ----

      attnum--;

!     if (!IndexTupleHasNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
***************
*** 301,307 ****
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (!IndexTupleAllFixed(tup))
          {
              int            j;

--- 301,307 ----
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (IndexTupleHasVarlenas(tup))
          {
              int            j;

***************
*** 365,371 ****

          for (i = 0; i < attnum; i++)
          {
!             if (!IndexTupleNoNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
--- 365,371 ----

          for (i = 0; i < attnum; i++)
          {
!             if (IndexTupleHasNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
Index: src/backend/access/gist/gist.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/gist/gist.c,v
retrieving revision 1.69
diff -c -w -r1.69 gist.c
*** src/backend/access/gist/gist.c    2001/01/29 00:39:12    1.69
--- src/backend/access/gist/gist.c    2001/02/22 00:27:57
***************
*** 1102,1108 ****
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= 0xe000;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

--- 1102,1108 ----
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= ~INDEX_SIZE_MASK;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

Index: src/backend/access/hash/hash.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hash.c,v
retrieving revision 1.48
diff -c -w -r1.48 hash.c
*** src/backend/access/hash/hash.c    2001/01/29 00:39:13    1.48
--- src/backend/access/hash/hash.c    2001/02/22 00:27:57
***************
*** 170,176 ****
           * of the way nulls are handled here.
           */

!         if (itup->t_info & INDEX_NULL_MASK)
          {
              pfree(itup);
              continue;
--- 170,176 ----
           * of the way nulls are handled here.
           */

!         if (IndexTupleHasNulls(itup))
          {
              pfree(itup);
              continue;
***************
*** 256,262 ****
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (itup->t_info & INDEX_NULL_MASK)
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
--- 256,262 ----
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (IndexTupleHasNulls(itup))
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.25
diff -c -w -r1.25 hashutil.c
*** src/backend/access/hash/hashutil.c    2001/01/24 19:42:47    1.25
--- src/backend/access/hash/hashutil.c    2001/02/22 00:27:57
***************
*** 72,78 ****
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (itup->t_info & INDEX_NULL_MASK)
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
--- 72,78 ----
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (IndexTupleHasNulls(itup))
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
Index: src/include/access/itup.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/itup.h,v
retrieving revision 1.29
diff -c -w -r1.29 itup.h
*** src/include/access/itup.h    2001/02/21 19:07:04    1.29
--- src/include/access/itup.h    2001/02/22 00:27:59
***************
*** 24,35 ****
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /*
       * t_info is layed out in the following fashion:
       *
!      * 15th (leftmost) bit: "has nulls" bit 14th bit: "has varlenas" bit 13th
!      * bit: "has rules" bit - (removed ay 11/94) bits 12-0 bit: size of
!      * tuple.
       */

      unsigned short t_info;        /* various info about tuple */
--- 24,36 ----
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /* ---------------
       * t_info is layed out in the following fashion:
       *
!      * 15th (high) bit: has nulls
!      * 14th bit: has varlenas
!      * 13-0 bit: size of tuple
!      * ---------------
       */

      unsigned short t_info;        /* various info about tuple */
***************
*** 66,81 ****
   * ----------------
   */

! #define INDEX_SIZE_MASK 0x1FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)    ((Size) (((IndexTuple) (itup))->t_info & 0x1FFF))
! #define IndexTupleDSize(itup)                  ((Size) ((itup).t_info & 0x1FFF))
! #define IndexTupleNoNulls(itup)  (!(((IndexTuple) (itup))->t_info & 0x8000))
! #define IndexTupleAllFixed(itup) (!(((IndexTuple) (itup))->t_info & 0x4000))

! #define IndexTupleHasMinHeader(itup) (IndexTupleNoNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
--- 67,82 ----
   * ----------------
   */

! #define INDEX_SIZE_MASK 0x3FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)        ((Size) (((IndexTuple) (itup))->t_info & INDEX_SIZE_MASK))
! #define IndexTupleDSize(itup)        ((Size) ((itup).t_info & INDEX_SIZE_MASK))
! #define IndexTupleHasNulls(itup)     ((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK))
! #define IndexTupleHasVarlenas(itup)        ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))

! #define IndexTupleHasMinHeader(itup) (!IndexTupleHasNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
***************
*** 107,113 ****
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     IndexTupleNoNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
--- 108,114 ----
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     !IndexTupleHasNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
Index: src/include/access/nbtree.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/nbtree.h,v
retrieving revision 1.52
diff -c -w -r1.52 nbtree.h
*** src/include/access/nbtree.h    2001/02/21 19:07:04    1.52
--- src/include/access/nbtree.h    2001/02/22 00:27:59
***************
*** 50,56 ****


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \
--- 50,56 ----


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162    /* magic number of btree pages */

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \

>
> I don't think it's a good idea to fill bit 13 by force.
> There's only 1 bit unused. IMHO there must be a discussion
> about how to use the bit.

OK, attached is a patch that just documents the bit as unused and
changes some poorly chosen macro names.  Do people want this committed
to the current tree?  Seems there is some interest in updating this area
of the code.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/access/common/indextuple.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/common/indextuple.c,v
retrieving revision 1.51
diff -c -w -r1.51 indextuple.c
*** src/backend/access/common/indextuple.c    2001/02/15 20:57:01    1.51
--- src/backend/access/common/indextuple.c    2001/02/22 02:17:56
***************
*** 230,236 ****

      attnum--;

!     if (IndexTupleNoNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
--- 230,236 ----

      attnum--;

!     if (!IndexTupleHasNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
***************
*** 301,307 ****
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (!IndexTupleAllFixed(tup))
          {
              int            j;

--- 301,307 ----
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (IndexTupleHasVarlenas(tup))
          {
              int            j;

***************
*** 365,371 ****

          for (i = 0; i < attnum; i++)
          {
!             if (!IndexTupleNoNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
--- 365,371 ----

          for (i = 0; i < attnum; i++)
          {
!             if (IndexTupleHasNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
Index: src/backend/access/gist/gist.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/gist/gist.c,v
retrieving revision 1.69
diff -c -w -r1.69 gist.c
*** src/backend/access/gist/gist.c    2001/01/29 00:39:12    1.69
--- src/backend/access/gist/gist.c    2001/02/22 02:17:57
***************
*** 1102,1108 ****
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= 0xe000;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

--- 1102,1108 ----
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= ~INDEX_SIZE_MASK;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

Index: src/backend/access/hash/hash.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hash.c,v
retrieving revision 1.48
diff -c -w -r1.48 hash.c
*** src/backend/access/hash/hash.c    2001/01/29 00:39:13    1.48
--- src/backend/access/hash/hash.c    2001/02/22 02:17:57
***************
*** 170,176 ****
           * of the way nulls are handled here.
           */

!         if (itup->t_info & INDEX_NULL_MASK)
          {
              pfree(itup);
              continue;
--- 170,176 ----
           * of the way nulls are handled here.
           */

!         if (IndexTupleHasNulls(itup))
          {
              pfree(itup);
              continue;
***************
*** 256,262 ****
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (itup->t_info & INDEX_NULL_MASK)
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
--- 256,262 ----
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (IndexTupleHasNulls(itup))
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.25
diff -c -w -r1.25 hashutil.c
*** src/backend/access/hash/hashutil.c    2001/01/24 19:42:47    1.25
--- src/backend/access/hash/hashutil.c    2001/02/22 02:17:57
***************
*** 72,78 ****
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (itup->t_info & INDEX_NULL_MASK)
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
--- 72,78 ----
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (IndexTupleHasNulls(itup))
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
Index: src/include/access/itup.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/itup.h,v
retrieving revision 1.29
diff -c -w -r1.29 itup.h
*** src/include/access/itup.h    2001/02/21 19:07:04    1.29
--- src/include/access/itup.h    2001/02/22 02:17:58
***************
*** 24,35 ****
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /*
       * t_info is layed out in the following fashion:
       *
!      * 15th (leftmost) bit: "has nulls" bit 14th bit: "has varlenas" bit 13th
!      * bit: "has rules" bit - (removed ay 11/94) bits 12-0 bit: size of
!      * tuple.
       */

      unsigned short t_info;        /* various info about tuple */
--- 24,37 ----
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /* ---------------
       * t_info is layed out in the following fashion:
       *
!      * 15th (high) bit: has nulls
!      * 14th bit: has varlenas
!      * 13th bit: unused
!      * 12-0 bit: size of tuple
!      * ---------------
       */

      unsigned short t_info;        /* various info about tuple */
***************
*** 69,81 ****
  #define INDEX_SIZE_MASK 0x1FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)    ((Size) (((IndexTuple) (itup))->t_info & 0x1FFF))
! #define IndexTupleDSize(itup)                  ((Size) ((itup).t_info & 0x1FFF))
! #define IndexTupleNoNulls(itup)  (!(((IndexTuple) (itup))->t_info & 0x8000))
! #define IndexTupleAllFixed(itup) (!(((IndexTuple) (itup))->t_info & 0x4000))

! #define IndexTupleHasMinHeader(itup) (IndexTupleNoNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
--- 71,84 ----
  #define INDEX_SIZE_MASK 0x1FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000
+ #define INDEX_UNUSED    0x2000

! #define IndexTupleSize(itup)        ((Size) (((IndexTuple) (itup))->t_info & INDEX_SIZE_MASK))
! #define IndexTupleDSize(itup)        ((Size) ((itup).t_info & INDEX_SIZE_MASK))
! #define IndexTupleHasNulls(itup)     ((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK))
! #define IndexTupleHasVarlenas(itup)        ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))

! #define IndexTupleHasMinHeader(itup) (!IndexTupleHasNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
***************
*** 107,113 ****
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     IndexTupleNoNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
--- 110,116 ----
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     !IndexTupleHasNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
Index: src/include/access/nbtree.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/nbtree.h,v
retrieving revision 1.52
diff -c -w -r1.52 nbtree.h
*** src/include/access/nbtree.h    2001/02/21 19:07:04    1.52
--- src/include/access/nbtree.h    2001/02/22 02:17:58
***************
*** 50,56 ****


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \
--- 50,56 ----


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162    /* magic number of btree pages */

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \

Bruce Momjian wrote:
>
> > > > > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >
> > I don't think it's a good idea to fill bit 13 by force.
> > There's only 1 bit unused. IMHO there must be a discussion
> > about how to use the bit.
>
> I am not doing anything to 7.1, just 7.2.  My patch is just an attempt
> to make the source accurate.

Hmm I've already been confused by your attempt.
For example, oops where's the discussion about changing
index tuple length limit ? etc ...
And I understand now that the original question was thrown
to Tom. I'll leave the choise to Tom.

Regards,
Hiroshi Inoue

> Bruce Momjian wrote:
> >
> > > > > > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > >
> > > I don't think it's a good idea to fill bit 13 by force.
> > > There's only 1 bit unused. IMHO there must be a discussion
> > > about how to use the bit.
> >
> > I am not doing anything to 7.1, just 7.2.  My patch is just an attempt
> > to make the source accurate.
>
> Hmm I've already been confused by your attempt.
> For example, oops where's the discussion about changing
> index tuple length limit ? etc ...
> And I understand now that the original question was thrown
> to Tom. I'll leave the choise to Tom.

The patch never intended to increase the index tuple length.  It was
only to better document how IndexTupleData is used.  Both Tom and I
agreed that the use of bits/contants/macros in itup.h was not idea, and
needed a little cleaning.  That's all the patch does.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> What happened to our discussion about keeping t_info bit 13 unused??

> I wasn't going to reserve it in the patch. I figured I would make all
> the items/flags match, and if someone wants to reserve it, it is easy to
> do in one place.  I imagine 7.2 is going to be dump/reload anyway so the
> decision can be made during development cycle.  I basically didn't want
> to leave a bit gap and leave it unnamed because it could cause
> confusion.

I object.  Strongly.  You are making a significant change without
discussion --- in fact, contrary to what discussion there has been.
This is not a "trivial cleanup".

            regards, tom lane

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> What happened to our discussion about keeping t_info bit 13 unused??
>
> > I wasn't going to reserve it in the patch. I figured I would make all
> > the items/flags match, and if someone wants to reserve it, it is easy to
> > do in one place.  I imagine 7.2 is going to be dump/reload anyway so the
> > decision can be made during development cycle.  I basically didn't want
> > to leave a bit gap and leave it unnamed because it could cause
> > confusion.
>
> I object.  Strongly.  You are making a significant change without
> discussion --- in fact, contrary to what discussion there has been.
> This is not a "trivial cleanup".

OK, attached is the patch.  I guess I don't understand how what I am
doing affects anything.

I realize you were commenting about the earlier patch that gave the 13th
bit to the length.  I now see the issue you were talking about was this
test:

    /*
     * Here we make sure that the size will fit in the field reserved for
     * it in t_info.
     */
    if ((size & INDEX_SIZE_MASK) != size)
        elog(ERROR, "index_formtuple: data takes %lu bytes, max is %d",
             (unsigned long) size, INDEX_SIZE_MASK);

I originally didn't realize that expanding the _storage_ space for the
index tuples actually allow storage of longer tuples.  I see that now,
and this is why I just mark the patch as UNUSED.  I will let others
handle it.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/backend/access/common/indextuple.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/common/indextuple.c,v
retrieving revision 1.51
diff -c -w -r1.51 indextuple.c
*** src/backend/access/common/indextuple.c    2001/02/15 20:57:01    1.51
--- src/backend/access/common/indextuple.c    2001/02/22 03:23:17
***************
*** 230,236 ****

      attnum--;

!     if (IndexTupleNoNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
--- 230,236 ----

      attnum--;

!     if (!IndexTupleHasNulls(tup))
      {
  #ifdef IN_MACRO
  /* This is handled in the macro */
***************
*** 301,307 ****
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (!IndexTupleAllFixed(tup))
          {
              int            j;

--- 301,307 ----
              return fetchatt(att[attnum],
                              tp + att[attnum]->attcacheoff);
          }
!         else if (IndexTupleHasVarlenas(tup))
          {
              int            j;

***************
*** 365,371 ****

          for (i = 0; i < attnum; i++)
          {
!             if (!IndexTupleNoNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
--- 365,371 ----

          for (i = 0; i < attnum; i++)
          {
!             if (IndexTupleHasNulls(tup))
              {
                  if (att_isnull(i, bp))
                  {
Index: src/backend/access/gist/gist.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/gist/gist.c,v
retrieving revision 1.69
diff -c -w -r1.69 gist.c
*** src/backend/access/gist/gist.c    2001/01/29 00:39:12    1.69
--- src/backend/access/gist/gist.c    2001/02/22 03:23:17
***************
*** 1102,1108 ****
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= 0xe000;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

--- 1102,1108 ----
      {
          memcpy(datum, entry.pred, entry.bytes);
          /* clear out old size */
!         t->t_info &= ~INDEX_SIZE_MASK;
          /* or in new size */
          t->t_info |= MAXALIGN(entry.bytes + sizeof(IndexTupleData));

Index: src/backend/access/hash/hash.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hash.c,v
retrieving revision 1.48
diff -c -w -r1.48 hash.c
*** src/backend/access/hash/hash.c    2001/01/29 00:39:13    1.48
--- src/backend/access/hash/hash.c    2001/02/22 03:23:18
***************
*** 170,176 ****
           * of the way nulls are handled here.
           */

!         if (itup->t_info & INDEX_NULL_MASK)
          {
              pfree(itup);
              continue;
--- 170,176 ----
           * of the way nulls are handled here.
           */

!         if (IndexTupleHasNulls(itup))
          {
              pfree(itup);
              continue;
***************
*** 256,262 ****
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (itup->t_info & INDEX_NULL_MASK)
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
--- 256,262 ----
      itup = index_formtuple(RelationGetDescr(rel), datum, nulls);
      itup->t_tid = *ht_ctid;

!     if (IndexTupleHasNulls(itup))
          PG_RETURN_POINTER((InsertIndexResult) NULL);

      hitem = _hash_formitem(itup);
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.25
diff -c -w -r1.25 hashutil.c
*** src/backend/access/hash/hashutil.c    2001/01/24 19:42:47    1.25
--- src/backend/access/hash/hashutil.c    2001/02/22 03:23:18
***************
*** 72,78 ****
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (itup->t_info & INDEX_NULL_MASK)
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
--- 72,78 ----
      Size        tuplen;

      /* disallow nulls in hash keys */
!     if (IndexTupleHasNulls(itup))
          elog(ERROR, "hash indices cannot include null keys");

      /* make a copy of the index tuple with room for the sequence number */
Index: src/include/access/itup.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/itup.h,v
retrieving revision 1.29
diff -c -w -r1.29 itup.h
*** src/include/access/itup.h    2001/02/21 19:07:04    1.29
--- src/include/access/itup.h    2001/02/22 03:23:22
***************
*** 24,35 ****
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /*
       * t_info is layed out in the following fashion:
       *
!      * 15th (leftmost) bit: "has nulls" bit 14th bit: "has varlenas" bit 13th
!      * bit: "has rules" bit - (removed ay 11/94) bits 12-0 bit: size of
!      * tuple.
       */

      unsigned short t_info;        /* various info about tuple */
--- 24,37 ----
  {
      ItemPointerData t_tid;        /* reference TID to heap tuple */

!     /* ---------------
       * t_info is layed out in the following fashion:
       *
!      * 15th (high) bit: has nulls
!      * 14th bit: has varlenas
!      * 13th bit: unused
!      * 12-0 bit: size of tuple
!      * ---------------
       */

      unsigned short t_info;        /* various info about tuple */
***************
*** 69,81 ****
  #define INDEX_SIZE_MASK 0x1FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000

! #define IndexTupleSize(itup)    ((Size) (((IndexTuple) (itup))->t_info & 0x1FFF))
! #define IndexTupleDSize(itup)                  ((Size) ((itup).t_info & 0x1FFF))
! #define IndexTupleNoNulls(itup)  (!(((IndexTuple) (itup))->t_info & 0x8000))
! #define IndexTupleAllFixed(itup) (!(((IndexTuple) (itup))->t_info & 0x4000))

! #define IndexTupleHasMinHeader(itup) (IndexTupleNoNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
--- 71,84 ----
  #define INDEX_SIZE_MASK 0x1FFF
  #define INDEX_NULL_MASK 0x8000
  #define INDEX_VAR_MASK    0x4000
+ #define INDEX_UNUSED    0x2000

! #define IndexTupleSize(itup)        ((Size) (((IndexTuple) (itup))->t_info & INDEX_SIZE_MASK))
! #define IndexTupleDSize(itup)        ((Size) ((itup).t_info & INDEX_SIZE_MASK))
! #define IndexTupleHasNulls(itup)     ((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK))
! #define IndexTupleHasVarlenas(itup)    ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))

! #define IndexTupleHasMinHeader(itup) (!IndexTupleHasNulls(itup))

  /*
   * Takes an infomask as argument (primarily because this needs to be usable
***************
*** 107,113 ****
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     IndexTupleNoNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
--- 110,116 ----
  ( \
      AssertMacro(PointerIsValid(isnull) && (attnum) > 0), \
      *(isnull) = false, \
!     !IndexTupleHasNulls(tup) ? \
      ( \
          (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
          ( \
Index: src/include/access/nbtree.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/access/nbtree.h,v
retrieving revision 1.52
diff -c -w -r1.52 nbtree.h
*** src/include/access/nbtree.h    2001/02/21 19:07:04    1.52
--- src/include/access/nbtree.h    2001/02/22 03:23:23
***************
*** 50,56 ****


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \
--- 50,56 ----


  #define BTREE_METAPAGE    0    /* first page is meta */
! #define BTREE_MAGIC        0x053162    /* magic number of btree pages */

  #define BTreeInvalidParent(opaque)    \
      (opaque->btpo_parent == InvalidBlockNumber || \

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > The patch never intended to increase the index tuple length.  It was
> > only to better document how IndexTupleData is used.  Both Tom and I
> > agreed that the use of bits/contants/macros in itup.h was not idea, and
> > needed a little cleaning.  That's all the patch does.
>
> The original version of the patch commandeered an extra bit for tuple
> length.  If you back off INDEX_SIZE_MASK to 1FFF, and document bit
> 13 as unused/reserved, then it's just a cleanup.

OK, we are both catching up now on the email.  Should I put it in
current?  Seems like cosmetic cleanup.  Of course, even if you say yes,
I have to wait 24 hours.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> The patch never intended to increase the index tuple length.  It was
> only to better document how IndexTupleData is used.  Both Tom and I
> agreed that the use of bits/contants/macros in itup.h was not idea, and
> needed a little cleaning.  That's all the patch does.

The original version of the patch commandeered an extra bit for tuple
length.  If you back off INDEX_SIZE_MASK to 1FFF, and document bit
13 as unused/reserved, then it's just a cleanup.

            regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> The original version of the patch commandeered an extra bit for tuple
>> length.  If you back off INDEX_SIZE_MASK to 1FFF, and document bit
>> 13 as unused/reserved, then it's just a cleanup.

> OK, we are both catching up now on the email.  Should I put it in
> current?  Seems like cosmetic cleanup.  Of course, even if you say yes,
> I have to wait 24 hours.

In the revised form I have no problem with it.

            regards, tom lane