Re: hash index improving v3

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM>
Cc: Xiao Meng <mx(dot)cogito(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org, "Jonah H(dot) Harris" <jonah(dot)harris(at)gmail(dot)com>
Subject: Re: hash index improving v3
Date: 2008-09-04 20:28:34
Message-ID: 29987.1220560114@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> pgsql/src/backend/access/hash/hashutil.c
> <http://reviewdemo.postgresql.org/r/26/#comment27>

> It would be better remove #define from hash.h and setup it there
> directly.

Actually, I don't like this aspect of the patch one bit: it means that
the system catalogs are lying about what is stored in the index, which
seems likely to break something somewhere, either now or down the road.
I think the correct way to handle this is to make the pg_attribute entries
(and hence the index's relation descriptor) accurately match what is
stored in the index. For testing purposes I propose this crude hack
in catalog/index.c's ConstructTupleDescriptor():

*** src/backend/catalog/index.c.orig Mon Aug 25 18:42:32 2008
--- src/backend/catalog/index.c Thu Sep 4 16:20:12 2008
***************
*** 133,138 ****
--- 133,139 ----
Form_pg_attribute to = indexTupDesc->attrs[i];
HeapTuple tuple;
Form_pg_type typeTup;
+ Form_pg_opclass opclassTup;
Oid keyType;

if (atnum != 0)
***************
*** 240,246 ****
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for opclass %u",
classObjectId[i]);
! keyType = ((Form_pg_opclass) GETSTRUCT(tuple))->opckeytype;
ReleaseSysCache(tuple);

if (OidIsValid(keyType) && keyType != to->atttypid)
--- 241,252 ----
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for opclass %u",
classObjectId[i]);
! opclassTup = (Form_pg_opclass) GETSTRUCT(tuple);
! /* HACK: make hash always use int4 as storage (really it's uint32) */
! if (opclassTup->opcmethod == HASH_AM_OID)
! keyType = INT4OID;
! else
! keyType = opclassTup->opckeytype;
ReleaseSysCache(tuple);

if (OidIsValid(keyType) && keyType != to->atttypid)

Assuming the patch gets accepted, we should devise some cleaner way
of letting index AMs adjust their indexes' reldescs; maybe declare a
new entry point column in pg_am that lets the AM modify the tupledesc
constructed by this function before it gets used to create the index.
But that is irrelevant to performance testing, so I'm not going to do
it right now.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2008-09-04 20:54:02 Re: Need more reviewers!
Previous Message Tom Lane 2008-09-04 20:06:23 Re: hash index improving v3

Browse pgsql-patches by date

  From Date Subject
Next Message Heikki Linnakangas 2008-09-04 22:31:15 Re: [HACKERS] TODO item: Implement Boyer-Moore searching (First time hacker)
Previous Message Tom Lane 2008-09-04 20:06:23 Re: hash index improving v3