troubleshooting pointers

Hello, Joe!

JC> With the current SRF patch, in certain circumstances selecting from
JC> a
JC> VIEW produces "Buffer Leak" warnings, while selecting from the
JC> function itself does not. Also the VIEW returns only one of the two

Selecting from the function produces such a warning when using it with
limit,
but it does not when the function returns less rows than specified in limit
.
e.g.

just_fun=# create table testtab(i integer, v varchar);
CREATE
just_fun=# insert into testtab values(1,'one');
INSERT 16592 1
just_fun=# insert into testtab values(2,'two');
INSERT 16593 1
just_fun=# insert into testtab values(3,'three');
INSERT 16594 1
just_fun=# insert into testtab values(1,'one again');
INSERT 16595 1
just_fun=# create function fun(integer) returns setof testtab as 'select *
from testtab where i= $1;' language 'sql';
just_fun=# select * from fun(1) as fun;
i | v
---+-----------
1 | one
1 | one again
(2 rows)

just_fun=# select * from fun(1) as fun limit 1;
WARNING: Buffer Leak: [050] (freeNext=-3, freePrev=-3, rel=16570/16587,
blockNum=0, flags=0x85, refcount=1 2)
i | v
---+-----
1 | one
(1 row)

....And there is no warning with "ORDER BY"

just_fun=# select * from fun(1) as fun order by v limit 1;
i | v
---+-----
1 | one
(1 row)

Hope this info maybe useful to solve the problem.

By the way, could you give an example of C-function returning set?

JC> expected rows. The same SQL function when declared as "...
JC> getfoo(int) RETURNS int AS ..." instead of "... getfoo(int) RETURNS
JC> *setof* int AS..." does not produce the warning. Any ideas what I
JC> should be focusing on to track this down? Does anyone have any
JC> favorite troubleshooting techniques for this type of problem?

JC> Thanks,
JC> Joe

Thank you for your work in this direction!

With best regards, Valentine Zaretsky

Valentine Zaretsky wrote:

just_fun=# select * from fun(1) as fun limit 1;
WARNING: Buffer Leak: [050] (freeNext=-3, freePrev=-3, rel=16570/16587,
blockNum=0, flags=0x85, refcount=1 2)
i | v
---+-----
1 | one
(1 row)

....And there is no warning with "ORDER BY"

just_fun=# select * from fun(1) as fun order by v limit 1;
i | v
---+-----
1 | one
(1 row)

Hope this info maybe useful to solve the problem.

Hmm. Yes, it looks like this is probably the same or a related issue.

By the way, could you give an example of C-function returning set?

In contrib/dblink, see dblink.c for a couple of examples (dblink(),
dblink_get_pkey()), or look at pg_stat_get_backend_idset() in the
backend code. I haven't written a C-function returning a setof composite
type yet, but probably will soon, because I'll need it for testing (and
ultimately for the regression test script).

Thanks for the help!

Joe

Joe Conway <mail@joeconway.com> writes:

With the current SRF patch, in certain circumstances selecting from a
VIEW produces "Buffer Leak" warnings, while selecting from the function
itself does not. Also the VIEW returns only one of the two expected
rows.

The buffer leak suggests failure to shut down a plan tree (ie, no
ExecutorEnd call). Probably related to not running the VIEW to
completion, but it's hard to guess at the underlying cause.

Do the plan trees (EXPLAIN display) look the same in both cases?

regards, tom lane

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

With the current SRF patch, in certain circumstances selecting from a
VIEW produces "Buffer Leak" warnings, while selecting from the function
itself does not. Also the VIEW returns only one of the two expected
rows.

The buffer leak suggests failure to shut down a plan tree (ie, no
ExecutorEnd call). Probably related to not running the VIEW to
completion, but it's hard to guess at the underlying cause.

Do the plan trees (EXPLAIN display) look the same in both cases?

Yes, but it suffers from the issue you brought up yesterday -- i.e.
EXPLAIN doesn't run from within the function, and EXPLAIN outside the
function (or VIEW which calls it) doesn't show very much:

test=# EXPLAIN SELECT * FROM vw_getfoo;
QUERY PLAN
-----------------------------------------------------------
Function Scan on getfoo (cost=0.00..0.00 rows=0 width=0)
(1 row)

test=# EXPLAIN SELECT * FROM getfoo(1);
QUERY PLAN
-----------------------------------------------------------
Function Scan on getfoo (cost=0.00..0.00 rows=0 width=0)
(1 row)

