Toast bug in CVS HEAD - Mailing list pgsql-hackers

A bug was introduced a while ago by this patch:

 > commit 447f7364dd7227a32b58a2aff24f587dd7d7051a
 > Author: Tom Lane <tgl@sss.pgh.pa.us>
 > Date:   Sat Apr 12 23:14:21 2008 +0000
 >
 >     Create new routines systable_beginscan_ordered,
systable_getnext_ordered,
 >     systable_endscan_ordered that have API similar to
systable_beginscan etc
 >     (in particular, the passed-in scankeys have heap not index attnums),
 >     but guarantee ordered output, unlike the existing functions.  For
the moment
 >     these are just very thin wrappers around
index_beginscan/index_getnext/etc.
 >     Someday they might need to get smarter; but for now this is just
a code
 >     refactoring exercise to reduce the number of direct callers of
index_getnext
 >     in preparation for changing that function's API.
 >
 >     In passing, remove index_getnext_indexitem, which has been dead
code for
 >     quite some time, and will have even less use than that in the
presence
 >     of run-time-lossy indexes.

You get this assertion failure:

TRAP: FailedAssertion("!(key[i].sk_attno ==
indexRelation->rd_index->indkey.values[i])", File: "genam.c", Line: 363)

with this test case:

CREATE DATABASE vdb WITH ENCODING = 'SQL_ASCII';
\c vdb
CREATE TABLE xtable(x text);

-- we must do this because otherwise the strings are too compressible
and don't
-- get toasted externally

ALTER TABLE xtable ALTER x SET STORAGE EXTERNAL;

---retrieving a substr(toasteddatum,x,y) where where x .. x+y spans two
chunks.
INSERT INTO xtable  (SELECT REPEAT('ABCDEFGHIJ',400));
SELECT substr(x,1000,2000) from xtable ;

Basically, this comment and code in genam.c:
> !               /*
> !                * Change attribute numbers to be index column numbers.
> !                *
> !                * This code could be generalized to search for the index key numbers
> !                * to substitute, but for now there's no need.
> !                */
>                 for (i = 0; i < nkeys; i++)
>                 {
> !                       Assert(key[i].sk_attno == irel->rd_index->indkey.values[i]);
> !                       key[i].sk_attno = i + 1;
>                 }

is wrong, because it assumes that there's only one scankey per index
column, but that's not true for toast_fetch_datum_slice(), which uses
two scankeys for the chunkid, to fetch a range. Attached is a patch to
fix that, as suggested in the comment. Comments? I'll apply if not..

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
*** src/backend/access/index/genam.c
--- src/backend/access/index/genam.c
***************
*** 194,209 **** systable_beginscan(Relation heapRelation,
      {
          int            i;

!         /*
!          * Change attribute numbers to be index column numbers.
!          *
!          * This code could be generalized to search for the index key numbers
!          * to substitute, but for now there's no need.
!          */
          for (i = 0; i < nkeys; i++)
          {
!             Assert(key[i].sk_attno == irel->rd_index->indkey.values[i]);
!             key[i].sk_attno = i + 1;
          }

          sysscan->iscan = index_beginscan(heapRelation, irel,
--- 194,214 ----
      {
          int            i;

!         /* Change attribute numbers to be index column numbers. */
          for (i = 0; i < nkeys; i++)
          {
!             int j;
!
!             for (j = 0; j < irel->rd_index->indnatts; j++)
!             {
!                 if (key[i].sk_attno == irel->rd_index->indkey.values[j])
!                 {
!                     key[i].sk_attno = j + 1;
!                     break;
!                 }
!             }
!             if (j == irel->rd_index->indnatts)
!                 elog(ERROR, "column is not in index");
          }

          sysscan->iscan = index_beginscan(heapRelation, irel,
***************
*** 352,367 **** systable_beginscan_ordered(Relation heapRelation,
      sysscan->heap_rel = heapRelation;
      sysscan->irel = indexRelation;

!     /*
!      * Change attribute numbers to be index column numbers.
!      *
!      * This code could be generalized to search for the index key numbers
!      * to substitute, but for now there's no need.
!      */
      for (i = 0; i < nkeys; i++)
      {
!         Assert(key[i].sk_attno == indexRelation->rd_index->indkey.values[i]);
!         key[i].sk_attno = i + 1;
      }

      sysscan->iscan = index_beginscan(heapRelation, indexRelation,
--- 357,377 ----
      sysscan->heap_rel = heapRelation;
      sysscan->irel = indexRelation;

!     /* Change attribute numbers to be index column numbers. */
      for (i = 0; i < nkeys; i++)
      {
!         int j;
!
!         for (j = 0; j < indexRelation->rd_index->indnatts; j++)
!         {
!             if (key[i].sk_attno == indexRelation->rd_index->indkey.values[j])
!             {
!                 key[i].sk_attno = j + 1;
!                 break;
!             }
!         }
!         if (j == indexRelation->rd_index->indnatts)
!             elog(ERROR, "column is not in index");
      }

      sysscan->iscan = index_beginscan(heapRelation, indexRelation,