Re: ffunc called multiple for same value

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Ian Burrell <imb(at)rentrak(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: ffunc called multiple for same value
Date: 2004-07-23 19:20:44
Message-ID: 27319.1090610444@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Ian Burrell <imb(at)rentrak(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2004-07-23 19:31:47 Re: [HACKERS] Tutorial
Previous Message Tony Reina 2004-07-23 18:52:14 Can I determine the server name from PGresult?