Feedback-ID: ie3de48e3:Fastmail
Date: Wed, 23 Sep 2026 10:36:10 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Devrim =?utf-8?B?R8O8bmTDvHo=?= <devrim@gunduz.org>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Misleading error message for REPACK USING INDEX on
 shared catalogs
Message-ID: <arONacrml7SnzLtF@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <A5DE1DA7-DD9E-4424-A2E7-6CC459BF090C@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/arONacrml7SnzLtF%40alvherre.pgsql>
Precedence: bulk

On 2026-Sep-23, Chao Li wrote:

> > On Sep 23, 2026, at 06:05, Devrim Gündüz <devrim@gunduz.org> wrote:

> > While poking at REPACK on v19 beta4 for my Chicago meetup talk, I
> > noticed that the error message emitted when trying to REPACK a shared
> > catalog with USING INDEX is misleading, since it makes it sound like
> > REPACK itself is disallowed on shared catalogs, when only the USING
> > INDEX form is:
> > 
> >    postgres=# REPACK pg_authid USING INDEX pg_authid_oid_index;
> >    ERROR:  cannot execute REPACK on a shared catalog
> > 
> >    postgres=# REPACK pg_authid;
> >    REPACK

Good catch.

> > Attached patch tweaks the message to name USING INDEX specifically,
> > and also includes the related comment in repack.c to an errdetail() so
> > it's visible to the user, not just to someone reading the source:
> > 
> >    ERROR:  cannot execute REPACK USING INDEX on a shared catalog
> >    DETAIL:  Marking an index as clustered on a shared catalog would
> >    only take effect in the current database.

Looks reasonable.

> But I have concern with the change. cluster_rel() is used by both
> REPACK and CLUSTER commands, but CLUSTER command, the syntax is
> CLUSTER … USING index-name.

Yeah, I think we should keep the "cannot execute %s on a shared catalog"
string as is, and make the SQL bit interpolated there be REPACK USING
INDEX or CLUSTER according to stmt->cmd; one because CLUSTER also allows
the older CLUSTER ON syntax AFAIR, but also you can't do CLUSTER with no
index.

The added errdetail() bit looks good to me. This is not a new omission:
the older cluster.c code also lacked it.  (The errmsg was "cannot
cluster a shared catalog".)

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/