| 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 |
| 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 |