public inbox for [email protected]
help / color / mirror / Atom feedFrom: Robert Treat <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Antonin Houska <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: Mihail Nikalayeu <[email protected]>
Subject: Re: Adding REPACK [concurrently]
Date: Fri, 26 Sep 2025 13:30:09 -0400
Message-ID: <CAJSLCQ303dwCBWoJmp55-f6uXDaRZpozeE+hvBUWau1QqvD2_A@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
On Thu, Sep 25, 2025 at 2:12 PM Álvaro Herrera <[email protected]> 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
view thread (106+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Adding REPACK [concurrently]
In-Reply-To: <CAJSLCQ303dwCBWoJmp55-f6uXDaRZpozeE+hvBUWau1QqvD2_A@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox