Horton

Re: Assert failure on running a completed portal again

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Yugo Nagata <nagata@sraoss.co.jp>
Cc: pgsql-hackers@postgresql.org
Date: 2024-12-05T22:09:14Z
Lists: pgsql-hackers 
Yugo Nagata <nagata@sraoss.co.jp> writes:
> I notice that the following Assert in PortalRun fails when a same portal is
> executed more than once by an Execute message whose "max number of rows"
> is specified to zero, that is, "no limit".  

>         /* Set run_once flag.  Shouldn't be clear if previously set. */
>         Assert(!portal->run_once || run_once);
>         portal->run_once = run_once;

Hmm.  It's fairly hard to tell what was meant there, because
(a) there is exactly zero documentation of what run_once means
(b) that comment is opaque and doesn't appear to describe the
code behavior anyway.

> I believe the server should return CommanComplete normally in this case. 
> his can be fixed by not setting execute_is_fetch flag (run_once as the result)
> when the portal is already completed since no rows will be fetched in this case.

Don't like this much.  I'm not sure I believe any part of this
approach to deciding whether the portal is "run once".  In any
case, a proper fix for this needs to include actually documenting
what that field means and does.

			regards, tom lane

Commits

  1. Doc: add some commentary about ExecutorRun's NoMovement special case.

  2. Simplify executor's determination of whether to use parallelism.

  3. Allow for parallel execution whenever ExecutorRun() is done only once.