Re: Clean-up callbacks for non-SR functions

From: James William Pye <flaw(at)rhid(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Clean-up callbacks for non-SR functions
Date: 2004-05-21 17:03:39
Message-ID: 20040521170339.GU62439@void.ph.cox.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 05/20/04:20/4, Tom Lane wrote:
> It's true that this setup doesn't allow non-SRFs to get at the econtext,
> but I'm not sure that they need to.
The only thing my goal has in common with getting at the econtext is the ability
to register a callback that can clean up my fn_extra at a relatively
appropriate time.

Effectively, the looked-up FmgrInfo "owns" a reference to the PyObject*
stored in fn_extra. Ideally, the reference count of the object that fn_extra
points to would be decremented by a callback/hook before the FmgrInfo is
destroyed/pfree'd..

> Such a function would however fail in situations
> where there simply isn't any econtext, which I think includes a lot of
> the system's internal uses.
Aye; this is why I was hoping for an ExprContext[normally executor?]
independent mechanism.

Although, I am mostly concerned with normal usage(for now), which I think
constitutes the context of the executor, and thus ExprContext being available.

> We have not previously seen any example where that's important.
> ....
> So let's see the use-case first.

Okay,

[a little foreword]
I use fn_extra to store state information, specifically[normally]
a pointer to an iterator(PyObject*). This usage occurs when the user
chooses to return an iterator object(A generator is an iterator). So that
the next time the function is called, the next iteration is retrieved
rather than calling the function. A generator will execute the function,
but it continues the execution after the last yield statement.

Initially, I wasn't sure how such functions differed from an explicit
VPC-SRF, but they do, especially in regard to isDone.

When a VPC-SRF is done, it should of course mark isDone, but one of these
"mock VPC-SRFs" should, effectively, restart, and continue on a given state
until its FmgrInfo is thrown away..

COUNTER:
i=0L
while True: # It doesn't stop, and shouldn't
i+=1
yield i
...

CREATE FUNCTION srfcount()
RETURNS SETOF numeric
LANGUAGE plpy AS '$COUNTER';

SELECT srfcount();
...

Well, you're gonna be waiting a while, as it will never be done, because isDone
in the RSI will never be ExprEndResult.

-----------

CREATE FUNCTION count()
RETURNS numeric
LANGUAGE plpy AS '$COUNTER';

SELECT count(), * FROM someTable;

Renders the desired result:

[snip previous 7]
1: count = "8" (typeid = 1700, len = -1, typmod = -1, byval = f)
2: somec = "foo" (typeid = 25, len = -1, typmod = -1, byval = f)

Indeed this is a trivial case, but I believe this functionality would solve
some of the issues that Elein discussed in a talk about plpython at OSCON2003.
And in an elegant, Pythonic way.

The code at the end:
http://www.varlena.com/varlena/Talks/PyAggs/pyaggs.html

> Indeed. Since passing a ReturnSetInfo in resultinfo occurs only when
> the system is expecting a set result (and is prepared to handle one),
> I do not see what you would expect different here.
Well, I guess I wasn't expecting the system to request a set from a function
that is not said to return a set(proretset):

SELECT * FROM Composite(); -- Example from my preceding letter
...
>> resultinfo = 0xbfbff1d4,

Shows that the system gives it resultinfo, thus implying that the system
wants a set, but Composite() merely returns a single complex type(tuple).

I understand that this is to be seen as a feature rather than a flaw.

> s/makes it work/breaks it/ ... this patch would effectively inform
> functions that *aren't* supposed to return set that a set result is
> expected.
Indeed, it does. The new patch attached isn't much better, and is included
for mostly discussion purposes.

The patch stands to imply that a function should only attempt to return a
set if the _contents_ of ResultSetInfo dictates such an action. ie,
allowedModes should be a valid mode.(It is set to 0 in my patch)

This seems to have some hackish qualities, so your solution would probably be better:
> If it really were important then we'd
> need to invent a different result node type (*not* ReturnSetInfo) to
> carry only econtext.
Although, I'm not sure if it's really that important..

> Which would certainly break plpgsql,
[From a _brief_ analysis of pl/plpgsql]
AFAICS, plpgsql only thinks about returning a set when retisset(fn_retset) is
true, which is only true when proretset is true, which is only true if the
function was explicitly defined to return a SETOF. So I'm not sure why
this would break plpgsql.

Needless to say, I don't see why plpgsql would act any differently if it
were given a not null RSI when !fn_retset, considering that it only seems
to use it when fn_retset is true.

I didn't test it, so I might need to eat my hat, but...
Line 353 of pl/plpgsql/pl_exec.c in plpgsql_exec_function()
Also, return next is only allowed if fn_retset: In gram.y, line ~1179.

> and probably any other callee that is coded to handle both cases.
Aye. Can't argue with that.

Regards,
James William Pye

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2004-05-21 17:23:50 Re: Delaying the planning of unnamed statements until Bind
Previous Message Merlin Moncure 2004-05-21 16:52:23 Re: ~ crashes backend