I found an explaination you gave a while back which sounds like it
explains the problem:
http://archives.postgresql.org/pgsql-bugs/2001-06/msg00051.php

I also confirmed that postquel_end(), which calls ExecutorEnd(), never
gets called for the VIEW case (or the LIMIT case that was pointed out on
an earlier post).

Just now I was looking for a way to propagate the necessary information
to call ExecutorEnd() from ExecEndFunctionScan() in the case that fmgr
doesn't. It looks like I might be able to add a member to the
ExprContext struct for this purpose. Does this sound like the correct
(or at least a reasonable) approach?

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Just now I was looking for a way to propagate the necessary information
to call ExecutorEnd() from ExecEndFunctionScan() in the case that fmgr
doesn't. It looks like I might be able to add a member to the
ExprContext struct for this purpose. Does this sound like the correct
(or at least a reasonable) approach?

Yeah, this is something that's bothered me in the past: with the
existing API, a function-returning-set will not get a chance to shut
down cleanly and release resources if its result is not read all the
way to completion. You can demonstrate the problem without any
use of the SRF patch. Using current CVS tip (no patch), and the
regression database:

regression=# create function foo(int) returns setof int as '
regression'# select unique1 from tenk1 where unique2 > $1'
regression-# language sql;

regression=# select foo(9990) limit 4;
WARNING: Buffer Leak: [009] (freeNext=-3, freePrev=-3, rel=16570/135224, blockNum=29, flags=0x4, refcount=1 1)
WARNING: Buffer Leak: [021] (freeNext=-3, freePrev=-3, rel=16570/18464, blockNum=232, flags=0x4, refcount=1 1)
foo
------
4093
6587
6093
429
(4 rows)

I don't much care for the thought of trawling every expression tree
looking for functions-returning-set during plan shutdown, so the thought
that comes to mind is to expect functions that want a shutdown callback
to register themselves somehow. Adding a list of callbacks to
ExprContext seems pretty reasonable, but you'd also need some link in
ReturnSetInfo to let the function find the ExprContext to register
itself with. Then FreeExprContext would call the callbacks.

Hmm ... another advantage of doing this is that the function would be
able to find the ecxt_per_query_memory associated with the ExprContext.
That would be a Good Thing.

We should also think about the fcache (FunctionCache) struct and whether
that needs to tie into this. See the FIXME in utils/fcache.h.

regards, tom lane

Tom Lane wrote:

I don't much care for the thought of trawling every expression tree
looking for functions-returning-set during plan shutdown, so the thought
that comes to mind is to expect functions that want a shutdown callback
to register themselves somehow. Adding a list of callbacks to
ExprContext seems pretty reasonable, but you'd also need some link in
ReturnSetInfo to let the function find the ExprContext to register
itself with. Then FreeExprContext would call the callbacks.

I've made changes which fix this and will send them in with a revised
SRF patch later today. Summary of design:
1.) moved the execution_state struct and ExecStatus enum to executor.h
2.) added "void *es" member to ExprContext
3.) added econtext member to ReturnSetInfo
4.) set rsi->econtext on the way in at ExecMakeFunctionResult()
5.) set rsi->econtext->es on the way in at fmgr_sql()
6.) used econtext->es on the way out at ExecFreeExprContext() to call
ExecutorEnd() if needed (because postquel_execute() never got the chance).

One note: I changed ExecFreeExprContext() because that's where all the
action was for SQL function calls. FreeExprContext() was not involved
for the test case, but it looked like it probably should have the same
changes, so I made them there also.

Hmm ... another advantage of doing this is that the function would be
able to find the ecxt_per_query_memory associated with the ExprContext.
That would be a Good Thing.

What does this allow done that can't be done today?

We should also think about the fcache (FunctionCache) struct and whether
that needs to tie into this. See the FIXME in utils/fcache.h.

While I was at it, I added an fcache member to ExprContext, and
populated it in ExecMakeFunctionResult() for SRF cases. I wasn't sure
what else to do with it at the moment, but at least it is a step in the
right direction.

Joe

Joe Conway <mail@joeconway.com> writes:

Tom Lane wrote:

Adding a list of callbacks to
ExprContext seems pretty reasonable, but you'd also need some link in
ReturnSetInfo to let the function find the ExprContext to register
itself with. Then FreeExprContext would call the callbacks.

