Re: Fix error handling in getCopyDataMessage and pqFunctionCall3

From: Anthonin Bonnefoy <anthonin(dot)bonnefoy(at)datadoghq(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Ewan Young <kdbase(dot)hack(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix error handling in getCopyDataMessage and pqFunctionCall3
Date: 2026-07-23 08:19:04
Message-ID: CAO6_Xqr2BPsDUWRxz0LdPaXhJG2HhbaF7vAh0WZp43BvN0D9eA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the reviews!

On Thu, Jul 23, 2026 at 9:43 AM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> + /*
> + * An error may have been triggered while processing the message, bail
> + * out
> + */
> + if (conn->error_result && conn->status == CONNECTION_BAD)
> + return pqPrepareAsyncResult(conn);
>
> getCopyDataMessage() checks for the fatal error before pqParseDone(),
> but pqFunctionCall3() does it after pqParseDone(). Shouldn't
> pqFunctionCall3() use the same ordering?
>
> If getNotify() or getParameterStatus() calls handleFatalError(), the input
> buffer has already been flushed, so calling pqParseDone() afterwards seems
> unnecessary and a bit confusing.

Right, I guess one possible reason would be to trace the message. But
on the other hand, pqParseDone is called for successfully parsed
messages, and other codepaths don't call pqParseDone on errors, so it
is definitely confusing.

I've updated the patch to move the error checks before pqParseDone in
both cases.

Regards,
Anthonin Bonnefoy

Attachment Content-Type Size
v2-0001-Fix-error-handling-in-getCopyDataMessage-and-pqFu.patch application/octet-stream 1.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-07-23 08:23:27 Re: Fix missing FORMAT when deparsing JSON_ARRAY(query)
Previous Message Michael Paquier 2026-07-23 08:06:25 Re: Don't use pq_putmessage in socket comm function