On Sun, Sep 13, 2026 at 10:35 PM shihao zhong <[email protected]> wrote: > > Hi, > > The check in 0c5d6269614 tests indisready, so it does not catch an index > that is ready but not valid. CREATE INDEX CONCURRENTLY leaves such an > index when it fails during validation, and for a unique index that means > the table contains duplicates. > > > I reproduced it: 6M rows, CREATE UNIQUE INDEX CONCURRENTLY, and a > duplicate inserted while the build was running. The index ends up with > indisready = true, indisvalid = false. On master with 0c5d6269614: > > REPACK u -> succeeds after 17s, index stays invalid > REPACK (CONCURRENTLY) u -> fails after 15s: > ERROR: could not create unique index "u_c_uniq_repacknew" > > So the inconsistency the commit wanted to remove is still there for this > case, and the concurrent form fails only after copying the table. >
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. > The attached patch checks indisvalid instead. Both forms then fail > immediately with the new error. It also adds the ready-but-invalid case > to the test. A validation failure needs a concurrent session, so the > test sets indisvalid = false in pg_index directly. If that is too hacky, > I can drop that part. > This does feel hacky, since we're being manipulative rather than testing a real scenario. I think what we want would be to follow the lead of src/test/modules/injection_points/sql/reindex_conc.sql? Robert Treat https://xzilla.net
