Re: BUG #18944: Assertion Failure in psql with idle_session_timeout Set

On Mon, Jun 2, 2025 at 10:13 AM Michael Paquier <michael@paquier.xyz> wrote:
> Reproduced here, thanks for the report.  In this case, we are in a
> PGRES_FATAL_ERROR state due to the timeout triggered by the backend,
> and the loop discarding the results in discardAbortedPipelineResults()
> does not take into account this case scenario: we don't have a result
> to process yet, think that there is one, and kaboom.  It seems to me
> that we should just add an extra case for the result = NULL case as
> there is nothing pending.  A FATAL means that we need to give up
> anyway.
>
> Anthonin, what do you think?

Also reproduced. On a cursory look (I should be able to dig into this
deeper tomorrow), it seems like we're missing a ReadyForQuery response
from the server.

A sync message is normally answered by a ReadyForQuery which is
translated to a PGRES_PIPELINE_SYNC in pqParseInput3. This is why we
add the number of piped_syncs to the requested_results as we need to
consume the generated PGRES_PIPELINE_SYNC with PQgetResult.

With the \sendsync following a COPY, we have two Sync messages but
only receive a single ReadyForQuery response. We have a similar issue
running the following:

\startpipeline
COPY psql_pipeline FROM STDIN;
\syncpipeline
\endpipeline
Enter data to be copied followed by a newline.
End with a backslash and a period on a line by itself, or an EOF signal.
>> COPY 0

The psql session gets stuck in PQgetResult as we're trying to read a
sync response that doesn't exist. Looking at CopyGetData, it seems
like Sync messages are consumed and ignored:

    case PqMsg_Sync:
        /*
         * Ignore Flush/Sync for the convenience of client
         * libraries (such as libpq) that may send those
         * without noticing that the command they just
         * sent was COPY.
         */
        goto readmessage;

So if that's expected, we may need additional logic to avoid
increasing piped_syncs when we're after a COPY? On the other hand,
ignoring the SYNC seems to break the protocol spec expectation that
"there is one and only one ReadyForQuery sent for each Sync".