Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: [PATCH] Improve REPACK (CONCURRENTLY) error messages for
 unsupported configurations
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CA+fm-ROdgh0rEVuXoViBk4TVgjodrN=MTR_RYuOuKLZ9voX4YA@mail.gmail.com>
Date: Wed, 27 May 2026 14:50:01 +0800
Cc: pgsql-hackers@lists.postgresql.org,
 =?utf-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
Content-Transfer-Encoding: quoted-printable
Message-Id: <172EB2C2-DE11-4E5B-B115-38A7AD3B6A3D@gmail.com>
References: 
 <CA+fm-ROdgh0rEVuXoViBk4TVgjodrN=MTR_RYuOuKLZ9voX4YA@mail.gmail.com>
To: Baji Shaik <baji.pgdev@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/172EB2C2-DE11-4E5B-B115-38A7AD3B6A3D%40gmail.com>
Precedence: bulk


> On May 27, 2026, at 11:06, Baji Shaik <baji.pgdev@gmail.com> wrote:
>=20
> Hi,
>=20
> While exploring the new REPACK (CONCURRENTLY) feature, I noticed=20
> a few user-facing error paths that could be made more accurate. =20
> Patch series of three:
>=20
>  0001 -- When wal_level < replica, REPACK (CONCURRENTLY) currently
>          surfaces generic "replication slots ... wal_level" error
>          from CheckSlotRequirements(), with a CONTEXT line referring
>          to an internal worker.  Add an upfront check that reports a
>          REPACK-specific error.
>=20

LGTM

>  0002 -- check_concurrent_repack_requirements() reports the same
>          generic "no identity index" error for several distinct
>          cases, two of which are misleading: REPLICA IDENTITY FULL
>          (which is set, but the hint says there is no identity), and
>          a deferrable PK as the only identity (skipped per commit
>          832e220d99a, but the hint suggests adding an index that
>          already exists).  Distinguish these cases.
>=20

When I was working on 832e220d99a, I actually considered for more =
detailed error messages, but I ended up giving up. I think we should be =
careful about adding more branches here unless the existing message is =
causing significant confusion in practice.

So, I personally don=E2=80=99t like 0002.

>  0003 -- Four ereport(ERROR) calls in the REPACK CONCURRENTLY code
>          path lack errcode() and default to ERRCODE_INTERNAL_ERROR.
>          Add appropriate errcodes; in particular, the
>          apply_concurrent_update/delete failures map cleanly to
>          ERRCODE_T_R_SERIALIZATION_FAILURE.
>=20

LGTM

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/