Commit aaff0a55 authored by Tom Lane's avatar Tom Lane

Clean up a couple of problems in crosstab_hash's use of a hash table.

The original coding leaked memory (at least 8K per crosstab_hash call)
because it allowed the hash table to be allocated as a child of
TopMemoryContext and then never freed it.  Fix that by putting the
hash table under per_query_ctx, instead.  Also get rid of use
of a static variable to point to the hash table.  Aside from being
ugly, that would actively do the wrong thing in the case of re-entrant
calls to crosstab_hash, which are at least theoretically possible
since it was expecting the static variable to stay valid across
a SPI_execute call.
parent cac82bb2
...@@ -44,9 +44,9 @@ ...@@ -44,9 +44,9 @@
PG_MODULE_MAGIC; PG_MODULE_MAGIC;
static int load_categories_hash(char *cats_sql, MemoryContext per_query_ctx); static HTAB *load_categories_hash(char *cats_sql, MemoryContext per_query_ctx);
static Tuplestorestate *get_crosstab_tuplestore(char *sql, static Tuplestorestate *get_crosstab_tuplestore(char *sql,
int num_categories, HTAB *crosstab_hash,
TupleDesc tupdesc, TupleDesc tupdesc,
MemoryContext per_query_ctx); MemoryContext per_query_ctx);
static void validateConnectbyTupleDesc(TupleDesc tupdesc, bool show_branch, bool show_serial); static void validateConnectbyTupleDesc(TupleDesc tupdesc, bool show_branch, bool show_serial);
...@@ -121,26 +121,23 @@ typedef struct ...@@ -121,26 +121,23 @@ typedef struct
/* sign, 10 digits, '\0' */ /* sign, 10 digits, '\0' */
#define INT32_STRLEN 12 #define INT32_STRLEN 12
/* hash table support */ /* stored info for a crosstab category */
static HTAB *crosstab_HashTable;
/* The information we cache about loaded procedures */
typedef struct crosstab_cat_desc typedef struct crosstab_cat_desc
{ {
char *catname; char *catname; /* full category name */
int attidx; /* zero based */ int attidx; /* zero based */
} crosstab_cat_desc; } crosstab_cat_desc;
#define MAX_CATNAME_LEN NAMEDATALEN #define MAX_CATNAME_LEN NAMEDATALEN
#define INIT_CATS 64 #define INIT_CATS 64
#define crosstab_HashTableLookup(CATNAME, CATDESC) \ #define crosstab_HashTableLookup(HASHTAB, CATNAME, CATDESC) \
do { \ do { \
crosstab_HashEnt *hentry; char key[MAX_CATNAME_LEN]; \ crosstab_HashEnt *hentry; char key[MAX_CATNAME_LEN]; \
\ \
MemSet(key, 0, MAX_CATNAME_LEN); \ MemSet(key, 0, MAX_CATNAME_LEN); \
snprintf(key, MAX_CATNAME_LEN - 1, "%s", CATNAME); \ snprintf(key, MAX_CATNAME_LEN - 1, "%s", CATNAME); \
hentry = (crosstab_HashEnt*) hash_search(crosstab_HashTable, \ hentry = (crosstab_HashEnt*) hash_search(HASHTAB, \
key, HASH_FIND, NULL); \ key, HASH_FIND, NULL); \
if (hentry) \ if (hentry) \
CATDESC = hentry->catdesc; \ CATDESC = hentry->catdesc; \
...@@ -148,13 +145,13 @@ do { \ ...@@ -148,13 +145,13 @@ do { \
CATDESC = NULL; \ CATDESC = NULL; \
} while(0) } while(0)
#define crosstab_HashTableInsert(CATDESC) \ #define crosstab_HashTableInsert(HASHTAB, CATDESC) \
do { \ do { \
crosstab_HashEnt *hentry; bool found; char key[MAX_CATNAME_LEN]; \ crosstab_HashEnt *hentry; bool found; char key[MAX_CATNAME_LEN]; \
\ \
MemSet(key, 0, MAX_CATNAME_LEN); \ MemSet(key, 0, MAX_CATNAME_LEN); \
snprintf(key, MAX_CATNAME_LEN - 1, "%s", CATDESC->catname); \ snprintf(key, MAX_CATNAME_LEN - 1, "%s", CATDESC->catname); \
hentry = (crosstab_HashEnt*) hash_search(crosstab_HashTable, \ hentry = (crosstab_HashEnt*) hash_search(HASHTAB, \
key, HASH_ENTER, &found); \ key, HASH_ENTER, &found); \
if (found) \ if (found) \
ereport(ERROR, \ ereport(ERROR, \
...@@ -704,7 +701,7 @@ crosstab_hash(PG_FUNCTION_ARGS) ...@@ -704,7 +701,7 @@ crosstab_hash(PG_FUNCTION_ARGS)
TupleDesc tupdesc; TupleDesc tupdesc;
MemoryContext per_query_ctx; MemoryContext per_query_ctx;
MemoryContext oldcontext; MemoryContext oldcontext;
int num_categories; HTAB *crosstab_hash;
/* check to see if caller supports us returning a tuplestore */ /* check to see if caller supports us returning a tuplestore */
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
...@@ -737,14 +734,14 @@ crosstab_hash(PG_FUNCTION_ARGS) ...@@ -737,14 +734,14 @@ crosstab_hash(PG_FUNCTION_ARGS)
"crosstab function are not compatible"))); "crosstab function are not compatible")));
/* load up the categories hash table */ /* load up the categories hash table */
num_categories = load_categories_hash(cats_sql, per_query_ctx); crosstab_hash = load_categories_hash(cats_sql, per_query_ctx);
/* let the caller know we're sending back a tuplestore */ /* let the caller know we're sending back a tuplestore */
rsinfo->returnMode = SFRM_Materialize; rsinfo->returnMode = SFRM_Materialize;
/* now go build it */ /* now go build it */
rsinfo->setResult = get_crosstab_tuplestore(sql, rsinfo->setResult = get_crosstab_tuplestore(sql,
num_categories, crosstab_hash,
tupdesc, tupdesc,
per_query_ctx); per_query_ctx);
...@@ -764,24 +761,29 @@ crosstab_hash(PG_FUNCTION_ARGS) ...@@ -764,24 +761,29 @@ crosstab_hash(PG_FUNCTION_ARGS)
/* /*
* load up the categories hash table * load up the categories hash table
*/ */
static int static HTAB *
load_categories_hash(char *cats_sql, MemoryContext per_query_ctx) load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
{ {
HTAB *crosstab_hash;
HASHCTL ctl; HASHCTL ctl;
int ret; int ret;
int proc; int proc;
MemoryContext SPIcontext; MemoryContext SPIcontext;
int num_categories = 0;
/* initialize the category hash table */ /* initialize the category hash table */
MemSet(&ctl, 0, sizeof(ctl));
ctl.keysize = MAX_CATNAME_LEN; ctl.keysize = MAX_CATNAME_LEN;
ctl.entrysize = sizeof(crosstab_HashEnt); ctl.entrysize = sizeof(crosstab_HashEnt);
ctl.hcxt = per_query_ctx;
/* /*
* use INIT_CATS, defined above as a guess of how many hash table entries * use INIT_CATS, defined above as a guess of how many hash table entries
* to create, initially * to create, initially
*/ */
crosstab_HashTable = hash_create("crosstab hash", INIT_CATS, &ctl, HASH_ELEM); crosstab_hash = hash_create("crosstab hash",
INIT_CATS,
&ctl,
HASH_ELEM | HASH_CONTEXT);
/* Connect to SPI manager */ /* Connect to SPI manager */
if ((ret = SPI_connect()) < 0) if ((ret = SPI_connect()) < 0)
...@@ -790,7 +792,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx) ...@@ -790,7 +792,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
/* Retrieve the category name rows */ /* Retrieve the category name rows */
ret = SPI_execute(cats_sql, true, 0); ret = SPI_execute(cats_sql, true, 0);
num_categories = proc = SPI_processed; proc = SPI_processed;
/* Check for qualifying tuples */ /* Check for qualifying tuples */
if ((ret == SPI_OK_SELECT) && (proc > 0)) if ((ret == SPI_OK_SELECT) && (proc > 0))
...@@ -828,7 +830,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx) ...@@ -828,7 +830,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
catdesc->attidx = i; catdesc->attidx = i;
/* Add the proc description block to the hashtable */ /* Add the proc description block to the hashtable */
crosstab_HashTableInsert(catdesc); crosstab_HashTableInsert(crosstab_hash, catdesc);
MemoryContextSwitchTo(SPIcontext); MemoryContextSwitchTo(SPIcontext);
} }
...@@ -838,7 +840,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx) ...@@ -838,7 +840,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
/* internal error */ /* internal error */
elog(ERROR, "load_categories_hash: SPI_finish() failed"); elog(ERROR, "load_categories_hash: SPI_finish() failed");
return num_categories; return crosstab_hash;
} }
/* /*
...@@ -846,11 +848,12 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx) ...@@ -846,11 +848,12 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
*/ */
static Tuplestorestate * static Tuplestorestate *
get_crosstab_tuplestore(char *sql, get_crosstab_tuplestore(char *sql,
int num_categories, HTAB *crosstab_hash,
TupleDesc tupdesc, TupleDesc tupdesc,
MemoryContext per_query_ctx) MemoryContext per_query_ctx)
{ {
Tuplestorestate *tupstore; Tuplestorestate *tupstore;
int num_categories = hash_get_num_entries(crosstab_hash);
AttInMetadata *attinmeta = TupleDescGetAttInMetadata(tupdesc); AttInMetadata *attinmeta = TupleDescGetAttInMetadata(tupdesc);
char **values; char **values;
HeapTuple tuple; HeapTuple tuple;
...@@ -978,7 +981,7 @@ get_crosstab_tuplestore(char *sql, ...@@ -978,7 +981,7 @@ get_crosstab_tuplestore(char *sql,
if (catname != NULL) if (catname != NULL)
{ {
crosstab_HashTableLookup(catname, catdesc); crosstab_HashTableLookup(crosstab_hash, catname, catdesc);
if (catdesc) if (catdesc)
values[catdesc->attidx + ncols - 2] = values[catdesc->attidx + ncols - 2] =
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment