pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery
6+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2022-05-12 79b58c6f68 301b91c565 7f7f1750d4 b53442f6fe b7579b25c8 ac51c9fba5 Make pull_var_clause() handle GroupingFuncs exactly like Aggrefs.
* BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery
@ 2022-05-10 01:02 PG Bug reporting form <noreply@postgresql.org>
2022-05-10 10:07 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: PG Bug reporting form @ 2022-05-10 01:02 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: sully@msully.net
The following bug has been logged on the website:
Bug reference: 17479
Logged by: Michael J. Sullivan
Email address: sully@msully.net
PostgreSQL version: 14.2
Operating system: Linux
Description:
The following query produces "plan should not reference subplan's
variable"
create table Card (id uuid);
SELECT
-- This line causes "variable not found in subplan target list"
-- grouping(res.cnt)
-- This line causes "plan should not reference subplan's variable"
(SELECT grouping(res.cnt))
FROM Card
CROSS JOIN LATERAL
(SELECT
(SELECT Card.id) AS cnt
) AS res
GROUP BY
res.cnt
As the comment says, a slight change instead errors with "variable not found
in subplan target list".
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery
2022-05-10 01:02 BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery PG Bug reporting form <noreply@postgresql.org>
@ 2022-05-10 10:07 ` Richard Guo <guofenglinux@gmail.com>
2022-05-10 11:05 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guo @ 2022-05-10 10:07 UTC (permalink / raw)
To: sully@msully.net, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, May 10, 2022 at 2:12 PM PG Bug reporting form <
noreply@postgresql.org> wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17479
> Logged by: Michael J. Sullivan
> Email address: sully@msully.net
> PostgreSQL version: 14.2
> Operating system: Linux
> Description:
>
> The following query produces "plan should not reference subplan's
> variable"
>
> create table Card (id uuid);
>
> SELECT
> -- This line causes "variable not found in subplan target list"
> -- grouping(res.cnt)
> -- This line causes "plan should not reference subplan's variable"
> (SELECT grouping(res.cnt))
> FROM Card
> CROSS JOIN LATERAL
> (SELECT
> (SELECT Card.id) AS cnt
> ) AS res
> GROUP BY
> res.cnt
>
> As the comment says, a slight change instead errors with "variable not
> found
> in subplan target list".
>
Reproduced this issue on HEAD:
# explain (verbose, costs off)
SELECT grouping(res.cnt) FROM Card CROSS JOIN LATERAL (SELECT (SELECT
Card.id) AS cnt) AS res GROUP BY res.cnt;
ERROR: variable not found in subplan target list
For this query, initially it has two TargetEntrys and both referencing
the RangeTblEntry of the subquery.
{TARGETENTRY
:expr
{GROUPINGFUNC
:args (
{VAR
:varno 2
:varattno 1
AND
{TARGETENTRY
:expr
{VAR
:varno 2
:varattno 1
More specifically, they are both referencing the first TargetEntry from
the subquery. And the first TargetEntry of the subquery is of type
EXPR SubLink. So after we pull up this subquery, the two TargetEntrys
become:
{TARGETENTRY
:expr
{GROUPINGFUNC
:args (
{SUBLINK
:subLinkType 4
:subLinkId 0
AND
{TARGETENTRY
:expr
{SUBLINK
:subLinkType 4
:subLinkId 0
Actually the two SubLink expressions are totally the same. But we did
not check that and proceeded to expand them to two SubPlans.
{TARGETENTRY
:expr
{GROUPINGFUNC
:args (
{SUBPLAN
:subLinkType 4
:testexpr <>
:paramIds <>
:plan_id 1
:plan_name SubPlan\ 1
AND
{TARGETENTRY
:expr
{SUBPLAN
:subLinkType 4
:testexpr <>
:paramIds <>
:plan_id 2
:plan_name SubPlan\ 2
The two SubPlans are assigned with different plan_ids/plan_names.
That's why when we fix up the GROUPINGFUNC target entry we failed to
match the whole SubPlan expression, i.e. we failed to match 'SubPlan 1'
against 'SubPlan 2'.
Thanks
Richard
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery
2022-05-10 01:02 BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery PG Bug reporting form <noreply@postgresql.org>
2022-05-10 10:07 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
@ 2022-05-10 11:05 ` Richard Guo <guofenglinux@gmail.com>
2022-05-11 23:11 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guo @ 2022-05-10 11:05 UTC (permalink / raw)
To: sully@msully.net, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, May 10, 2022 at 6:07 PM Richard Guo <guofenglinux@gmail.com> wrote:
>
> On Tue, May 10, 2022 at 2:12 PM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>
>> The following bug has been logged on the website:
>>
>> Bug reference: 17479
>> Logged by: Michael J. Sullivan
>> Email address: sully@msully.net
>> PostgreSQL version: 14.2
>> Operating system: Linux
>> Description:
>>
>> The following query produces "plan should not reference subplan's
>> variable"
>>
>> create table Card (id uuid);
>>
>> SELECT
>> -- This line causes "variable not found in subplan target list"
>> -- grouping(res.cnt)
>> -- This line causes "plan should not reference subplan's variable"
>> (SELECT grouping(res.cnt))
>> FROM Card
>> CROSS JOIN LATERAL
>> (SELECT
>> (SELECT Card.id) AS cnt
>> ) AS res
>> GROUP BY
>> res.cnt
>>
>> As the comment says, a slight change instead errors with "variable not
>> found
>> in subplan target list".
>>
>
> Reproduced this issue on HEAD:
>
> # explain (verbose, costs off)
> SELECT grouping(res.cnt) FROM Card CROSS JOIN LATERAL (SELECT (SELECT
> Card.id) AS cnt) AS res GROUP BY res.cnt;
> ERROR: variable not found in subplan target list
>
> For this query, initially it has two TargetEntrys and both referencing
> the RangeTblEntry of the subquery.
>
> {TARGETENTRY
> :expr
> {GROUPINGFUNC
> :args (
> {VAR
> :varno 2
> :varattno 1
>
>
> AND
>
> {TARGETENTRY
> :expr
> {VAR
> :varno 2
> :varattno 1
>
> More specifically, they are both referencing the first TargetEntry from
> the subquery. And the first TargetEntry of the subquery is of type
> EXPR SubLink. So after we pull up this subquery, the two TargetEntrys
> become:
>
> {TARGETENTRY
> :expr
> {GROUPINGFUNC
> :args (
> {SUBLINK
> :subLinkType 4
> :subLinkId 0
>
> AND
>
> {TARGETENTRY
> :expr
> {SUBLINK
> :subLinkType 4
> :subLinkId 0
>
>
> Actually the two SubLink expressions are totally the same. But we did
> not check that and proceeded to expand them to two SubPlans.
>
> {TARGETENTRY
> :expr
> {GROUPINGFUNC
> :args (
> {SUBPLAN
> :subLinkType 4
> :testexpr <>
> :paramIds <>
> :plan_id 1
> :plan_name SubPlan\ 1
>
> AND
>
> {TARGETENTRY
> :expr
> {SUBPLAN
> :subLinkType 4
> :testexpr <>
> :paramIds <>
> :plan_id 2
> :plan_name SubPlan\ 2
>
> The two SubPlans are assigned with different plan_ids/plan_names.
> That's why when we fix up the GROUPINGFUNC target entry we failed to
> match the whole SubPlan expression, i.e. we failed to match 'SubPlan 1'
> against 'SubPlan 2'.
>
When we generate PathTarget for initial input to grouping nodes in
make_group_input_target(), for non-grouping columns we would pull out
all the Vars they mention with the help of pull_var_clause(), and add
them to the input target. But this ignores the Vars included inside
GroupingFunc node, even with flag PVC_RECURSE_AGGREGATES.
If we change that and include the Vars inside GroupingFunc node, we
would be able to fix up the GROUPINGFUNC target entry correctly by
walking into the GroupingFunc->args and matching the Vars there.
inline patch 1: src/backend/optimizer/util/var.c (+0/-0 lines) download inline-1.patch
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -661,13 +661,7 @@ pull_var_clause_walker(Node *node,
* Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery
2022-05-10 01:02 BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery PG Bug reporting form <noreply@postgresql.org>
2022-05-10 10:07 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
2022-05-10 11:05 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
@ 2022-05-11 23:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-05-12 07:51 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2022-05-11 23:11 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: sully@msully.net, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Richard Guo <guofenglinux@gmail.com> writes:
>> Actually the two SubLink expressions are totally the same. But we did
>> not check that and proceeded to expand them to two SubPlans.
Right. It'd be good to improve on that situation somehow, but I doubt
that that line of thought will lead to anything backpatchable.
> When we generate PathTarget for initial input to grouping nodes in
> make_group_input_target(), for non-grouping columns we would pull out
> all the Vars they mention with the help of pull_var_clause(), and add
> them to the input target. But this ignores the Vars included inside
> GroupingFunc node, even with flag PVC_RECURSE_AGGREGATES.
> If we change that and include the Vars inside GroupingFunc node, we
> would be able to fix up the GROUPINGFUNC target entry correctly by
> walking into the GroupingFunc->args and matching the Vars there.
At first I didn't like this solution at all, but on reflection I think
it's okay. The existing comment there is wrong because it neglects
the fact that whatever's inside the GROUPING construct has to be amenable
to later processing by setrefs.c and possibly EXPLAIN. So we cannot
have Vars there that aren't output by the lower plan nodes, which is the
optimization the comment is incorrectly thinking can work. This code
has accidentally worked so far because whatever's inside GROUPING must
duplicate a GROUP BY clause elsewhere, which'll result in the same Vars
getting propagated up the tree as far as the plan node responsible for
grouping. But once you phrase it that way, the fragility become obvious:
what if the GROUPING construct is placed above whatever node does the
grouping? I think it might be possible to break it today, even without
using SubPlans; and if you can't, it's only because the planner is leaving
money on the table by not aggressively pruning the output tlists for upper
plan nodes.
So I now agree that the thing to do is make this logic work the same for
GroupingFunc as it does for Aggref (and incidentally make the comment
about that less wishy-washy). The apparent savings of not pulling the
contained references is illusory, because we'll end up with exactly
the same plan tree --- or else a broken plan tree.
We may have more to do here, though, because with this patch I get
explain verbose
SELECT
grouping(res.cnt)
FROM Card
CROSS JOIN LATERAL
(SELECT
(SELECT Card.id) AS cnt
) AS res
GROUP BY
res.cnt;
HashAggregate (cost=67.38..71.88 rows=200 width=8)
Output: GROUPING((SubPlan 1)), ((SubPlan 2))
Group Key: (SubPlan 2)
-> Seq Scan on public.card (cost=0.00..61.00 rows=2550 width=8)
Output: (SubPlan 2), card.id
SubPlan 2
-> Result (cost=0.00..0.01 rows=1 width=4)
Output: card.id
What became of SubPlan 1? Maybe this is fine but it looks a little
shaky. We should at least run down why that's happening and make
sure we're not leaving dangling pointers anywhere.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery
2022-05-10 01:02 BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery PG Bug reporting form <noreply@postgresql.org>
2022-05-10 10:07 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
2022-05-10 11:05 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
2022-05-11 23:11 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Tom Lane <tgl@sss.pgh.pa.us>
@ 2022-05-12 07:51 ` Richard Guo <guofenglinux@gmail.com>
2022-05-12 14:51 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guo @ 2022-05-12 07:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: sully@msully.net, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, May 12, 2022 at 7:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> We may have more to do here, though, because with this patch I get
>
> explain verbose
> SELECT
> grouping(res.cnt)
> FROM Card
> CROSS JOIN LATERAL
> (SELECT
> (SELECT Card.id) AS cnt
> ) AS res
> GROUP BY
> res.cnt;
>
> HashAggregate (cost=67.38..71.88 rows=200 width=8)
> Output: GROUPING((SubPlan 1)), ((SubPlan 2))
> Group Key: (SubPlan 2)
> -> Seq Scan on public.card (cost=0.00..61.00 rows=2550 width=8)
> Output: (SubPlan 2), card.id
> SubPlan 2
> -> Result (cost=0.00..0.01 rows=1 width=4)
> Output: card.id
>
> What became of SubPlan 1? Maybe this is fine but it looks a little
> shaky. We should at least run down why that's happening and make
> sure we're not leaving dangling pointers anywhere.
>
I did some debug on this. The 'SubPlan 1' is not explained because its
SubPlanState is not created and added to AggState->subPlan.
When we compile each tlist column for the agg node, the GroupingFunc
expressions would not be collected to AggState->args, because only
Aggref nodes are expected there. As a result, the 'SubPlan 1', as the
arg of the GroupingFunc node, does not have a chance to be initialized
by ExecInitSubPlan() and added to AggState->subPlan when evaluating
arguments to aggregate function in ExecBuildAggTrans().
Not sure if this is fine or not.
Thanks
Richard
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery
2022-05-10 01:02 BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery PG Bug reporting form <noreply@postgresql.org>
2022-05-10 10:07 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
2022-05-10 11:05 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
2022-05-11 23:11 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Tom Lane <tgl@sss.pgh.pa.us>
2022-05-12 07:51 ` Re: BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery Richard Guo <guofenglinux@gmail.com>
@ 2022-05-12 14:51 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 6+ messages in thread
From: Tom Lane @ 2022-05-12 14:51 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: sully@msully.net, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Richard Guo <guofenglinux@gmail.com> writes:
> On Thu, May 12, 2022 at 7:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> What became of SubPlan 1? Maybe this is fine but it looks a little
>> shaky. We should at least run down why that's happening and make
>> sure we're not leaving dangling pointers anywhere.
> I did some debug on this. The 'SubPlan 1' is not explained because its
> SubPlanState is not created and added to AggState->subPlan.
Yeah, concur: there are two subplans in the plan tree, so no dangling
pointers.
> Not sure if this is fine or not.
It seems cosmetic. We could somehow force initialization of the
GroupingFunc's argument tree, but that would just be wasted cycles
in non-EXPLAIN cases, and it seems semantically wrong anyway given
that evaluation of the argument is not supposed to happen. (Right
now that's hypothetical, but what if ExecInitExpr started to
pre-evaluate some things?)
Given the history of this thing, maybe we will find something that
depends on such initialization happening. But without evidence
for that I'm not inclined to add code for it.
I'll run with the patch as you suggested.
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2022-05-12 14:51 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-05-10 01:02 BUG #17479: "plan should not reference subplan's variable" when calling `grouping` on result of subquery PG Bug reporting form <noreply@postgresql.org>
2022-05-10 10:07 ` Richard Guo <guofenglinux@gmail.com>
2022-05-10 11:05 ` Richard Guo <guofenglinux@gmail.com>
2022-05-11 23:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-05-12 07:51 ` Richard Guo <guofenglinux@gmail.com>
2022-05-12 14:51 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox