Re: REPACK (CONCURRENTLY) doesn't handle invalid indexes

On Tue, Sep 15, 2026 at 11:30 AM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
> On 2026-Sep-14, Robert Treat wrote:
>
> > The patch looks right to me, and I think I am +1 for this generally,
> > though I would note that we are widening the scope here such that
> > indexes that could succeed with a rebuild would now cause an error, so
> > we're a little less functional though a little more behaviorally
> > consistent.
>
> Yeah. I considered the idea of adding a REPACK option like
> "rebuild_invalid_indexes=on" that would attempt to rebuild rather than
> failing outright, at the user's risk. Not sure it's worth the trouble.
>

Yeah, I started thinking if there were some way to handle this better,
but it quickly gets in to late stage design work that is probably
better left to v20 and some user feedback.

> I have pushed this change now, without the test. I did throw in a short
> doc update.
>
> One thing I didn't want to say in said doc update, is that while you can
> do a REINDEX of the non-validated index prior to REPACK, it's rather a
> complete waste of time, because REPACK has to rebuild that index again.
> It's probably better to DROP the index, then REPACK, then do CREATE
> INDEX CONCURRENTLY. If we didn't cause REPACK to error out in the
> presence of a buildable invalid index, then REPACK could rebuild the
> index just fine. I'm happy to listen to your operationally-experienced
> opinion on this.
>

I'd agree with you that in most cases, the double rebuild isn't going
to be worth it. I mean maybe you want to fix a broken unique index so
that it is enforced before (and thus during) a long REPACKCC; but that
feels pretty contrived since we are talking about leftover broken
indexes anyway.

> > This does feel hacky, since we're being manipulative rather than
> > testing a real scenario.
>
> Yeah.
>
> > I think what we want would be to follow the lead of
> > src/test/modules/injection_points/sql/reindex_conc.sql?
>
> Do you mean creating test invalid indexes by way of using injection
> points to interrupt creation in the various phases? I don't see
> reindex_conc.sql doing that. But also, we run these tests in
> test_decoding because it's the test suite that is certain to have
> wal_level=logical; but if we wanted to also require injection_points,
> the changes in meson.build / Makefile get more involved. I didn't find
> a test suite that has conditional tests depending on injection_points.
> src/test/modules/authentication does, but for a TAP test, not a plain
> regress or isolation test. It is surely just a SMOP ...
>

Ah, yeah, I'm less sure its worth it given the above, but I was
thinking something like this:

CREATE EXTENSION injection_points;
SELECT injection_points_set_local();
SELECT injection_points_attach('define-index-before-set-valid', 'error');

CREATE TABLE x (a int PRIMARY KEY, b int);
INSERT INTO x VALUES (1,1),(2,2);
CREATE INDEX CONCURRENTLY x_b ON x (b);

SELECT injection_points_detach('define-index-before-set-valid');

Robert Treat
https://xzilla.net