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


Thu, 22 Jul 2004 15:44:14 -0700
Ian Burrell <imb@rentrak.com>
ffunc called multiple for same value
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

Fri, 23 Jul 2004 15:20:44 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: ffunc called multiple for same value 
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



Fri, 23 Jul 2004 16:39:05 -0400
Mike Mascari <mascarm@mascari.com>
Re: ffunc called multiple for same value
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



Fri, 23 Jul 2004 13:48:39 -0700
Ian Burrell <imb@rentrak.com>
Re: ffunc called multiple for same value
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

Fri, 23 Jul 2004 17:01:23 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: ffunc called multiple for same value 