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
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)
  
  
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to