> On May 27, 2026, at 11:06, Baji Shaik <[email protected]> wrote:
>
> Hi,
>
> While exploring the new REPACK (CONCURRENTLY) feature, I noticed
> a few user-facing error paths that could be made more accurate.
> Patch series of three:
>
> 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.
>
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.
>
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’t 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.
>
LGTM
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
