Mihail Nikalayeu <[email protected]> wrote: > On Thu, Dec 4, 2025 at 6:43 PM Antonin Houska <[email protected]> wrote: > > 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.
ok, will be fixed in the next version (supposedly later today). > 2) > > > if (vacopts->mode == MODE_VACUUM) > I think for better compatibility it is better to handle new value in > if - (vacopts->mode == MODE_REPACK) to keep old cases unchanged I suppose you mean vacuuming.c. We're considering removal of pg_repackdb from the patchset, so let's decide on this later. > 3) > > > case T_RepackStmt: > > tag = CMDTAG_REPACK; > > break; > > should we use instead: > > case T_RepackStmt: > if (((RepackStmt *) parsetree)->command == REPACK_COMMAND_CLUSTER) > tag = CMDTAG_CLUSTER; > else > tag = CMDTAG_REPACK; > break; > > or delete CMDTAG_CLUSTER - since it not used anymore LGTM, will include it in the next version. > 4) > "has been superceded by" > typo ok. (This may also be removed, as it's specific to pg_repackdb.) -- Antonin Houska Web: https://www.cybertec-postgresql.com
