Received: from localhost (unknown [200.46.204.183]) by postgresql.org (Postfix) with ESMTP id 071B864FC51 for ; Thu, 25 Sep 2008 13:27:05 -0300 (ADT) Received: from postgresql.org ([200.46.204.86]) by localhost (mx1.hub.org [200.46.204.183]) (amavisd-maia, port 10024) with ESMTP id 49469-03-5 for ; Thu, 25 Sep 2008 13:27:00 -0300 (ADT) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from outmail137175.authsmtp.co.uk (outmail137175.authsmtp.co.uk [62.13.137.175]) by postgresql.org (Postfix) with ESMTP id 2566164FC4A for ; Thu, 25 Sep 2008 13:26:59 -0300 (ADT) Received: from mail-c188.authsmtp.com (mail-c188.authsmtp.com [62.13.128.25]) by punt6.authsmtp.com (8.14.2/8.14.2/Kp) with ESMTP id m8PGQKVE096412; Thu, 25 Sep 2008 17:26:20 +0100 (BST) Received: from [192.168.0.3] (85-211-71-126.dyn.gotadsl.co.uk [85.211.71.126]) (authenticated bits=0) by mail.authsmtp.com (8.14.2/8.14.2/Kp) with ESMTP id m8PGQIbD020203; Thu, 25 Sep 2008 17:26:18 +0100 (BST) Subject: Re: get_relation_stats_hook() From: Simon Riggs To: Gregory Stark Cc: Postgres In-Reply-To: <87fxnsoz6r.fsf@oxford.xeocode.com> References: <1218062303.4549.302.camel@ebony.2ndQuadrant> <87fxnsoz6r.fsf@oxford.xeocode.com> Content-Type: multipart/mixed; boundary="=-nBCz7VtAR1fNYAlXiSBq" Date: Thu, 25 Sep 2008 17:27:21 +0100 Message-Id: <1222360041.4445.809.camel@ebony.2ndQuadrant> Mime-Version: 1.0 X-Mailer: Evolution 2.12.0 X-Server-Quench: af01b3ba-8b1e-11dd-aecc-001871e930f4 X-AuthRoute: OCdxZQATClZOTQEd DAteCiNZVAwpPBRK HVkIKg5MJUcNSQVJ NksadBtFagBbflxB HGQLW1xEUVp7W2V/ agMfZQBDYEtPQQJr TklLRE1QEQdtHhxP WxZ+JXgLJXFGen5y ZUUsX3BdXEw0JBB9 QxxXRzwbbTVhPX0e URQMalIBI1BXfh9E Yld+AXoOYTMDKBg1 TUcAHxkaHhhlExEd TQwRGkgfRU8XHzk6 cAseFCk0Kk0CQCZz bnQ9 X-Authentic-SMTP: 61633235383639.cat.dmpriest.net.uk:1378/Kp X-Report-SPAM: If SPAM / abuse - report it at: http://www.authsmtp.com/abuse X-Virus-Status: No virus detected - but ensure you scan with your own anti-virus system! X-Virus-Scanned: Maia Mailguard 1.0.1 X-Spam-Status: No, hits=0 tagged_above=0 required=5 tests=none X-Spam-Level: X-Archive-Number: 200809/1649 X-Sequence-Number: 124778 --=-nBCz7VtAR1fNYAlXiSBq Content-Type: text/plain Content-Transfer-Encoding: 7bit On Mon, 2008-09-22 at 18:41 +0100, Gregory Stark wrote: > The easiest way to fix this seems like also the best way, instead of storing a > boolean store the pointer to the release function. Implemented as suggested. v5 enclosed. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support --=-nBCz7VtAR1fNYAlXiSBq Content-Disposition: attachment; filename=stat_hooks.v5.patch Content-Type: text/x-patch; name=stat_hooks.v5.patch; charset=utf-8 Content-Transfer-Encoding: 7bit Index: src/backend/utils/adt/selfuncs.c =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/utils/adt/selfuncs.c,v retrieving revision 1.253 diff -c -r1.253 selfuncs.c *** src/backend/utils/adt/selfuncs.c 25 Aug 2008 22:42:34 -0000 1.253 --- src/backend/utils/adt/selfuncs.c 25 Sep 2008 15:57:58 -0000 *************** *** 118,123 **** --- 118,126 ---- #include "utils/selfuncs.h" #include "utils/syscache.h" + /* Hook for plugins to get control when we ask for stats */ + get_relation_stats_hook_type get_relation_stats_hook = NULL; + release_relation_stats_hook_type release_relation_stats_hook = NULL; static double var_eq_const(VariableStatData *vardata, Oid operator, Datum constval, bool constisnull, *************** *** 3996,4005 **** } else if (rte->rtekind == RTE_RELATION) { ! vardata->statsTuple = SearchSysCache(STATRELATT, ObjectIdGetDatum(rte->relid), Int16GetDatum(var->varattno), 0, 0); } else { --- 3999,4017 ---- } else if (rte->rtekind == RTE_RELATION) { ! if (get_relation_stats_hook) ! vardata->statsTuple = (*get_relation_stats_hook) ! (rte->relid, ! var->varattno, ! vardata->freefunc); ! if (!vardata->statsTuple) ! { ! vardata->statsTuple = SearchSysCache(STATRELATT, ObjectIdGetDatum(rte->relid), Int16GetDatum(var->varattno), 0, 0); + vardata->freefunc = ReleaseSysCache; + } } else { *************** *** 4116,4125 **** index->indpred == NIL) vardata->isunique = true; /* Has it got stats? */ ! vardata->statsTuple = SearchSysCache(STATRELATT, ObjectIdGetDatum(index->indexoid), ! Int16GetDatum(pos + 1), ! 0, 0); if (vardata->statsTuple) break; } --- 4128,4147 ---- index->indpred == NIL) vardata->isunique = true; /* Has it got stats? */ ! if (get_relation_stats_hook) ! vardata->statsTuple = (*get_relation_stats_hook) ! (index->indexoid, ! pos + 1, ! vardata->freefunc); ! if (!vardata->statsTuple) ! { ! vardata->statsTuple = SearchSysCache(STATRELATT, ObjectIdGetDatum(index->indexoid), ! Int16GetDatum(pos + 1), ! 0, 0); ! vardata->freefunc = ReleaseSysCache; ! } ! if (vardata->statsTuple) break; } *************** *** 5551,5557 **** double *indexCorrelation = (double *) PG_GETARG_POINTER(7); Oid relid; AttrNumber colnum; ! HeapTuple tuple; double numIndexTuples; List *indexBoundQuals; int indexcol; --- 5573,5580 ---- double *indexCorrelation = (double *) PG_GETARG_POINTER(7); Oid relid; AttrNumber colnum; ! HeapTuple tuple = NULL; ! void (*freefunc) (HeapTuple tuple) = NULL; double numIndexTuples; List *indexBoundQuals; int indexcol; *************** *** 5756,5765 **** colnum = 1; } ! tuple = SearchSysCache(STATRELATT, ! ObjectIdGetDatum(relid), ! Int16GetDatum(colnum), ! 0, 0); if (HeapTupleIsValid(tuple)) { --- 5779,5795 ---- colnum = 1; } ! if (get_relation_stats_hook) ! tuple = (*get_relation_stats_hook) (relid, colnum, freefunc); ! ! if (!tuple) ! { ! tuple = SearchSysCache(STATRELATT, ! ObjectIdGetDatum(relid), ! Int16GetDatum(colnum), ! 0, 0); ! freefunc = ReleaseSysCache; ! } if (HeapTupleIsValid(tuple)) { *************** *** 5800,5806 **** free_attstatsslot(InvalidOid, NULL, 0, numbers, nnumbers); } ! ReleaseSysCache(tuple); } PG_RETURN_VOID(); --- 5830,5837 ---- free_attstatsslot(InvalidOid, NULL, 0, numbers, nnumbers); } ! ! ReleaseStatsTuple(tuple, freefunc); } PG_RETURN_VOID(); Index: src/backend/utils/cache/lsyscache.c =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/utils/cache/lsyscache.c,v retrieving revision 1.159 diff -c -r1.159 lsyscache.c *** src/backend/utils/cache/lsyscache.c 2 Aug 2008 21:32:00 -0000 1.159 --- src/backend/utils/cache/lsyscache.c 25 Sep 2008 14:16:01 -0000 *************** *** 27,32 **** --- 27,33 ---- #include "catalog/pg_proc.h" #include "catalog/pg_statistic.h" #include "catalog/pg_type.h" + #include "optimizer/plancat.h" #include "miscadmin.h" #include "nodes/makefuncs.h" #include "utils/array.h" *************** *** 35,40 **** --- 36,43 ---- #include "utils/lsyscache.h" #include "utils/syscache.h" + /* Hook for plugins to get control in get_attavgwidth() */ + get_attavgwidth_hook_type get_attavgwidth_hook = NULL; /* ---------- AMOP CACHES ---------- */ *************** *** 2492,2507 **** * * Given the table and attribute number of a column, get the average * width of entries in the column. Return zero if no data available. */ int32 get_attavgwidth(Oid relid, AttrNumber attnum) { HeapTuple tp; tp = SearchSysCache(STATRELATT, ! ObjectIdGetDatum(relid), ! Int16GetDatum(attnum), ! 0, 0); if (HeapTupleIsValid(tp)) { int32 stawidth = ((Form_pg_statistic) GETSTRUCT(tp))->stawidth; --- 2495,2524 ---- * * Given the table and attribute number of a column, get the average * width of entries in the column. Return zero if no data available. + * + * Calling a hook at this point looks somewhat strange, but is required + * because the optimizer handles inheritance relations by calling for + * the avg width later in the planner than get_relation_info_hook(). + * So the APIs and call points of hooks must match the optimizer. */ int32 get_attavgwidth(Oid relid, AttrNumber attnum) { HeapTuple tp; + int32 stawidth; + + if (get_attavgwidth_hook) + { + stawidth = (*get_attavgwidth_hook) (relid, attnum); + if (stawidth > 0) + return stawidth; + } tp = SearchSysCache(STATRELATT, ! ObjectIdGetDatum(relid), ! Int16GetDatum(attnum), ! 0, 0); ! if (HeapTupleIsValid(tp)) { int32 stawidth = ((Form_pg_statistic) GETSTRUCT(tp))->stawidth; Index: src/include/optimizer/plancat.h =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/include/optimizer/plancat.h,v retrieving revision 1.51 diff -c -r1.51 plancat.h *** src/include/optimizer/plancat.h 16 Aug 2008 00:01:38 -0000 1.51 --- src/include/optimizer/plancat.h 25 Sep 2008 15:57:06 -0000 *************** *** 14,19 **** --- 14,20 ---- #ifndef PLANCAT_H #define PLANCAT_H + #include "access/htup.h" #include "nodes/relation.h" #include "utils/relcache.h" *************** *** 24,29 **** --- 25,43 ---- RelOptInfo *rel); extern PGDLLIMPORT get_relation_info_hook_type get_relation_info_hook; + /* Hooks for plugins to get control in lsyscache.c and selfuncs.c */ + typedef HeapTuple (*get_relation_stats_hook_type) (Oid relid, AttrNumber attnum, + void (*freefunc) (HeapTuple tuple)); + extern PGDLLIMPORT get_relation_stats_hook_type get_relation_stats_hook; + + /* must match ReleaseSysCache call signature */ + typedef void (*release_relation_stats_hook_type) (HeapTuple tuple); + extern PGDLLIMPORT release_relation_stats_hook_type release_relation_stats_hook; + + typedef int32 (*get_attavgwidth_hook_type) (Oid relid, AttrNumber attnum); + extern PGDLLIMPORT get_attavgwidth_hook_type get_attavgwidth_hook; + + extern void get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, RelOptInfo *rel); Index: src/include/utils/selfuncs.h =================================================================== RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/include/utils/selfuncs.h,v retrieving revision 1.46 diff -c -r1.46 selfuncs.h *** src/include/utils/selfuncs.h 16 Aug 2008 00:01:38 -0000 1.46 --- src/include/utils/selfuncs.h 25 Sep 2008 15:55:05 -0000 *************** *** 74,85 **** Oid atttype; /* type to pass to get_attstatsslot */ int32 atttypmod; /* typmod to pass to get_attstatsslot */ bool isunique; /* true if matched to a unique index */ } VariableStatData; #define ReleaseVariableStats(vardata) \ do { \ if (HeapTupleIsValid((vardata).statsTuple)) \ ! ReleaseSysCache((vardata).statsTuple); \ } while(0) --- 74,102 ---- Oid atttype; /* type to pass to get_attstatsslot */ int32 atttypmod; /* typmod to pass to get_attstatsslot */ bool isunique; /* true if matched to a unique index */ + void (*freefunc) (HeapTuple tuple); /* funct ptr to free statsTuple */ } VariableStatData; + #define ReleaseStatsTuple(tuple, freefunc) \ + do { \ + if (HeapTupleIsValid(tuple)) \ + { \ + if (freefunc) \ + freefunc(tuple); \ + else \ + elog(ERROR, "unable to release variable stats correctly"); \ + } \ + } while(0) + #define ReleaseVariableStats(vardata) \ do { \ if (HeapTupleIsValid((vardata).statsTuple)) \ ! { \ ! if ((vardata).freefunc) \ ! (vardata).freefunc((vardata).statsTuple); \ ! else \ ! elog(ERROR, "unable to release variable stats correctly"); \ ! } \ } while(0) --=-nBCz7VtAR1fNYAlXiSBq--