Thread: pgsql: Fix calculation of plan node extParams to account for the
pgsql: Fix calculation of plan node extParams to account for the
From
tgl@postgresql.org (Tom Lane)
Date:
Log Message:
-----------
Fix calculation of plan node extParams to account for the possibility that one
initPlan sets a parameter for another. This could not (I think) happen before
8.1, but it's possible now because the initPlans generated by MIN/MAX
optimization might themselves use initPlans. We attach those initPlans as
siblings of the MIN/MAX ones, not children, to avoid duplicate computation
when multiple MIN/MAX aggregates are present; so this leads to the case of an
initPlan needing the result of a sibling initPlan, which is not possible with
ordinary query nesting. Hadn't been noticed because in most contexts having
too much stuff listed in extParam is fairly harmless. Fixes "plan should not
reference subplan's variable" bug reported by Catalin Pitis.
Tags:
----
REL8_1_STABLE
Modified Files:
--------------
pgsql/src/backend/optimizer/plan:
subselect.c (r1.100.2.2 -> r1.100.2.3)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/optimizer/plan/subselect.c.diff?r1=1.100.2.2&r2=1.100.2.3)
I trid the following two queries in the version before your patch:
(1) which is reported in the bug("plan should not reference subplan's
variable") reported by Catalin Pitis:
INSERT INTO PROJECT(PROJECT_ID,PROJECT_DESC)(SELECT
MAX(PROJECT_ID),'MYPROJECT' FROM PROJECT WHERE NOT EXISTS (
SELECT PROJECT_DESC FROM PROJECT WHERE PROJECT_DESC = 'MYPROJECT' ) );
it reported an error;
(2)
select * from project where (1,1) = (SELECT MAX(PROJECT_ID),1 FROM
PROJECT WHERE NOT EXISTS (SELECT PROJECT_DESC FROM PROJECT WHERE
PROJECT_DESC = 'MYPROJECT'))
but, there was no error at all!!
Then I noticed that when add IDs of all PARAM_EXEC params appearing in the
given expression tree to the result set:
(1) for subplans corresponding to a SubLink, it was processed like this:
/* in finalize_primnode */
if (is_subplan(node))
{
SubPlan *subplan = (SubPlan *) node;
/* Add outer-level params needed by the subplan to paramids */
context->paramids = bms_join(context->paramids,
bms_intersect(subplan->plan->extParam,
context->outer_params));
/* fall through to recurse into subplan args */
}
Attention: there's a bms_intersect op here before bms_join.
(2) but for subplans correspoonding to a RangeTable, say SubqueryScan, it
was processed like this:
/* in finalize_plan */
case T_SubqueryScan:
/*
* In a SubqueryScan, SS_finalize_plan has already been run on the
* subplan by the inner invocation of subquery_planner, so there's
* no need to do it again. Instead, just pull out the subplan's
* extParams list, which represents the params it needs from my
* level and higher levels.
*/
context.paramids = bms_add_members(context.paramids,
((SubqueryScan *) plan)->subplan->extParam);
break;
Attention: there's no bms_intersect .
So, my question is why not just add a bms_intersect in the second occasion
just like the first one? Do we need to change so much?
"Tom Lane" <tgl@postgresql.org> д����Ϣ����
:20060503002507.562419FB1F8@postgresql.org...
> Log Message:
> -----------
> Fix calculation of plan node extParams to account for the possibility that
one
> initPlan sets a parameter for another. This could not (I think) happen
before
> 8.1, but it's possible now because the initPlans generated by MIN/MAX
> optimization might themselves use initPlans. We attach those initPlans as
> siblings of the MIN/MAX ones, not children, to avoid duplicate computation
> when multiple MIN/MAX aggregates are present; so this leads to the case of
an
> initPlan needing the result of a sibling initPlan, which is not possible
with
> ordinary query nesting. Hadn't been noticed because in most contexts
having
> too much stuff listed in extParam is fairly harmless. Fixes "plan should
not
> reference subplan's variable" bug reported by Catalin Pitis.
>
> Tags:
> ----
> REL8_1_STABLE
>
> Modified Files:
> --------------
> pgsql/src/backend/optimizer/plan:
> subselect.c (r1.100.2.2 -> r1.100.2.3)
>
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/optimizer/plan
/subselect.c.diff?r1=1.100.2.2&r2=1.100.2.3)
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>
"Jackie Leng" <lengjianquan@163.com> writes:
> So, my question is why not just add a bms_intersect in the second occasion
> just like the first one? Do we need to change so much?
finalize_plan already has a bms_intersect, but it's further down in the
routine (to share code instead of duplicating it in each of the switch
cases) --- in CVS HEAD, line 1199. This is not relevant to the patch
AFAICS.
regards, tom lane
Yes, finalize_plan really has a bms_intersect, but it's used to compute
"plan->extParam":
/* Now we have all the paramids */
if (!bms_is_subset(context.paramids, valid_params))
elog(ERROR, "plan should not reference subplan's variable");
plan->extParam = bms_intersect(context.paramids, outer_params);
plan->allParam = context.paramids;
But what I means is that when we compute "context.paramids", we can filter
out those params refering to a sibling initPlan through bms_intersect, just
like the process to subplan in "finalize_primnode". So, we can change the
following code in finalize_plan. I think it's equivalent to your methods:
case T_SubqueryScan:
/*
* In a SubqueryScan, SS_finalize_plan has already been run on the
* subplan by the inner invocation of subquery_planner, so there's
* no need to do it again. Instead, just pull out the subplan's
* extParams list, which represents the params it needs from my
* level and higher levels.
*/
context.paramids = bms_add_members(context.paramids,
bms_intersect(((SubqueryScan *) plan)->subplan->extParam,
outer_params)); << this line was changed >>
break;
Am I right?
Thank you! :)
"Tom Lane" <tgl@sss.pgh.pa.us> д����Ϣ����
:21744.1149198794@sss.pgh.pa.us...
> "Jackie Leng" <lengjianquan@163.com> writes:
> > So, my question is why not just add a bms_intersect in the second
occasion
> > just like the first one? Do we need to change so much?
>
> finalize_plan already has a bms_intersect, but it's further down in the
> routine (to share code instead of duplicating it in each of the switch
> cases) --- in CVS HEAD, line 1199. This is not relevant to the patch
> AFAICS.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match
>