pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18859: ERROR: unexpected plan node type: 356
7+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2025-03-21 7fe312f609 0f60e1fba1 1353b1161a a28c1fb619 5e56efa7c0 c196c610b3 Fix plpgsql's handling of simple expressions in scrollable cursors.
* BUG #18859: ERROR: unexpected plan node type: 356
@ 2025-03-20 13:53 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 7+ messages in thread
From: PG Bug reporting form @ 2025-03-20 13:53 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: splarv@ya.ru
The following bug has been logged on the website:
Bug reference: 18859
Logged by: Olleg Samoylov
Email address: splarv@ya.ru
PostgreSQL version: 17.4
Operating system: RedOS 8
Description:
This working.
postgres=> do $$ declare p_CurData refcursor; begin OPEN p_CurData FOR
SELECT NULL::int id; end;$$;
DO
But this is not. Internal error.
postgres=> do $$ declare p_CurData refcursor; begin OPEN p_CurData SCROLL
FOR SELECT NULL::int id; end;$$;
ERROR: unexpected plan node type: 356
CONTEXT: PL/pgSQL function inline_code_block line 1 at OPEN
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #18859: ERROR: unexpected plan node type: 356
@ 2025-03-21 12:58 Andrei Lepikhov <lepihov@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 7+ messages in thread
From: Andrei Lepikhov @ 2025-03-21 12:58 UTC (permalink / raw)
To: splarv@ya.ru, pgsql-bugs@lists.postgresql.org, PG Bug reporting form <noreply@postgresql.org>
On 20/3/2025 14:53, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 18859
> Logged by: Olleg Samoylov
> Email address: splarv@ya.ru
> PostgreSQL version: 17.4
> Operating system: RedOS 8
> Description:
>
> This working.
> postgres=> do $$ declare p_CurData refcursor; begin OPEN p_CurData FOR
> SELECT NULL::int id; end;$$;
> DO
> But this is not. Internal error.
> postgres=> do $$ declare p_CurData refcursor; begin OPEN p_CurData SCROLL
> FOR SELECT NULL::int id; end;$$;
> ERROR: unexpected plan node type: 356
> CONTEXT: PL/pgSQL function inline_code_block line 1 at OPEN
>
It may be reproduced with any expression, treated as a simple
expression. For example:
DO $$
DECLARE
p_CurData refcursor;
BEGIN
OPEN p_CurData SCROLL FOR SELECT now();
END; $$;
The problem here is in the scrollable cursors code which inserts
Material node:
if (cursorOptions & CURSOR_OPT_SCROLL)
{
if (!ExecSupportsBackwardScan(top_plan))
top_plan = materialize_finished_plan(top_plan);
}
But the exec_save_simple_expr() doesn't process Material node and causes
the ERROR.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #18859: ERROR: unexpected plan node type: 356
@ 2025-03-21 13:00 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrei Lepikhov <lepihov@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Tom Lane @ 2025-03-21 13:00 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: splarv@ya.ru; pgsql-bugs@lists.postgresql.org
Andrei Lepikhov <lepihov@gmail.com> writes:
> The problem here is in the scrollable cursors code which inserts
> Material node:
Yeah, I'd just come to the same conclusion. I guess we can make
this code look through a Material node as well as Gather.
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #18859: ERROR: unexpected plan node type: 356
@ 2025-03-21 14:49 Andrei Lepikhov <lepihov@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 7+ messages in thread
From: Andrei Lepikhov @ 2025-03-21 14:49 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: splarv@ya.ru; pgsql-bugs@lists.postgresql.org
On 21/3/2025 14:00, Tom Lane wrote:
> Andrei Lepikhov <lepihov@gmail.com> writes:
>> The problem here is in the scrollable cursors code which inserts
>> Material node:
>
> Yeah, I'd just come to the same conclusion. I guess we can make
> this code look through a Material node as well as Gather.
Yes, as I see there are no additional corner cases. See the code in
attachment.
--
regards, Andrei Lepikhov
Attachments:
scrollable-fix.diff (text/plain, 2.1K) download
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index d4377ceecbf..f3691eb2f58 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8306,6 +8306,8 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan)
* Gather node. The Gather node's tlist would normally contain a Var
* referencing the child node's output, but it could also be a Param, or
* it could be a Const that setrefs.c copied as-is.
+ * Also, in case of scrollable cursor it contains Material node at the top
+ * of the plan tree.
*/
plan = stmt->planTree;
for (;;)
@@ -8338,6 +8340,15 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan)
/* Descend to the child node */
plan = plan->lefttree;
}
+ else if (IsA(plan, Material))
+ {
+ Assert(plan->lefttree != NULL &&
+ plan->righttree == NULL &&
+ plan->initPlan == NULL &&
+ plan->qual == NULL);
+ Assert(plan == stmt->planTree);
+ plan = plan->lefttree;
+ }
else
elog(ERROR, "unexpected plan node type: %d",
(int) nodeTag(plan));
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index d8ce39dba3c..0e5435ebdfc 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3656,6 +3656,13 @@ $$ language plpgsql;
ERROR: cursor FOR loop must use a bound cursor variable
LINE 5: for r in c loop
^
+-- Test scrollable cursor and simple expression optimisation
+DO $$
+DECLARE
+ p_CurData refcursor;
+BEGIN
+ OPEN p_CurData SCROLL FOR SELECT NULL;
+END; $$;
-- test RETURN QUERY EXECUTE
create or replace function return_dquery()
returns setof int as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index d413d995d17..7797beb6e03 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -3034,6 +3034,14 @@ begin
end;
$$ language plpgsql;
+-- Test scrollable cursor and simple expression optimisation
+DO $$
+DECLARE
+ p_CurData refcursor;
+BEGIN
+ OPEN p_CurData SCROLL FOR SELECT NULL;
+END; $$;
+
-- test RETURN QUERY EXECUTE
create or replace function return_dquery()
* Re: BUG #18859: ERROR: unexpected plan node type: 356
@ 2025-03-21 15:09 Richard Guo <guofenglinux@gmail.com>
parent: Andrei Lepikhov <lepihov@gmail.com>
1 sibling, 1 reply; 7+ messages in thread
From: Richard Guo @ 2025-03-21 15:09 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; splarv@ya.ru; pgsql-bugs@lists.postgresql.org
On Fri, Mar 21, 2025 at 11:49 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
> On 21/3/2025 14:00, Tom Lane wrote:
> > Andrei Lepikhov <lepihov@gmail.com> writes:
> >> The problem here is in the scrollable cursors code which inserts
> >> Material node:
> > Yeah, I'd just come to the same conclusion. I guess we can make
> > this code look through a Material node as well as Gather.
> Yes, as I see there are no additional corner cases. See the code in
> attachment.
A Material's tlist could also be a Const copied up by setrefs.c, in
which case we can avoid looking further, similar to what Gather does.
I wonder if we could have Material use the same handle as Gather.
- else if (IsA(plan, Gather))
+ else if (IsA(plan, Gather) || IsA(plan, Material))
Thanks
Richard
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #18859: ERROR: unexpected plan node type: 356
@ 2025-03-21 15:35 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrei Lepikhov <lepihov@gmail.com>
1 sibling, 0 replies; 7+ messages in thread
From: Tom Lane @ 2025-03-21 15:35 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: splarv@ya.ru; pgsql-bugs@lists.postgresql.org
Andrei Lepikhov <lepihov@gmail.com> writes:
> On 21/3/2025 14:00, Tom Lane wrote:
>> Yeah, I'd just come to the same conclusion. I guess we can make
>> this code look through a Material node as well as Gather.
> Yes, as I see there are no additional corner cases. See the code in
> attachment.
I think actually we want to use the same processing as for Gather,
in particular the check for a Const. That's all about what
setrefs.c will do to the plan tree, and it'll do the same things
to Material as it would to Gather. Since that stanza doesn't
actually do anything that's specific to Gather, the code change
can be as simple as
@@ -8323,7 +8325,7 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan)
((Result *) plan)->resconstantqual == NULL);
break;
}
- else if (IsA(plan, Gather))
+ else if (IsA(plan, Gather) || IsA(plan, Material))
{
Assert(plan->lefttree != NULL &&
plan->righttree == NULL &&
I fixed that, did some cosmetic fooling with the comment
and test case, and pushed it. Thanks for the report,
and for the patch!
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #18859: ERROR: unexpected plan node type: 356
@ 2025-03-21 15:36 Tom Lane <tgl@sss.pgh.pa.us>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 0 replies; 7+ messages in thread
From: Tom Lane @ 2025-03-21 15:36 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Andrei Lepikhov <lepihov@gmail.com>; splarv@ya.ru; pgsql-bugs@lists.postgresql.org
Richard Guo <guofenglinux@gmail.com> writes:
> A Material's tlist could also be a Const copied up by setrefs.c, in
> which case we can avoid looking further, similar to what Gather does.
> I wonder if we could have Material use the same handle as Gather.
Right, I'd come to the same conclusion before seeing your message.
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2025-03-21 15:36 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-03-20 13:53 BUG #18859: ERROR: unexpected plan node type: 356 PG Bug reporting form <noreply@postgresql.org>
2025-03-21 12:58 ` Re: BUG #18859: ERROR: unexpected plan node type: 356 Andrei Lepikhov <lepihov@gmail.com>
2025-03-21 13:00 ` Re: BUG #18859: ERROR: unexpected plan node type: 356 Tom Lane <tgl@sss.pgh.pa.us>
2025-03-21 14:49 ` Re: BUG #18859: ERROR: unexpected plan node type: 356 Andrei Lepikhov <lepihov@gmail.com>
2025-03-21 15:09 ` Re: BUG #18859: ERROR: unexpected plan node type: 356 Richard Guo <guofenglinux@gmail.com>
2025-03-21 15:36 ` Re: BUG #18859: ERROR: unexpected plan node type: 356 Tom Lane <tgl@sss.pgh.pa.us>
2025-03-21 15:35 ` Re: BUG #18859: ERROR: unexpected plan node type: 356 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