I've made changes which fix this and will send them in with a revised
SRF patch later today. Summary of design:
1.) moved the execution_state struct and ExecStatus enum to executor.h
2.) added "void *es" member to ExprContext
3.) added econtext member to ReturnSetInfo
4.) set rsi->econtext on the way in at ExecMakeFunctionResult()
5.) set rsi->econtext->es on the way in at fmgr_sql()
6.) used econtext->es on the way out at ExecFreeExprContext() to call
ExecutorEnd() if needed (because postquel_execute() never got the chance).

Um. I don't like that; it assumes not only that ExecutorEnd is the only
kind of callback needed, but also that there is at most one function
per ExprContext that needs a shutdown callback. Neither of these
assumptions hold water IMO.

The design I had in mind was more like this: add to ExprContext a list
header field pointing to a list of structs along the lines of

struct exprcontext_callback {
struct exprcontext_callback *next;
void (*function) (Datum);
Datum arg;
}

and then call each specified function with given argument during
FreeExprContext. Probably ought to be careful to do that in reverse
order of registration. We'd also need to invent a RescanExprContext
operation to call the callbacks during a Rescan. The use of Datum
(and not, say, void *) as PG's standard callback arg type was settled on
some time ago --- originally for on_proc_exit IIRC --- and seems to have
worked well enough.

Hmm ... another advantage of doing this is that the function would be
able to find the ecxt_per_query_memory associated with the ExprContext.
That would be a Good Thing.

What does this allow done that can't be done today?

It provides a place for the function to allocate stuff that needs to
live over multiple calls, ie, until it gets its shutdown callback.
Right now a function has to use TransactionCommandContext for that,
but that's really too coarse-grained.

We should also think about the fcache (FunctionCache) struct and whether
that needs to tie into this. See the FIXME in utils/fcache.h.

While I was at it, I added an fcache member to ExprContext, and
populated it in ExecMakeFunctionResult() for SRF cases. I wasn't sure
what else to do with it at the moment, but at least it is a step in the
right direction.

Well, I was debating whether that's good or not. The existing fcache
approach is wrong (per cited FIXME); it might be better not to propagate
access of it into more places. Unless you can see a specific reason to
allow the function to have access to the fcache struct, I think I'm
inclined not to.

What's really more relevant here is that during the hypothetical new
RescanExprContext function, we ought to go around and clear any fcaches
in the context that have setArgsValid = true, so that they will be
restarted afresh during the next scan of the plan. (The fact that that
doesn't happen now is another shortcoming of the existing set-functions-
in-expressions code.) So this suggests making a callback function type
specifically to do that, and registering every fcache that is executing
a set function in the callback list...

regards, tom lane

Tom Lane wrote:

Um. I don't like that; it assumes not only that ExecutorEnd is the only
kind of callback needed, but also that there is at most one function
per ExprContext that needs a shutdown callback. Neither of these
assumptions hold water IMO.

The design I had in mind was more like this: add to ExprContext a list
header field pointing to a list of structs along the lines of

struct exprcontext_callback {
struct exprcontext_callback *next;
void (*function) (Datum);
Datum arg;
}

and then call each specified function with given argument during
FreeExprContext. Probably ought to be careful to do that in reverse
order of registration. We'd also need to invent a RescanExprContext
operation to call the callbacks during a Rescan. The use of Datum
(and not, say, void *) as PG's standard callback arg type was settled on
some time ago --- originally for on_proc_exit IIRC --- and seems to have
worked well enough.

Well, I guess I set my sights too low ;-) This is a very nice design.

I have the shutdown callback working now, and will send a new patch in a
few minutes. I have not started RescanExprContext() yet, but will do it
when I address rescans in general.

What's really more relevant here is that during the hypothetical new
RescanExprContext function, we ought to go around and clear any fcaches
in the context that have setArgsValid = true, so that they will be
restarted afresh during the next scan of the plan. (The fact that that
doesn't happen now is another shortcoming of the existing set-functions-
in-expressions code.) So this suggests making a callback function type
specifically to do that, and registering every fcache that is executing
a set function in the callback list...

I also added FunctionCachePtr_callback struct and a member to
ExprContext. I have not yet created the registration or shutdown
functions, but again, I'll work on them as part of the rescan work.

I still have a couple of issues related to VIEWs that I need to figure
out, then I'll start the rescan work.

Thanks for the review and help!

Joe

Tom Lane wrote:

Um. I don't like that; it assumes not only that ExecutorEnd is the only
kind of callback needed, but also that there is at most one function
per ExprContext that needs a shutdown callback. Neither of these
assumptions hold water IMO.

