Lists: | pgsql-hackers |
---|
From: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
---|---|
To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Toast bug in CVS HEAD |
Date: | 2008-11-05 19:49:08 |
Message-ID: | 4911F8B4.3030301@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
A bug was introduced a while ago by this patch:
> commit 447f7364dd7227a32b58a2aff24f587dd7d7051a
> Author: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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
Attachment | Content-Type | Size |
---|---|---|
toast-fix.patch | text/x-diff | 2.2 KB |
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Toast bug in CVS HEAD |
Date: | 2008-11-05 20:41:50 |
Message-ID: | 2486.1225917710@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Basically, this comment and code in genam.c:
> ...
> 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..
Huh, can't believe I missed that that caller might use non-sequential
column numbers.
It's kind of annoying to introduce a search when it's so seldom needed,
though. How about something like
/* fast path for common case */
if (key[i].sk_attno == irel->rd_index->indkey.values[i])
key[i].sk_attno = i + 1;
else
... search as you have it ...
regards, tom lane
From: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Toast bug in CVS HEAD |
Date: | 2008-11-05 20:46:50 |
Message-ID: | 4912063A.7060803@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> It's kind of annoying to introduce a search when it's so seldom needed,
> though. How about something like
>
> /* fast path for common case */
> if (key[i].sk_attno == irel->rd_index->indkey.values[i])
> key[i].sk_attno = i + 1;
> else
> ... search as you have it ...
I doubt it's worth it, given that there's only a couple of columns in
the index and in the scan key anyway.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com