On Thu, Sep 25, 2025 at 2:12 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> So here's v22 with those and rebased to current sources. Only the first
> two patches this time, which are the ones I would be glad to receive
> input on.
>
A number of small issues I noticed. I don't know that they all need
addressing right now, but seems worth asking the questions...
#1
"pg_repackdb --help" does not mention the --index option, although the
flag is accepted. I'm not sure if this is meant to match clusterdb,
but since we need the index option to invoke the clustering behavior,
I think it needs to be there.
#2
[xzilla@zebes] pgsql/bin/pg_repackdb -d pagila -v -t customer
--index=idx_last_name
pg_repackdb: repacking database "pagila"
INFO: clustering "public.customer" using sequential scan and sort
[xzilla@zebes] pgsql/bin/pg_repackdb -d pagila -v -t customer
pg_repackdb: repacking database "pagila"
INFO: vacuuming "public.customer"
This was less confusing once I figured out we could pass the --index
option, but even with that it is a little confusing, I think mostly
because it looks like we are "vacuuming" the table, which in a world
of repack and vacuum (ie. no vacuum full) doesn't make sense. I think
the right thing to do here would be to modify it to be "repacking %s"
in both cases, with the "using sequential scan and sort" as the means
to understand which version of repack is being executed.
#3
pg_repackdb does not offer an --analyze option, which istm it should
to match the REPACK command
#4
SQL level REPACK help shows:
where option can be one of:
VERBOSE [ boolean ]
ANALYSE | ANALYZE
but SQL level VACUUM does
VERBOSE [ boolean ]
ANALYZE [ boolean ]
These operate the same way, so I would expect it to match the language
in vacuum.
#5
[xzilla@zebes] pgsql/bin/pg_repackdb -d pagila -v -t film --index
pg_repackdb: repacking database "pagila"
In the above scenario, I am repacking without having previously
specified an index. At the SQL level this would throw an error, at the
command line it gives me a heart attack. :-)
It's actually not that bad, because we don't actually do anything, but
maybe we should throw an error?
#6
On the individual command pages (like sql-repack.html), I think there
should be more cross-linking, ie. repack should probably say "see also
cluster" and vice versa. Likely similarly with vacuum and repack.
#7
Is there some reason you chose to intermingle the repack regression
tests with the existing tests? I feel like it'd be easier to
differentiate potential regressions and new functionality if these
were separated.
Robert Treat
https://xzilla.net
