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