From: Antonin Houska <ah@cybertec.at>
To: alvherre@kurilemu.de
cc: Chao Li <li.evan.chao@gmail.com>,
    Kirill Reshke <reshkekirill@gmail.com>,
    PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Fix REPACK with WITHOUT OVERLAPS replica identity indexes
In-reply-to: <agIBukr85PlM6um_@alvherre.pgsql>
References: <agIBukr85PlM6um_@alvherre.pgsql>
Comments: In-reply-to =?us-ascii?Q?=3D=3Futf-8=3FQ=3F=3DC3=3D81lvaro=3F=3D?=
 Herrera <alvherre@kurilemu.de>
   message dated "Mon, 11 May 2026 18:21:17 +0200."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Tue, 12 May 2026 18:48:48 +0200
Message-ID: <70401.1778604528@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/70401.1778604528%40localhost>
Precedence: bulk

=C3=81lvaro Herrera <alvherre@kurilemu.de> wrote:

> On 2026-May-11, Chao Li wrote:
>=20
> > > On May 10, 2026, at 06:38, =C3=81lvaro Herrera <alvherre@kurilemu.de>=
 wrote:
>=20
> > > I think it would be a good idea to make identity_key_equal() not defo=
rm
> > > all attributes, but instead only up to the last one it needs for the =
key
> > > comparisons.
> >=20
> > That=E2=80=99s true. Please see v3.
>=20
> Thanks.  I did one further small change, namely to determine these last
> attnums just once per run rather than once per tuple.  Pushed now.

I appreciate that REPACK can handle more cases now! However, I found a prob=
lem
(or at least a question) when rebasing the improvements for the next
release(s). (It's related to splitting the table scan into multiple block
ranges and use one snapshot per range, details are not too important here, )
Assertion failure in the new code made me think if other than B-tree indexes
should be allowed in the USING INDEX clause of REPACK.

AFAICS, only B-tree indexes (and some special ones that don't appear in the
core) provide ordering information - see get_relation_info():

	/*
	 * Fetch the ordering information for the index, if any.
	 */
	if (info->relam =3D=3D BTREE_AM_OID)
	{
		...
		info->sortopfamily =3D info->opfamily;
		...
	}
	else if (amroutine->amcanorder)
	{
		/*
		 * Otherwise, identify the corresponding btree opfamilies
		 * by trying to map this index's "<" operators into btree.
		 * Since "<" uniquely defines the behavior of a sort
		 * order, this is a sufficient test.
		 ...
	}
	else
	{
		...
		info->sortopfamily =3D NULL;
		...
	}


Therefore, index scan shouldn't be possible for GIST index - see
build_index_paths():

	index_is_ordered =3D (index->sortopfamily !=3D NULL);


So I'm not sure if clustering makes sense here. What makes me confused is t=
hat
GIST has IndexAmRoutine.amclusterable=3Dtrue. As it has amcanorder=3Dfalse =
at the
same time, I suspect it might be just a thinko. However, if we simply set
amclusterable to false, it can break upgrade to PG 19 for users who already
"clustered" some table by a GIST index (for mysterious reasons). (BTW, do we
need the amclusterable field at all?)

REPACK currently rejects explicit sort if non-B-tree index is specified (due
to lack of ordering information), but it still scans the index rather than
the heap - see copy_table_data() and heapam_relation_copy_for_cluster().

Does this seem worth fixing now? Or maybe at least worth some comments (unl=
ess
I'm completely wrong)?

--=20
Antonin Houska
Web: https://www.cybertec-postgresql.com