On Wed, 3 Apr 2024 at 21:49, Robert Haas <robertmh...@gmail.com> wrote: > It seems to me that 0001 should either remove the pg_unreachable() > call or change the break to a return, but not both.
Updated the patch to only remove the pg_unreachable call (and keep the breaks). > but there's not much value in omitting > pg_unreachable() from unreachable places, either, so I'm not > convinced. But I don't agree with this. Having pg_unreachable in places where it brings no perf benefit has two main downsides: 1. It's extra lines of code 2. If a programmer/reviewer is not careful about maintaining this unreachable invariant (now or in the future), undefined behavior can be introduced quite easily. Also, I'd expect any optimizing compiler to know that the code after the loop is already unreachable if there are no break/goto statements in its body. > I agree with Tristan's analysis of 0002. Updated 0002, to only change the comment. But honestly I don't think using "default" really introduces many chances for future bugs in this case, since it seems very unlikely we'll ever add new variants to the PostgresPollingStatusType enum.
From b0ccfb6fca3e4ae9e1e62ac3b6a4c5f428b56e04 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <jelte.fenn...@microsoft.com> Date: Wed, 3 Apr 2024 15:19:04 +0200 Subject: [PATCH v14 1/2] Fix actually reachable pg_unreachable call In cafe1056558fe07cdc52b95205588fcd80870362 a call to pg_unreachable was introduced that was actually reachable, because there were break statements in the loop. This addresses that by removing the call to pg_unreachable, which seemed of dubious necessity anyway. --- src/bin/psql/command.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index c005624e9c3..479f9f2be59 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -3815,8 +3815,6 @@ wait_until_connected(PGconn *conn) pg_unreachable(); } } - - pg_unreachable(); } void base-commit: 936e3fa3787a51397280c1081587586e83c20399 -- 2.34.1
From 799eb6989d481ab617c473b8acbbfc1a405c3c7d Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio <jelte.fenn...@microsoft.com> Date: Wed, 3 Apr 2024 15:21:52 +0200 Subject: [PATCH v14 2/2] Change comment on PGRES_POLLING_ACTIVE Updates the comment on PGRES_POLLING_ACTIVE, since we're stuck with it forever due to ABI compatibility guarantees. --- src/interfaces/libpq/libpq-fe.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 8d3c5c6f662..c184e853889 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -91,8 +91,7 @@ typedef enum PGRES_POLLING_READING, /* These two indicate that one may */ PGRES_POLLING_WRITING, /* use select before polling again. */ PGRES_POLLING_OK, - PGRES_POLLING_ACTIVE /* unused; keep for awhile for backwards - * compatibility */ + PGRES_POLLING_ACTIVE /* unused; keep for backwards compatibility */ } PostgresPollingStatusType; typedef enum -- 2.34.1
