| From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
|---|---|
| To: | zsolt(dot)parragi(at)percona(dot)com |
| Cc: | alvherre(at)kurilemu(dot)de, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: REPACK (CONCURRENTLY) doesn't handle invalid indexes |
| Date: | 2026-08-26 06:28:41 |
| Message-ID: | 20260826.152841.502876403542656812.horikyota.ntt@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Hello,
At Thu, 13 Aug 2026 22:37:25 +0100, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote in
> > I'll create a patch for that approach, but it is a bit more complex than v1.
>
> It turned out not to be that complex, attached v2.
>
> One thing to point out is that this is a slight behavior change for
> REINDEX TABLE. We could avoid that, but the current behavior doesn't
> seem to be documented anywhere, and this version seems more consistent
> to me.
I think Alvaro's point about whether invalid indexes should be rebuilt
in the first place is worth considering further. In fact, I wonder
whether REPACK should accept a relation containing an invalid index at
all.
I understand that an invalid index with indisready set cannot simply
be ignored by normal DML. As I understand it, indisready represents an
intermediate state in a concurrent index build, allowing INSERT/UPDATE
maintenance to start before the index becomes available for
queries. Therefore, if a concurrent index build fails at that stage,
an index with indisready = true and indisvalid = false can be left
behind.
However, I think there is a distinction between DML having to continue
maintaining such an index according to indisready and a later,
unrelated DDL command rebuilding it as if it were a normal index.
An invalid index left behind by a failed concurrent index build cannot
be used for queries, and is normally either dropped or explicitly
rebuilt with REINDEX. The fact that indisready is true does not mean
that the index is valid for normal use; it can simply mean that
maintenance had already been enabled as part of the concurrent
operation before it failed.
For that reason, I am somewhat uncomfortable with REPACK implicitly
rebuilding such an index. REPACK is not a command for repairing
indexes, so wouldn't it be more natural to reject the operation if an
invalid index exists and require the user to DROP or REINDEX it first,
rather than trying to reproduce or repair that state as part of
REPACK?
I understand that non-concurrent REPACK and VACUUM FULL currently
rebuild invalid indexes. However, that seems to be a consequence of
rebuilding all indexes as part of the heap rewrite, and I am not sure
that this behavior should necessarily define the semantics for REPACK
CONCURRENTLY. In fact, Alvaro's point also makes me wonder about the
existing behavior itself. I wonder whether non-concurrent REPACK or
VACUUM FULL should implicitly rebuild such an index as a side effect
of an operation with a different purpose.
So rather than deciding which invalid indexes to rebuild based on
indisready, as in v2, perhaps we should first decide whether a
relation containing an invalid index should be considered a valid
input for REPACK at all.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jochen Bandhauer | 2026-08-26 07:18:35 | Fwd: Re: repack with verbose output: not showing the number of removable row versions when using USING INDEX or CONCURRENTLY option |
| Previous Message | Andrey Rachitskiy | 2026-08-26 03:08:06 | Re: BUG #19637: pg_event_trigger_ddl_commands can't process GRANT ON PROPERTY GRAPH |