Thread: Many Pl/PgSQL parameters -> AllocSetAlloc(128)?
I'm creating a new OpenACS package that uses PostgreSQL, and in doing
so have encountered what seems to be a problem in PostgreSQL.
Specifically, I defined a Pl/PgSQL function with 24 parameters. This
should be OK under PostgreSQL 7.3, which I'm running. (I'm using the
RPM version under Red Hat 7.3, and haven't encountered any other
problems with PostgreSQL on this machine.)
I can define the function just fine, as you can see form this stub
that I created for testing:
CREATE OR REPLACE FUNCTION add_news__test (integer,varchar,timestamptz,varchar,varchar,varchar,
varchar,integer,timestamptz,integer,timestamptz,varchar,varchar,varchar,integer,boolean, varchar, varchar, varchar,
timestamptz,integer, varchar, integer, integer)
returns integer as '
declare
p_item_id alias for $1; -- default null/integer
--
p_locale alias for $2; -- default null/varchar
--
p_publish_date alias for $3; -- default null/timestamptz
p_text alias for $4; -- default null/text
p_nls_language alias for $5; -- default null/varchar
p_title alias for $6; -- default null/varchar
p_mime_type alias for $7; -- default ''text/plain''/varchar
--
p_package_id alias for $8; -- default null/integer
p_archive_date alias for $9; -- default null/timestamptz
p_approval_user alias for $10; -- default null/integer
p_approval_date alias for $11; -- default null/timestamptz
p_approval_ip alias for $12; -- default null/varchar
--
p_relation_tag alias for $13; -- default null/varchar
--
p_creation_ip alias for $14; -- default null/varchar
p_creation_user alias for $15; -- default null/integer
--
p_is_live_p alias for $16; -- default ''f''/boolean
p_subtitle alias for $17; -- default null/text
p_abstract alias for $18; -- default null/text
p_notes alias for $19; -- default null/text
p_last_mod_date alias for $20; -- default null/timestamptz
p_modified_by alias for $21; -- default null/integer
p_last_mod_date alias for $20; -- default null/timestamptz
p_modified_by alias for $21; -- default null/integer
p_image_filename alias for $22; -- default null/text
p_headline_page alias for $23; -- default null/integer
p_headline_position alias for $24; -- default null/integer
v_add_news_id integer;
v_item_id integer;
v_id integer;
v_revision_id integer;
v_parent_id integer;
v_name varchar;
v_log_string varchar;
begin
RAISE NOTICE ''blah''
end;
' language 'plpgsql';
But when I try to invoke the function, one of the postmaster processes
consumes all available memory until I get an AllocSetAlloc error.
Here's a sample invocation:
select add_news__test(
1000::integer, -- p_item_id
'en_US'::varchar, -- p_locale
'2003-6-23'::timestamptz, -- p_publish_date
'text text text'::varchar, -- p_text
'language'::varchar, -- p_nls_language
'title'::varchar, -- p_title
'text/plain'::varchar, -- p_mime_type
'1049'::integer, -- p_package_id
'2003-7-7'::timestamptz, -- p_archive_date
'298'::integer, -- p_approval_user
'2003-06-23'::timestamptz, -- p_approval_date
'212.29.241.228'::varchar, -- p_approval_ip
'relation'::varchar, -- p_relation_tag
'212.29.241.228'::varchar, -- p_creation_ip
'298'::integer, -- p_creation_user
't'::boolean, -- p_is_live_p
'subtitle'::varchar, -- p_subtitle
'abstract'::varchar, -- p_abstract
'note'::varchar, -- p_notes
now(), -- p_last_mod_date
'298'::integer, -- p_modified_by
'image.jpeg'::varchar, -- p_image_filename
'Category page'::varchar, -- p_headline_page
'1'::integer -- p_headline_position
);
I thought that it was my function definition that was causing
trouble. But I create a new dummy function (as you can see from its
definition above) that does nothing but produce a notification. And
yet, that function also consumes all available memory.
Several questions:
- It is OK to have 24 parameters to a function in 7.3, right? (No, I
wouldn't normally want to have that many, but I find it hard to see
where I can give up on any of them.)
- I've tried to change "text" parameters to varchar, and set all null
parameters to non-null values, just to see if either of these would
make a difference. Unfortunately, they did not.
- Would compiling PostgreSQL myself, rather than using the RPM
version, change things?
- Is there a parameter in postgresql.conf that I can/should update?
- I've already raised the ceiling on memory usage, so far as I can
tell -- and invoking my function now consumes lots more memory
before giving me this error. So the problem is PostgreSQL, not my
machine.
- Am I missing some other important issue?
At this point, I think that I'm going to look at a completely
different implementation strategy that will (a) avoid the use of
functions and (b) allow me to hit my deadline. But it's a shame that
I have to ignore the OpenACS functionality on which I wanted to
depend, simply because of this issue.
Reuven
(cross-posting to HACKERS)
Reuven M. Lerner wrote:
> I'm creating a new OpenACS package that uses PostgreSQL, and in doing
> so have encountered what seems to be a problem in PostgreSQL.
[...snip...]
> CREATE OR REPLACE FUNCTION add_news__test
> (integer,varchar,timestamptz,varchar,varchar,varchar,
> varchar,integer,timestamptz,integer,timestamptz,varchar,varchar,
> varchar,integer,boolean, varchar, varchar, varchar, timestamptz,
> integer, varchar, integer, integer)
^^^^^^^^
[...snip...]
> p_last_mod_date alias for $20; -- default null/timestamptz
> p_modified_by alias for $21; -- default null/integer
> p_last_mod_date alias for $20; -- default null/timestamptz
> p_modified_by alias for $21; -- default null/integer
^^^^^^^^^^^^^ above two lines repeated
> p_image_filename alias for $22; -- default null/text
> p_headline_page alias for $23; -- default null/integer
^^^^^^^^^^^^^^^^^^^^
more importantly, you call the function (below) with a varchar here, not
integer
> p_headline_position alias for $24; -- default null/integer
[...snip...]
> now(), -- p_last_mod_date
^^^^^^^ try 'now'::timestamptz
> '298'::integer, -- p_modified_by
> 'image.jpeg'::varchar, -- p_image_filename
> 'Category page'::varchar, -- p_headline_page
^^^^^^^^^^^ this one should be an integer
> '1'::integer -- p_headline_position
> );
You found a real bug, I can confirm it on CVS tip.
However your workaround is to call the function *exactly* as declared.
Otherwise in parse_func.c:gen_cross_product() the following code is
executed:
<snippet>
nanswers = 1;
for (i = 0; i < nargs; i++)
{
nanswers *= (arginh[i].nsupers + 2);
cur[i] = 0;
}
iter = result = (Oid **) palloc(sizeof(Oid *) * nanswers);
</snippet>
I get nanswers = 16777216, so right off the bat 67MB or so is allocated.
Then there's this:
<snippet>
/* compute the cross product from right to left */
for (;;)
{
oneres = (Oid *) palloc0(FUNC_MAX_ARGS * sizeof(Oid));
</snippet>
I'm guessing this gets executed nanswers times. I saw memory usage grow
to 880 MB and then killed the process.
I'm not sure of the best way to fix this yet, but I found that when
calling the function with argument types matching the prototype
perfectly, this code never gets executed.
HTH,
Joe
p.s. here's a backtrace:
#0 AllocSetAlloc (context=0x830a624, size=128) at aset.c:731
#1 0x081bcb14 in MemoryContextAllocZero (context=0x830a624, size=128)
at mcxt.c:505
#2 0x080c5c03 in gen_cross_product (arginh=0xbfffd120, nargs=24) at
parse_func.c:1094
#3 0x080c59b6 in argtype_inherit (nargs=24, argtypes=0xbfffd350) at
parse_func.c:975
#4 0x080c5836 in func_get_detail (funcname=0x831451c, fargs=0x83178e8,
nargs=24, argtypes=0xbfffd350, funcid=0xbfffd33c,
rettype=0xbfffd340, retset=0xbfffd347 "\bÁ\002",
true_typeids=0xbfffd348) at parse_func.c:891
#5 0x080c4c4c in ParseFuncOrColumn (pstate=0x8317810,
funcname=0x831451c, fargs=0x83178e8, agg_star=0 '\0',
agg_distinct=0 '\0', is_column=0 '\0') at parse_func.c:241
#6 0x080c41de in transformExpr (pstate=0x8317810, expr=0x8317714) at
parse_expr.c:399
#7 0x080cb4ed in transformTargetEntry (pstate=0x8317810,
node=0x8317714, expr=0x0, colname=0x0, resjunk=0 '\0')
at parse_target.c:60
#8 0x080cb53b in transformTargetList (pstate=0x8317810,
targetlist=0x831774c) at parse_target.c:193
#9 0x080b61c8 in transformSelectStmt (pstate=0x8317810, stmt=0x8317768)
at analyze.c:1771
#10 0x080b41b7 in transformStmt (pstate=0x8317810, parseTree=0x8317768,
extras_before=0xbfffd574, extras_after=0xbfffd578)
at analyze.c:407
#11 0x080b402b in do_parse_analyze (parseTree=0x8317768,
pstate=0x8317810) at analyze.c:234
#12 0x080b3f44 in parse_analyze (parseTree=0x8317768,
paramTypes=0x830a624, numParams=137405988) at analyze.c:159
#13 0x08159c3c in pg_analyze_and_rewrite (parsetree=0x8317768,
paramTypes=0x0, numParams=0) at postgres.c:482
#14 0x08159f83 in exec_simple_query (
query_string=0x8313c40 " select add_news__test(\n
1000::integer,", ' ' <repeats 15 times>, "\n 'en_US'::varchar,", ' '
<repeats 15 times>, "\n '2003-6-23'::timestamptz, \n 'text text
text'::varchar, \n 'language'::varchar, "...) at
postgres.c:795
#15 0x0815bd1b in PostgresMain (argc=4, argv=0x829aa9c,
username=0x829aa64 "postgres") at postgres.c:2753
#16 0x0813a531 in BackendFork (port=0x82a80c0) at postmaster.c:2471
#17 0x0813a026 in BackendStartup (port=0x82a80c0) at postmaster.c:2118
#18 0x08138b5f in ServerLoop () at postmaster.c:1090
#19 0x081384dd in PostmasterMain (argc=5, argv=0x829a4c8) at
postmaster.c:872
#20 0x0810f713 in main (argc=5, argv=0xbfffe334) at main.c:211
#21 0x420156a4 in __libc_start_main () from /lib/tls/libc.so.6
Excellent -- thanks so much for your help. I just tried the function with the right arguments, and it worked just fine. Yet more proof of named parameters being a good thing... Reuven
Joe Conway wrote:
> I get nanswers = 16777216, so right off the bat 67MB or so is allocated.
> Then there's this:
>
> <snippet>
> /* compute the cross product from right to left */
> for (;;)
> {
> oneres = (Oid *) palloc0(FUNC_MAX_ARGS * sizeof(Oid));
> </snippet>
>
> I'm guessing this gets executed nanswers times. I saw memory usage grow
> to 880 MB and then killed the process.
>
> I'm not sure of the best way to fix this yet, but I found that when
> calling the function with argument types matching the prototype
> perfectly, this code never gets executed.
Actually, adding a "pfree(oneres);" to the end of that for loop plugs
the memory leak and allows me to see the error message:
ERROR: Function add_news__test(integer, character varying, timestamp
with time zone, character varying, character varying, character varying,
character varying, integer, timestamp with time zone, integer, timestamp
with time zone, character varying, character varying, character varying,
integer, boolean, character varying, character varying, character
varying, timestamp with time zone, integer, character varying, character
varying, integer) does not exist
Unable to identify a function that satisfies the given argument
types
You may need to add explicit typecasts
Takes a while to check all 16777216 possibilities though, so I'm still
not sure more isn't needed here.
Joe
How come you didn't get a "No such function with those arguments" error that I always get when I do that?
planning=# create function oops (integer) returns int language pltcl as '
planning'# elog NOTICE "blah"
planning'# ';
CREATE FUNCTION
planning=# select oops (cast('duh' as varchar));
ERROR: Function oops(character varying) does not exist
Unable to identify a function that satisfies the given argument types
You may need to add explicit typecasts
Ian Harding
Programmer/Analyst II
Tacoma-Pierce County Health Department
iharding@tpchd.org
Phone: (253) 798-3549
Pager: (253) 754-0002
>>> "Reuven M. Lerner" <reuven@lerner.co.il> 06/23/03 11:56PM >>>
Excellent -- thanks so much for your help. I just tried the function
with the right arguments, and it worked just fine.
Yet more proof of named parameters being a good thing...
Reuven
---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings
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:
Good catch.
> Takes a while to check all 16777216 possibilities though, so I'm still
> not sure more isn't needed here.
I wonder why it's doing anything at all --- the check for supertypes
ought to trigger only for complex type (rowtype) arguments, and there
are none here.
regards, tom lane
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;
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.
regards, tom lane
I said:
> 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.
I've fixed the problem along the above lines. The patch against 7.3
branch is attached in case Reuven would like to apply it locally.
regards, tom lane
*** src/backend/parser/parse_func.c.orig Wed Apr 23 14:20:10 2003
--- src/backend/parser/parse_func.c Wed Jun 25 15:32:52 2003
***************
*** 904,926 ****
* argtype_inherit() -- Construct an argtype vector reflecting the
* inheritance properties of the supplied argv.
*
! * This function is used to disambiguate among functions with the
! * same name but different signatures. It takes an array of input
! * type ids. For each type id in the array that's a complex type
! * (a class), it walks up the inheritance tree, finding all
! * superclasses of that type. A vector of new Oid type arrays
! * is returned to the caller, reflecting the structure of the
! * inheritance tree above the supplied arguments.
*
* The order of this vector is as follows: all superclasses of the
* rightmost complex class are explored first. The exploration
* continues from right to left. This policy means that we favor
* keeping the leftmost argument type as low in the inheritance tree
* as possible. This is intentional; it is exactly what we need to
! * do for method dispatch. The last type array we return is all
! * zeroes. This will match any functions for which return types are
! * not defined. There are lots of these (mostly builtins) in the
! * catalogs.
*/
static Oid **
argtype_inherit(int nargs, Oid *argtypes)
--- 904,932 ----
* argtype_inherit() -- Construct an argtype vector reflecting the
* inheritance properties of the supplied argv.
*
! * This function is used to handle resolution of function calls when
! * there is no match to the given argument types, but there might be
! * matches based on considering complex types as members of their
! * superclass types (parent classes).
! *
! * It takes an array of input type ids. For each type id in the array
! * that's a complex type (a class), it walks up the inheritance tree,
! * finding all superclasses of that type. A vector of new Oid type
! * arrays is returned to the caller, listing possible alternative
! * interpretations of the input typeids as members of their superclasses
! * rather than the actually given argument types. The vector is
! * terminated by a NULL pointer.
*
* The order of this vector is as follows: all superclasses of the
* rightmost complex class are explored first. The exploration
* continues from right to left. This policy means that we favor
* keeping the leftmost argument type as low in the inheritance tree
* as possible. This is intentional; it is exactly what we need to
! * do for method dispatch.
! *
! * The vector does not include the case where no complex classes have
! * been promoted, since that was already tried before this routine
! * got called.
*/
static Oid **
argtype_inherit(int nargs, Oid *argtypes)
***************
*** 929,950 ****
int i;
InhPaths arginh[FUNC_MAX_ARGS];
! for (i = 0; i < FUNC_MAX_ARGS; i++)
{
! if (i < nargs)
! {
! arginh[i].self = argtypes[i];
! if ((relid = typeidTypeRelid(argtypes[i])) != InvalidOid)
! arginh[i].nsupers = find_inheritors(relid, &(arginh[i].supervec));
! else
! {
! arginh[i].nsupers = 0;
! arginh[i].supervec = (Oid *) NULL;
! }
! }
else
{
- arginh[i].self = InvalidOid;
arginh[i].nsupers = 0;
arginh[i].supervec = (Oid *) NULL;
}
--- 935,947 ----
int i;
InhPaths arginh[FUNC_MAX_ARGS];
! for (i = 0; i < nargs; i++)
{
! arginh[i].self = argtypes[i];
! if ((relid = typeidTypeRelid(argtypes[i])) != InvalidOid)
! arginh[i].nsupers = find_inheritors(relid, &(arginh[i].supervec));
else
{
arginh[i].nsupers = 0;
arginh[i].supervec = (Oid *) NULL;
}
***************
*** 954,959 ****
--- 951,963 ----
return gen_cross_product(arginh, nargs);
}
+ /*
+ * Look up the parent superclass(es) of the given relation.
+ *
+ * *supervec is set to an array of the type OIDs (not the relation OIDs)
+ * of the parents, with nearest ancestors listed first. It's set to NULL
+ * if there are no parents. The return value is the number of parents.
+ */
static int
find_inheritors(Oid relid, Oid **supervec)
{
***************
*** 1047,1105 ****
return nvisited;
}
static Oid **
gen_cross_product(InhPaths *arginh, int nargs)
{
int nanswers;
! Oid **result,
! **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 *) palloc(FUNC_MAX_ARGS * sizeof(Oid));
! MemSet(oneres, 0, 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;
}
}
--- 1051,1132 ----
return nvisited;
}
+ /*
+ * Generate the ordered list of substitute argtype vectors to try.
+ *
+ * See comments for argtype_inherit.
+ */
static Oid **
gen_cross_product(InhPaths *arginh, int nargs)
{
int nanswers;
! Oid **result;
Oid *oneres;
int i,
j;
int cur[FUNC_MAX_ARGS];
+ /*
+ * At each position we want to try the original datatype, plus each
+ * supertype. So the number of possible combinations is this:
+ */
nanswers = 1;
for (i = 0; i < nargs; i++)
! nanswers *= (arginh[i].nsupers + 1);
! /*
! * We also need an extra slot for the terminating NULL in the result
! * array, but that cancels out with the fact that we don't want to
! * generate the zero-changes case. So we need exactly nanswers slots.
! */
! result = (Oid **) palloc(sizeof(Oid *) * nanswers);
! j = 0;
!
! /*
! * Compute the cross product from right to left. When cur[i] == 0,
! * generate the original input type at position i. When cur[i] == k
! * for k > 0, generate its k'th supertype.
! */
! MemSet(cur, 0, sizeof(cur));
for (;;)
{
! /*
! * Find a column we can increment. All the columns after it get
! * reset to zero. (Essentially, we're adding one to the multi-
! * digit number represented by cur[].)
! */
! for (i = nargs - 1; i >= 0 && cur[i] >= arginh[i].nsupers; i--)
! cur[i] = 0;
! /* if none, we're done */
if (i < 0)
! break;
! /* increment this column */
! cur[i] += 1;
!
! /* Generate the proper output type-OID vector */
! oneres = (Oid *) palloc(FUNC_MAX_ARGS * sizeof(Oid));
! MemSet(oneres, 0, FUNC_MAX_ARGS * sizeof(Oid));
for (i = 0; i < nargs; i++)
{
if (cur[i] == 0)
oneres[i] = arginh[i].self;
else
oneres[i] = arginh[i].supervec[cur[i] - 1];
}
! result[j++] = oneres;
}
+
+ /* terminate result vector with NULL pointer */
+ result[j++] = NULL;
+
+ Assert(j == nanswers);
+
+ return result;
}
Wow. I just want to express my appreciation to Tom and the rest of the folks in the PostgreSQL community. You can be sure that this little episode, in which less than 48 hours passed from bug discovery to patch, will make it into my future talks about the wonders of open-source software. I'm extremely impressed and grateful. Reuven