Zdenek Kotala <[EMAIL PROTECTED]> 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

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Reply via email to