Re: SRF patch (was Re: [HACKERS] troubleshooting pointers)

Joe Conway <mail(at)joeconway(dot)com> writes:
> I've been looking through the SRF patch as-committed, and I think I
> understand most of your changes, but I have a question: FunctionNext()
> now seems to *always* use a tuplestore instead of conditionally using
> the store only for rescans,

The problem is that as things now stand, you do not know whether you
will be asked to rescan, so you must materialize the result just in
case.

I would like to improve the system so that lower-level plan nodes will
be told whether they need to support rescan; but we aren't there yet,
and I don't think it's the first priority to work on for SRF. Always
materializing will do for the moment.

> Do you still think there should be an option to project
> tuples without first storing them, or should we eliminate the notion of
> function mode and always materialize?

If the function is going to produce a materialized tupleset to begin
with (because that's convenient for it internally) then there's no value
in having nodeFunctionscan.c make duplicate storage of the tupleset.
We need some way of communicating that fact from the function back to
the plan node ... but again, not first priority.

>> Parameters also need thought. This should be rejected:
>>
>> regression=# select * from foo, foot(fooid) z where foo.f2 = z.f2;
>> server closed the connection unexpectedly

> I don't understand why this should be rejected, but it does fail for me
> also, due to a NULL slot pointer. At what point should it be rejected?

In the parser. Ideally, fooid should not even be *visible* while we are
parsing the arguments to the sibling FROM node. Compare the handling of
variable resolution in JOIN/ON clauses --- the namespace gets
manipulated so that those clauses can't see vars from sibling FROM nodes.

>> On the other hand, IMHO this should work:
>>
>> regression=# select * from foo where f2 in
>> regression-# (select f2 from foot(foo.fooid) z where z.fooid = foo.fooid);
>> server closed the connection unexpectedly

> This also fails in (based on a quick look) exactly the same way -- a
> NULL slot pointer (econtext->ecxt_scantuple) passed to ExecEvalVar().

Right. This should work, but the Var has to be converted into a Param
referencing the upper-level variable. I've forgotten right at the
moment where that happens (someplace in the planner) ... but I'll bet
that the someplace doesn't know it needs to process function argument
nodetrees in function RTEs.

regards, tom lane