Content-Type: multipart/alternative;
 boundary="------------ptXOi7TeubMyfKchTvH0Xu3P"
Message-ID: <3f3dc050-aab1-4908-a6b8-bdadafd451cc@gmail.com>
Date: Mon, 1 Jun 2026 08:00:01 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: BUG #19494: Error on transaction commit inside pipeline triggers
 psql's Assert
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs@lists.postgresql.org
References: <19494-97a86d84fee71c47@postgresql.org>
 <ahe7ylLcESTmnZfk@paquier.xyz> <ahfR7s1ZXC6MSC0i@paquier.xyz>
 <f7cfb56d-c7d6-40d9-b08d-ee5232f95cba@gmail.com>
 <ahkdZJMZvyYlsfXK@paquier.xyz>
 <cce2ba8c-307e-4244-978e-91174073a4dc@gmail.com>
 <ahzqaOwc2DprQRrV@paquier.xyz>
Content-Language: en-US
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <ahzqaOwc2DprQRrV@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/3f3dc050-aab1-4908-a6b8-bdadafd451cc%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------ptXOi7TeubMyfKchTvH0Xu3P
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hello Michael,

01.06.2026 05:11, Michael Paquier пишет:
> I have looked at that as well, and I don't think that this is fixable
> only from the point of psql, because the error injected creates a
> state where libpq's internal command queue gets out of sync regarding
> what the backend has sent.  The only thing that could be done is
> inside libpq, as far as I can see, where we should try to detect that
> the state is not synchronized anymore and fail rather than block.  So
> IMO, and with the error injected (which would never happen in
> production in practice), the best thing I can come up with is the
> attached for now.
>
> One thing that I could see ourselves do as an extra improvement in
> ExecQueryAndProcessResults() where we consume the results and check if
> we're still in a busy state (some PQconsumeInput+PQisBusy).  I don't
> think that this should be a problem in practice, but this feels like
> just hiding the real problem on the libpq side with the inconsistent
> protocol state generated by the backend.  I have also quickly tested
> an approach based on that, unfortunately this leads to some
> instability in the tests to due the async nature of the commands.
>
> Anyway, the v3 attached passes the regression tests, handles the
> pg_terminate_backend() case gracefully, handles the error case with
> the error injected on backend-side a but better, and can avoid
> some of the issues in the fourth case, but not all as we don't have
> access to the pipe state when reaching the results do to the backend
> missing up with the libpq state.  Handling the 4th case more
> gracefully would require some libpq changes, which may not justify the
> cases we are dealing with here, at least to me.  As a whole, I'd feel
> that v3 is a good improvement in itself, and it addresses your
> original issues and the assertions.
>
> What do you think?

I agree with your points, the v3 looks good to me. Thank you for paying
attention to all of these issues!

Best regards,
Alexander
--------------ptXOi7TeubMyfKchTvH0Xu3P
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">Hello Michael,<br>
      <br>
      01.06.2026 05:11, Michael Paquier пишет:<br>
    </div>
    <blockquote type="cite" cite="mid:ahzqaOwc2DprQRrV@paquier.xyz">
      <pre wrap="" class="moz-quote-pre">I have looked at that as well, and I don't think that this is fixable 
only from the point of psql, because the error injected creates a
state where libpq's internal command queue gets out of sync regarding
what the backend has sent.  The only thing that could be done is
inside libpq, as far as I can see, where we should try to detect that
the state is not synchronized anymore and fail rather than block.  So
IMO, and with the error injected (which would never happen in
production in practice), the best thing I can come up with is the
attached for now.

One thing that I could see ourselves do as an extra improvement in
ExecQueryAndProcessResults() where we consume the results and check if
we're still in a busy state (some PQconsumeInput+PQisBusy).  I don't
think that this should be a problem in practice, but this feels like
just hiding the real problem on the libpq side with the inconsistent
protocol state generated by the backend.  I have also quickly tested
an approach based on that, unfortunately this leads to some
instability in the tests to due the async nature of the commands.

Anyway, the v3 attached passes the regression tests, handles the
pg_terminate_backend() case gracefully, handles the error case with
the error injected on backend-side a but better, and can avoid 
some of the issues in the fourth case, but not all as we don't have
access to the pipe state when reaching the results do to the backend
missing up with the libpq state.  Handling the 4th case more
gracefully would require some libpq changes, which may not justify the 
cases we are dealing with here, at least to me.  As a whole, I'd feel
that v3 is a good improvement in itself, and it addresses your
original issues and the assertions.

What do you think?
</pre>
    </blockquote>
    <br>
    I agree with your points, the v3 looks good to me. Thank you for
    paying<br>
    attention to all of these issues!<br>
    <br>
    Best regards,<br>
    Alexander<br>
  </body>
</html>

--------------ptXOi7TeubMyfKchTvH0Xu3P--