| From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
|---|---|
| To: | Robert Treat <rob(at)xzilla(dot)net> |
| Cc: | shihao zhong <zhong950419(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, Christophe Pettus <xof(at)thebuild(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, rmt(at)lists(dot)postgresql(dot)org |
| Subject: | Re: REPACK (CONCURRENTLY) doesn't handle invalid indexes |
| Date: | 2026-09-15 15:30:54 |
| Message-ID: | aqkXyUZFk__dKqvm@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs pgsql-hackers |
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.
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.
> 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 ...
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
“Cuando no hay humildad las personas se degradan” (A. Christie)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Treat | 2026-09-15 18:30:34 | Re: REPACK (CONCURRENTLY) doesn't handle invalid indexes |
| Previous Message | Alexander Korotkov | 2026-09-15 13:05:41 | Re: BUG #19633: Unexpected results of IN (subquery) with a non-deterministic collation |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | 신성준 | 2026-09-15 15:35:47 | Re: Add wait events for server logging destination writes |
| Previous Message | Greg Burd | 2026-09-15 15:26:11 | Re: Support for 8-byte TOAST values, round two |