MIME-Version: 1.0
References: <202510301734.pj4uds3mqxx4@alvherre.pgsql>
 <116433.1764870207@localhost>
In-Reply-To: <116433.1764870207@localhost>
From: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Date: Fri, 5 Dec 2025 01:03:00 +0100
Message-ID: 
 <CADzfLwWFbXVN-QrKaVXvW96eYQD1AiRVCYd99nX7EQFG3q_yfg@mail.gmail.com>
Subject: Re: Adding REPACK [concurrently]
To: Antonin Houska <ah@cybertec.at>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Pg Hackers <pgsql-hackers@lists.postgresql.org>,
 Robert Treat <rob@xzilla.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CADzfLwWFbXVN-QrKaVXvW96eYQD1AiRVCYd99nX7EQFG3q_yfg%40mail.gmail.com>
Precedence: bulk

Hello, Antonin!

On Thu, Dec 4, 2025 at 6:43=E2=80=AFPM Antonin Houska <ah@cybertec.at> wrot=
e:
> v26 attached here. It's been rebased and reflects most of the feedback.

Some comments on 0001-0002:
1)

> cluster_rel(stmt->command, rel, indexOid, params);
cluster_rel closes relation, and after it is dereferenced a few lines after=
.
Technically it may be correct, but feels a little bit strange.

2)

> if (vacopts->mode =3D=3D MODE_VACUUM)
I think for better compatibility it is better to handle new value in
if - (vacopts->mode =3D=3D MODE_REPACK) to keep old cases unchanged

3)

> case T_RepackStmt:
>    tag =3D CMDTAG_REPACK;
>    break;

should we use instead:

case T_RepackStmt:
    if (((RepackStmt *) parsetree)->command =3D=3D REPACK_COMMAND_CLUSTER)
       tag =3D CMDTAG_CLUSTER;
    else
       tag =3D CMDTAG_REPACK;
    break;

or delete CMDTAG_CLUSTER - since it not used anymore

4)
"has been superceded by"
typo

Best regards,
Mikhail.