Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix plpgsql's handling of simple expressions in scrollable cursors.

  1. BUG #18859: ERROR: unexpected plan node type: 356

    PG Bug reporting form <noreply@postgresql.org> — 2025-03-20T13:53:59Z

    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

  2. Re: BUG #18859: ERROR: unexpected plan node type: 356

    Andrei Lepikhov <lepihov@gmail.com> — 2025-03-21T12:58:54Z

    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

  3. Re: BUG #18859: ERROR: unexpected plan node type: 356

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-21T13:00:53Z

    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

  4. Re: BUG #18859: ERROR: unexpected plan node type: 356

    Andrei Lepikhov <lepihov@gmail.com> — 2025-03-21T14:49:40Z

    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

  5. Re: BUG #18859: ERROR: unexpected plan node type: 356

    Richard Guo <guofenglinux@gmail.com> — 2025-03-21T15:09:26Z

    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

  6. Re: BUG #18859: ERROR: unexpected plan node type: 356

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-21T15:35:03Z

    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

  7. Re: BUG #18859: ERROR: unexpected plan node type: 356

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-21T15:36:14Z

    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