Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY - Mailing list pgsql-hackers

From Álvaro Herrera
Subject Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
Date
Msg-id 202601271449.xxtnvk7kbbrt@alvherre.pgsql
Whole thread Raw
In response to Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY  (Mihail Nikalayeu <mihailnikalayeu@gmail.com>)
Responses Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
List 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)



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgsql: Prevent invalidation of newly synced replication slots.
Next
From: Mahendra Singh Thalor
Date:
Subject: Re: Non-text mode for pg_dumpall