The design I had in mind was more like this: add to ExprContext a list
header field pointing to a list of structs along the lines of

struct exprcontext_callback {
struct exprcontext_callback *next;
void (*function) (Datum);
Datum arg;
}

and then call each specified function with given argument during
FreeExprContext. Probably ought to be careful to do that in reverse
order of registration. We'd also need to invent a RescanExprContext
operation to call the callbacks during a Rescan. The use of Datum
(and not, say, void *) as PG's standard callback arg type was settled on
some time ago --- originally for on_proc_exit IIRC --- and seems to have
worked well enough.

Here's the patch, per my post to HACKERS.

It builds cleanly on my dev box, and passes all regression tests.

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

... I have not started RescanExprContext() yet, but will do it
when I address rescans in general.

I still have a couple of issues related to VIEWs that I need to figure
out, then I'll start the rescan work.

It's not unlikely that those issues are exactly due to not having rescan
handled properly. What misbehavior are you seeing?

regards, tom lane

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

... I have not started RescanExprContext() yet, but will do it
when I address rescans in general.

I still have a couple of issues related to VIEWs that I need to figure
out, then I'll start the rescan work.

It's not unlikely that those issues are exactly due to not having rescan
handled properly. What misbehavior are you seeing?

Hmm, that might just be it.

When I select from a view based on a function which returns a base type,
I only get the first row. When I select from a view which is based on a
function returning a composite type, it triggers an assertion. I've
traced the latter down to a slot pointer which is reset to NULL
somewhere. Haven't had the time to get much further. In both cases,
selecting from the function directly works great.

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Tom Lane wrote:

It's not unlikely that those issues are exactly due to not having rescan
handled properly. What misbehavior are you seeing?

Hmm, that might just be it.

When I select from a view based on a function which returns a base type,
I only get the first row. When I select from a view which is based on a
function returning a composite type, it triggers an assertion. I've
traced the latter down to a slot pointer which is reset to NULL
somewhere.

Um, that's probably not it then. Rescan would only come into play for
a plan node that's being used as the inside of a join, or some other
contexts more complicated than this. A simple view ought to make no
difference at all in the generated plan --- perhaps there's some bit
of the planner that you missed teaching about function RTEs or
FunctionScan plan nodes?

Anyway, I plan to review and apply your patch today, if I don't run
into any major problems. Will look to see if I see a reason for the
view trouble.

regards, tom lane

Tom Lane wrote:

Um, that's probably not it then. Rescan would only come into play for
a plan node that's being used as the inside of a join, or some other
contexts more complicated than this. A simple view ought to make no
difference at all in the generated plan --- perhaps there's some bit
of the planner that you missed teaching about function RTEs or
FunctionScan plan nodes?

Anyway, I plan to review and apply your patch today, if I don't run
into any major problems. Will look to see if I see a reason for the
view trouble.

(Sorry for the slow response -- been out all day)

Actually I found late last night that when the view is used, the RTE is
a RangeVar, so the RangeFunction code never gets executed. So I think
your comment above is right on. That may well explain both problems.
I'll start looking again tonight.

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Actually I found late last night that when the view is used, the RTE is
a RangeVar, so the RangeFunction code never gets executed. So I think
your comment above is right on. That may well explain both problems.

Hmm. I thought your view problems were explained by the cut-and-pasteos
I noticed in _readRangeTblEntry. Maybe there's more though. I haven't
got to the point of trying to actually execute the patch ... will work
on it more today.

regards, tom lane

Joe Conway <mail@joeconway.com> writes:

Here's the patch, per my post to HACKERS.
It builds cleanly on my dev box, and passes all regression tests.

I've committed this with some revisions. The VIEW cases you were
worried about seem to work now. I think you'll find that
single-FROM-item cases generally work, and it's time to start worrying
about joins (ie, rescans).

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

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

and here again rescanning is going to be critical.

regards, tom lane

PS: test case for above:

create table foo(fooid int, f2 int);
insert into foo values(1, 11);
insert into foo values(2, 22);
insert into foo values(1, 111);

create function foot(int) returns setof foo as '
select * from foo where fooid = $1' language sql;

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

Here's the patch, per my post to HACKERS.
It builds cleanly on my dev box, and passes all regression tests.

I've committed this with some revisions. The VIEW cases you were
worried about seem to work now. I think you'll find that
single-FROM-item cases generally work, and it's time to start worrying
about joins (ie, rescans).

