Re: [GENERAL] Many Pl/PgSQL parameters -> AllocSetAlloc(128)? - Mailing list pgsql-patches
From | Joe Conway |
---|---|
Subject | Re: [GENERAL] Many Pl/PgSQL parameters -> AllocSetAlloc(128)? |
Date | |
Msg-id | 3EF8DD37.5000806@joeconway.com Whole thread Raw |
Responses |
Re: [GENERAL] Many Pl/PgSQL parameters -> AllocSetAlloc(128)?
|
List | pgsql-patches |
(moving to patches) Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: >>Actually, adding a "pfree(oneres);" to the end of that for loop plugs >>the memory leak and allows me to see the error message: > > On second look, you can't pfree oneres at the bottom of > gen_cross_product() because it's part of the returned data structure > --- note the assignment > *iter++ = oneres; Yup, I see that now ;-) > I think the bug here is that gen_cross_product should be ignoring > argument positions that have nsupers == 0; those should always be > assigned the same type as the input, since the regular type resolution > algorithm is responsible for dealing with 'em. > > It might work to get rid of the "wild card" case (line 1115), which'd > reduce the number of outputs to product(nsupers+1). I doubt we really > want any wild cards in there anymore. > The above didn't work, but if I understand correctly what that function is intended to do, it seemed very broken. Basically this code: nanswers = 1; for (i = 0; i < nargs; i++) { nanswers *= (arginh[i].nsupers + 2); cur[i] = 0; } for 24 arguments means 2^24 answers, even when there are no superclasses. In that case (no arguments with superclasses), we ought to get: nanswers = 1 iter should be sized at 1(all arginh[i].self) + 1(all wildcard) + 1 (null terminator) The attached patch fixes this issue, but I'm still not entirely sure I understand what the function was supposed to be doing, so please check over my logic. Here's how I tested (after `make installcheck`) CREATE OR REPLACE FUNCTION inhtest (d, integer, stud_emp, integer) returns text as 'select ''OK''::text' language 'sql'; CREATE OR REPLACE FUNCTION get_d() returns d as 'select * from d limit 1' language 'sql'; CREATE OR REPLACE FUNCTION get_stud_emp() returns stud_emp as 'select * from stud_emp limit 1' language 'sql'; select inhtest(get_d(), 'a'::text, get_stud_emp(), 1); During testing, I had an elog(NOTICE,...) in there, and got the following (reformatted) results: arg num = 3 2 1 0 iter[j] NOTICE: j = 0 23 1764386 25 1881220 NOTICE: j = 1 23 1764391 25 1881220 NOTICE: j = 2 23 1764381 25 1881220 NOTICE: j = 3 23 1764386 25 1881225 NOTICE: j = 4 23 1764391 25 1881225 NOTICE: j = 5 23 1764381 25 1881225 NOTICE: j = 6 23 1764386 25 1881215 NOTICE: j = 7 23 1764391 25 1881215 NOTICE: j = 8 23 1764381 25 1881215 Seems correct to me. Passes all regression tests. Joe Index: src/backend/parser/parse_func.c =================================================================== RCS file: /opt/src/cvs/pgsql-server/src/backend/parser/parse_func.c,v retrieving revision 1.149 diff -c -r1.149 parse_func.c *** src/backend/parser/parse_func.c 6 Jun 2003 15:04:02 -0000 1.149 --- src/backend/parser/parse_func.c 24 Jun 2003 23:33:50 -0000 *************** *** 1075,1124 **** **iter; Oid *oneres; int i, ! j; ! int cur[FUNC_MAX_ARGS]; nanswers = 1; for (i = 0; i < nargs; i++) { ! nanswers *= (arginh[i].nsupers + 2); ! cur[i] = 0; } ! iter = result = (Oid **) palloc(sizeof(Oid *) * nanswers); /* compute the cross product from right to left */ ! for (;;) { oneres = (Oid *) palloc0(FUNC_MAX_ARGS * sizeof(Oid)); ! for (i = nargs - 1; i >= 0 && cur[i] > arginh[i].nsupers; i--) ! continue; ! ! /* if we're done, terminate with NULL pointer */ ! if (i < 0) ! { ! *iter = NULL; ! return result; ! } ! ! /* no, increment this column and zero the ones after it */ ! cur[i] = cur[i] + 1; ! for (j = nargs - 1; j > i; j--) ! cur[j] = 0; ! ! for (i = 0; i < nargs; i++) { ! if (cur[i] == 0) ! oneres[i] = arginh[i].self; ! else if (cur[i] > arginh[i].nsupers) ! oneres[i] = 0; /* wild card */ else ! oneres[i] = arginh[i].supervec[cur[i] - 1]; } ! ! *iter++ = oneres; } } --- 1076,1117 ---- **iter; Oid *oneres; int i, ! j, ! cum; nanswers = 1; for (i = 0; i < nargs; i++) { ! if (arginh[i].nsupers) ! nanswers *= arginh[i].nsupers; } ! /* need room for wildcard vector and null terminator */ ! iter = result = (Oid **) palloc(sizeof(Oid *) * (nanswers + 2)); /* compute the cross product from right to left */ ! for (j = 0; j < nanswers; j++) { oneres = (Oid *) palloc0(FUNC_MAX_ARGS * sizeof(Oid)); + cum = 1; ! for (i = nargs - 1; i >= 0; i--) { ! if (arginh[i].nsupers) ! { ! oneres[i] = arginh[i].supervec[(j / cum) % arginh[i].nsupers]; ! cum *= arginh[i].nsupers; ! } else ! oneres[i] = arginh[i].self; } ! iter[j] = oneres; } + oneres = (Oid *) palloc0(FUNC_MAX_ARGS * sizeof(Oid)); + iter[nanswers] = oneres; + iter[nanswers + 1] = NULL; + + return result; }
pgsql-patches by date: