I have been looking at the planner's handling of subplans, and I see
something that I think is wrong, but I'm not quite certain.  In
_make_subplan() in backend/optimizer/plan/subselect.c, there is the
code

    /* make parParam list */
    foreach(lst, plan->extParam)
    {
        Var      *var = nth(lfirsti(lst), PlannerParamVar);

        if (var->varlevelsup == PlannerQueryLevel)
            node->parParam = lappendi(node->parParam, lfirsti(lst));
    }

It looks to me like this code is supposed to find parameters that
reference the immediate parent plan level, as opposed to higher levels.
So, shouldn't it be looking for varlevelsup == 1, not PlannerQueryLevel?

For a first-level subplan, PlannerQueryLevel will be 1 at the time
this code runs, so the result is the same anyway.  But I think it
does the wrong thing for more deeply nested subplans.  Am I right?

			regards, tom lane

Wed, 16 Jun 1999 19:02:05 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Apparent bug in _make_subplan
Tom Lane wrote:
> 
>     /* make parParam list */
>     foreach(lst, plan->extParam)
>     {
>         Var      *var = nth(lfirsti(lst), PlannerParamVar);
> 
>         if (var->varlevelsup == PlannerQueryLevel)
>             node->parParam = lappendi(node->parParam, lfirsti(lst));
>     }
> 
> It looks to me like this code is supposed to find parameters that
> reference the immediate parent plan level, as opposed to higher levels.
> So, shouldn't it be looking for varlevelsup == 1, not PlannerQueryLevel?
> 
> For a first-level subplan, PlannerQueryLevel will be 1 at the time
> this code runs, so the result is the same anyway.  But I think it

PlannerQueryLevel will be 0 here - subselect.c:140

    /* and now we are parent again */
    PlannerInitPlan = saved_ip;
    PlannerQueryLevel--;

> does the wrong thing for more deeply nested subplans.  Am I right?

I'm not sure. Seems that I made assumption here that 
varlevelsup is _absolute_ level number and seems that
_replace_var() and _new_param() replace parser' varlevelsup
with absolute level value.

Vadim

Thu, 17 Jun 1999 09:49:50 +0800
Vadim Mikheev <vadim@krs.ru>
Re: Apparent bug in _make_subplan
Vadim Mikheev <vadim@krs.ru> writes:
>> For a first-level subplan, PlannerQueryLevel will be 1 at the time
>> this code runs, so the result is the same anyway.  But I think it

> PlannerQueryLevel will be 0 here - subselect.c:140

No, it's never 0.  It starts out 1 in planner(), and _make_subplan
increments it at line 116 before recursing, then decrements again at
line 142.  So it's at least one when we arrive at the parParam code.

> I'm not sure. Seems that I made assumption here that 
> varlevelsup is _absolute_ level number and seems that
> _replace_var() and _new_param() replace parser' varlevelsup
> with absolute level value.

After looking through all the references to varlevelsup, it's clear
that all pieces of the system *except* subselect.c treat varlevelsup
as a relative level number, so-many-levels-out-from-current-subplan.
subselect.c has a couple of places that think nonzero varlevelsup
is an absolute level number, with 1 as the top plan.  This is certainly
a source of bugs --- it happens to work for two-level plans, but will
fail for anything more deeply nested.  I will work on fixing subselect.c
to bring it in line with the rest of the world...

			regards, tom lane

Thu, 17 Jun 1999 10:43:44 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Re: Apparent bug in _make_subplan 
> After looking through all the references to varlevelsup, it's clear
> that all pieces of the system *except* subselect.c treat varlevelsup
> as a relative level number, so-many-levels-out-from-current-subplan.
> subselect.c has a couple of places that think nonzero varlevelsup
> is an absolute level number, with 1 as the top plan.  This is certainly
> a source of bugs --- it happens to work for two-level plans, but will
> fail for anything more deeply nested.  I will work on fixing subselect.c
> to bring it in line with the rest of the world...

varlevelsup was always intended to be relative.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Thu, 17 Jun 1999 10:56:17 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: Apparent bug in _make_subplan
Tom Lane wrote:
> 
> > I'm not sure. Seems that I made assumption here that
> > varlevelsup is _absolute_ level number and seems that
> > _replace_var() and _new_param() replace parser' varlevelsup
> > with absolute level value.
> 
> After looking through all the references to varlevelsup, it's clear
> that all pieces of the system *except* subselect.c treat varlevelsup
> as a relative level number, so-many-levels-out-from-current-subplan.
> subselect.c has a couple of places that think nonzero varlevelsup
> is an absolute level number, with 1 as the top plan.  This is certainly
> a source of bugs --- it happens to work for two-level plans, but will
> fail for anything more deeply nested.  I will work on fixing subselect.c
> to bring it in line with the rest of the world...

subselect.c uses varlevelsup as absolute level number only
for correlation vars <--> params mapping, so why should it be
source of bugs? SS_replace_correlation_vars replaces all
correlation vars with parameters. Vars with absolute varlevelsup
are in PlannerParamVar only. To identify correlation vars and
to know is parameter already assigned to a var we obviously
need in absolute level number.

Vadim

Fri, 18 Jun 1999 12:10:57 +0800
Vadim Mikheev <vadim@krs.ru>
Re: [HACKERS] Re: Apparent bug in _make_subplan
> subselect.c uses varlevelsup as absolute level number only
> for correlation vars <--> params mapping, so why should it be
> source of bugs? SS_replace_correlation_vars replaces all
> correlation vars with parameters. Vars with absolute varlevelsup
> are in PlannerParamVar only. To identify correlation vars and
> to know is parameter already assigned to a var we obviously
> need in absolute level number.

But the varlevelsup I pass in from the parser are relative to the
current level, not absolute. 

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 18 Jun 1999 00:43:07 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: Apparent bug in _make_subplan
Bruce Momjian wrote:
> 
> > subselect.c uses varlevelsup as absolute level number only
> > for correlation vars <--> params mapping, so why should it be
> > source of bugs? SS_replace_correlation_vars replaces all
> > correlation vars with parameters. Vars with absolute varlevelsup
> > are in PlannerParamVar only. To identify correlation vars and
> > to know is parameter already assigned to a var we obviously
> > need in absolute level number.
> 
> But the varlevelsup I pass in from the parser are relative to the
> current level, not absolute.

subselect.c takes it into account, computes absolute numbers
and stores them in PlannerParamVar only...

Vadim

Fri, 18 Jun 1999 14:28:30 +0800
Vadim Mikheev <vadim@krs.ru>
Re: [HACKERS] Re: Apparent bug in _make_subplan
Vadim Mikheev <vadim@krs.ru> writes:
>> But the varlevelsup I pass in from the parser are relative to the
>> current level, not absolute.

> subselect.c takes it into account, computes absolute numbers
> and stores them in PlannerParamVar only...

Right, I eventually figured that out, and I see that it's probably the
best way.  I have added the following documentation to subselect.c:

/*--------------------
 * PlannerParamVar is a list of Var nodes, wherein the n'th entry
 * (n counts from 0) corresponds to Param->paramid = n.  The Var nodes
 * are ordinary except for one thing: their varlevelsup field does NOT
 * have the usual interpretation of "subplan levels out from current".
 * Instead, it contains the absolute plan level, with the outermost
 * plan being level 1 and nested plans having higher level numbers.
 * This nonstandardness is useful because we don't have to run around
 * and update the list elements when we enter or exit a subplan
 * recursion level.  But we must pay attention not to confuse this
 * meaning with the normal meaning of varlevelsup.
 *--------------------
 */

along with other changes that I will commit once I get subselects in
HAVING working right ...

			regards, tom lane

Fri, 18 Jun 1999 10:19:44 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Re: Apparent bug in _make_subplan 
> Bruce Momjian wrote:
> > 
> > > subselect.c uses varlevelsup as absolute level number only
> > > for correlation vars <--> params mapping, so why should it be
> > > source of bugs? SS_replace_correlation_vars replaces all
> > > correlation vars with parameters. Vars with absolute varlevelsup
> > > are in PlannerParamVar only. To identify correlation vars and
> > > to know is parameter already assigned to a var we obviously
> > > need in absolute level number.
> > 
> > But the varlevelsup I pass in from the parser are relative to the
> > current level, not absolute.
> 
> subselect.c takes it into account, computes absolute numbers
> and stores them in PlannerParamVar only...

Oh.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 18 Jun 1999 12:47:35 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: Apparent bug in _make_subplan