Thanks! I've been offline most of the weekend, but I can get back on
this now. I'll start work on the rescans and test cases below right
away. Were your revisions extensive? Any major misconceptions on my part?

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Were your revisions extensive? Any major misconceptions on my part?

I did a fair amount of polishing on the ExprContext callback stuff,
and removed or moved around some node fields that I thought were
unnecessary or in the wrong place. I also set up proper infrastructure
for cost estimation on function RTEs (though the estimates themselves
are still pretty lame). Nothing I'd call "major"... more in the
line of stylistic improvements...

regards, tom lane

Tom Lane wrote:

I've committed this with some revisions. The VIEW cases you were
worried about seem to work now. I think you'll find that
single-FROM-item cases generally work, and it's time to start worrying
about joins (ie, rescans).

Hi Tom,

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, or if the function was explicitly marked as
PM_MATERIALIZE. 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?

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?

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().

BTW, The test cases I was using previously now all pass (copy below).

Thanks,

Joe

DROP TABLE foo;
CREATE TABLE foo (fooid int, foosubid int, fooname text, primary
key(fooid,foosubid));
INSERT INTO foo VALUES(1,1,'Joe');
INSERT INTO foo VALUES(1,2,'Ed');
INSERT INTO foo VALUES(2,1,'Mary');

-- sql, proretset = f, prorettype = b
DROP FUNCTION getfoo(int);
CREATE FUNCTION getfoo(int) RETURNS int AS 'SELECT $1;' LANGUAGE SQL;
SELECT * FROM getfoo(1) AS t1;
DROP VIEW vw_getfoo;
CREATE VIEW vw_getfoo AS SELECT * FROM getfoo(1);
SELECT * FROM vw_getfoo;

-- sql, proretset = t, prorettype = b
DROP FUNCTION getfoo(int);
CREATE FUNCTION getfoo(int) RETURNS setof int AS 'SELECT fooid FROM foo
WHERE fooid = $1;' LANGUAGE SQL;
SELECT * FROM getfoo(1) AS t1;
DROP VIEW vw_getfoo;
CREATE VIEW vw_getfoo AS SELECT * FROM getfoo(1);
SELECT * FROM vw_getfoo;

-- sql, proretset = t, prorettype = b
DROP FUNCTION getfoo(int);
CREATE FUNCTION getfoo(int) RETURNS setof text AS 'SELECT fooname FROM
foo WHERE fooid = $1;' LANGUAGE SQL;
SELECT * FROM getfoo(1) AS t1;
DROP VIEW vw_getfoo;
CREATE VIEW vw_getfoo AS SELECT * FROM getfoo(1);
SELECT * FROM vw_getfoo;

-- sql, proretset = f, prorettype = c
DROP FUNCTION getfoo(int);
CREATE FUNCTION getfoo(int) RETURNS foo AS 'SELECT * FROM foo WHERE
fooid = $1;' LANGUAGE SQL;
SELECT * FROM getfoo(1) AS t1;
DROP VIEW vw_getfoo;
CREATE VIEW vw_getfoo AS SELECT * FROM getfoo(1);
SELECT * FROM vw_getfoo;

-- sql, proretset = t, prorettype = c
DROP FUNCTION getfoo(int);
CREATE FUNCTION getfoo(int) RETURNS setof foo AS 'SELECT * FROM foo
WHERE fooid = $1;' LANGUAGE SQL;
SELECT * FROM getfoo(1) AS t1;
DROP VIEW vw_getfoo;
CREATE VIEW vw_getfoo AS SELECT * FROM getfoo(1);
SELECT * FROM vw_getfoo;

-- C, proretset = f, prorettype = b
SELECT * FROM dblink_replace('123456789987654321', '99', 'HelloWorld');
DROP VIEW vw_dblink_replace;
CREATE VIEW vw_dblink_replace AS SELECT * FROM
dblink_replace('123456789987654321', '99', 'HelloWorld');
SELECT * FROM vw_dblink_replace;

-- C, proretset = t, prorettype = b
SELECT dblink_get_pkey FROM dblink_get_pkey('foo');
DROP VIEW vw_dblink_get_pkey;
CREATE VIEW vw_dblink_get_pkey AS SELECT dblink_get_pkey FROM
dblink_get_pkey('foo');
SELECT * FROM vw_dblink_get_pkey;

Joe Conway <mail@joeconway.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