Skip site navigation (1) Skip section navigation (2)

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 (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-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

pgsql-hackers by date

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

pgsql-patches by date

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

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group