Thread: ffunc called multiple for same value
I posted a message a couple weeks ago abou having a problem with a user-defined C language aggregate and the ffunc being called multiple times with the same state. I came up with a test case which shows the problem with plpgsql functions. It occurs with an aggregate in an inner query, when a nested loop is used. ANALYZE the tables with zero rows causes it to use a nested loop. We first discovered the problem when we analyzed a test database and our ffunc started failing because we assumed the ffunc was called once and could free memory. CREATE TABLE foo (a integer); CREATE TABLE bar (a integer, b integer, c integer); ANALYZE foo; ANALYZE bar; INSERT INTO foo VALUES (1); INSERT INTO foo VALUES (2); INSERT INTO foo VALUES (3); INSERT INTO bar VALUES (1, 5, 19); INSERT INTO bar VALUES (2, 7, 23); INSERT INTO bar VALUES (2, 9, 29); INSERT INTO bar VALUES (3, 11, 31); INSERT INTO bar VALUES (3, 13, 37); INSERT INTO bar VALUES (3, 17, 41); CREATE OR REPLACE FUNCTION custom_agg_sfunc(integer, integer) RETURNS integer LANGUAGE 'plpgsql' AS ' BEGIN RAISE NOTICE ''custom_agg_sfunc: state: % value % '', $1, $2; RETURN $1 * $2; END; '; CREATE OR REPLACE FUNCTION custom_agg_ffunc(integer) RETURNS integer LANGUAGE 'plpgsql' AS ' BEGIN RAISE NOTICE ''custom_agg_ffunc: % '', $1; RETURN $1; END; '; CREATE AGGREGATE custom_agg ( sfunc = custom_agg_sfunc, basetype = integer, stype = integer, finalfunc = custom_agg_ffunc, initcond = 1 ); SELECT foo.a, comp FROM foo, ( SELECT a, custom_agg(c) AS comp FROM bar GROUP BY a ) x WHERE foo.a = x.a; The results are: NOTICE: custom_agg_sfunc: state: 1 value 31 NOTICE: custom_agg_sfunc: state: 1 value 37 NOTICE: custom_agg_sfunc: state: 37 value 41 NOTICE: custom_agg_sfunc: state: 1 value 43 NOTICE: custom_agg_sfunc: state: 43 value 47 NOTICE: custom_agg_sfunc: state: 2021 value 53 NOTICE: custom_agg_ffunc: 31 NOTICE: custom_agg_ffunc: 1517 NOTICE: custom_agg_ffunc: 107113 NOTICE: custom_agg_ffunc: 31 NOTICE: custom_agg_ffunc: 1517 NOTICE: custom_agg_ffunc: 107113 NOTICE: custom_agg_ffunc: 31 NOTICE: custom_agg_ffunc: 1517 NOTICE: custom_agg_ffunc: 107113 a | comp ---+-------- 3 | 31 5 | 1517 7 | 107113 (3 rows) - Ian
Ian Burrell <imb@rentrak.com> writes:
> I posted a message a couple weeks ago abou having a problem with a
> user-defined C language aggregate and the ffunc being called multiple
> times with the same state. I came up with a test case which shows the
> problem with plpgsql functions. It occurs with an aggregate in an inner
> query, when a nested loop is used.
I looked into this and found that the unexpected behavior occurs only
when a HashAggregate plan is used. If you force a GroupAggregate to be
used (set enable_hashagg = false), then you get one series of sfunc
calls and one ffunc call, per group per scan of the inner relation.
In the HashAgg code, the series of sfunc calls is executed only once per
group, with the final transvalue being stored in the hash table. The
ffunc will be re-evaluated on each traversal of the hash table for
output --- which could be multiple times, if the grouped table is used
as the inside of a nestloop, as in this example.
I can imagine fixing this by having the HashAgg code replace the final
transvalue in the hash table with the ffunc result value. It would not
be a whole lot of additional code, but it would make things noticeably
more complicated in what's already a rather complex bit of code (mainly
because transvalue and result could be different datatypes). Probably
the worst objection is that with pass-by-reference result types, an
additional datumCopy step would be needed to stash the result in the
hash table (and there'd be an extra pfree, too). That would slow things
down for everybody, with no gain unless the HashAgg result is in fact
read multiple times.
A different alternative which would be much lower-impact in terms of
code changes would be to change ExecReScanAgg() to always throw away
the hash table, even if it knows that the input data has not changed.
While this would avoid any time penalty for those not making use of
repeated scans, it would be a huge penalty for those that are, so it
hardly seems like an appealing choice either.
So I'm rather inclined to define this behavior as "not a bug". The fact
that you're complaining seems to indicate that your ffunc scribbles on
its input, which is bad programming practice in any case. Ordinarily
I would not think that an ffunc should have any problem with being
executed repeatedly on the same final transvalue. (If you really want
to do things that way, maybe your code should take responsibility for
keeping a flag to execute just once, rather than pushing the cost onto
everybody.)
Comments anyone?
regards, tom lane
Tom Lane wrote: > So I'm rather inclined to define this behavior as "not a bug". The fact > that you're complaining seems to indicate that your ffunc scribbles on > its input, which is bad programming practice in any case. Ordinarily > I would not think that an ffunc should have any problem with being > executed repeatedly on the same final transvalue. (If you really want > to do things that way, maybe your code should take responsibility for > keeping a flag to execute just once, rather than pushing the cost onto > everybody.) > > Comments anyone? As someone who makes use of C language aggregate functions, I agree with your analysis, so long as the fact that an ffunc may be invoked more than once is well documented, (i.e. an SGML <note> section might be nice.) Mike Mascari
Tom Lane wrote: > > So I'm rather inclined to define this behavior as "not a bug". The fact > that you're complaining seems to indicate that your ffunc scribbles on > its input, which is bad programming practice in any case. Ordinarily > I would not think that an ffunc should have any problem with being > executed repeatedly on the same final transvalue. (If you really want > to do things that way, maybe your code should take responsibility for > keeping a flag to execute just once, rather than pushing the cost onto > everybody.) > We are doing things in the aggregates that make them troublesome when called the ffunc is called multiple times. The state structure uses a lot of memory for intermediate work. The memory needs to be freed as soon as possible otherwise there is a danger of running of out memory. It is possible to store the resuts on the first ffunc call, free the intermediate state, return the results on later calls, and make sure the free only happens once. The docs didn't make clear that calling ffunc multiple times could happens so we did not code to allow it. - Ian
Ian Burrell <imb@rentrak.com> writes:
> We are doing things in the aggregates that make them troublesome when
> called the ffunc is called multiple times. The state structure uses a
> lot of memory for intermediate work. The memory needs to be freed as
> soon as possible otherwise there is a danger of running of out memory.
Possibly you should just force enable_hashagg off, if you are concerned
about memory usage. ISTM that running multiple transvalue calculations
in parallel is a bad idea from the start, if you are feeling that tense
about the amount of memory that will be chewed up by just one.
regards, tom lane