| From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
|---|---|
| To: | Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com> |
| Cc: | Alexander Lakhin <exclusion(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY |
| Date: | 2026-01-27 15:22:51 |
| Message-ID: | 202601271449.xxtnvk7kbbrt@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-Jan-25, Mihail Nikalayeu wrote:
> Hello, Álvaro!
>
> Fixes are in attachment. I think the comment message and comments are good
> enough to explain the changes.
Actually, about this fragment ... if we track these ancestors for all
indexes, not just the ones that we consider as arbiters, don't we risk
doing something stupid when a completely unrelated index is being
reindexed? Namely, also consider that unrelated index as arbiter.
if (ancestors != NIL &&
!list_member_oid(ancestors_seen, linitial_oid(ancestors)))
{
foreach_oid(parent_idx, rootResultRelInfo->ri_onConflictArbiterIndexes)
{
if (list_member_oid(ancestors, parent_idx))
{
arbiterIndexes = lappend_oid(arbiterIndexes, indexoid);
arbiters_listidxs = lappend_int(arbiters_listidxs, listidx);
break;
}
}
/*
* Track which ancestor we saw, add other indexes that seem to have
* the same ancestor as "unparented".
*/
ancestors_seen = lappend_oid(ancestors_seen, linitial_oid(ancestors));
}
else
unparented_idxs = lappend_int(unparented_idxs, listidx);
I think the lappend_oid(ancestors_seen) should happen inside the
"if list_member_oid(ancestors)" block instead. I'm imagining a test
case like:
CREATE TABLE test.tblparted(i int primary key, updated_at timestamp) PARTITION BY RANGE (i);
CREATE TABLE test.tbl_partition PARTITION OF test.tblparted
FOR VALUES FROM (0) TO (10000)
WITH (parallel_workers = 0);
CREATE INDEX unrelated_index ON test.tblparted (updated_at);
where the reindex is
REINDEX INDEX CONCURRENTLY test.tbl_partition_updated_at_idx;
This doesn't fail, but I suspect it's just from me not setting up the
test case correctly.
I'm also missing a comment for why the use linitial_oid(ancestors) is
correct. At first I thought we should walk the entire ancestors list,
and do this dance if any OID there matches the ancestors_seen list. I
convinced myself that this isn't necessary because the scenario is a
single table being under REINDEX CONCURRENTLY, so the two indexes would
have the same root relation (top-most ancestor index). So comparing
linitial() is correct.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"La persona que no quería pecar / estaba obligada a sentarse
en duras y empinadas sillas / desprovistas, por cierto
de blandos atenuantes" (Patricio Vogel)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Mahendra Singh Thalor | 2026-01-27 15:40:50 | Re: Non-text mode for pg_dumpall |
| Previous Message | Tom Lane | 2026-01-27 15:11:12 | Re: pgsql: Prevent invalidation of newly synced replication slots. |