Feedback-ID: ie3de48e3:Fastmail
Date: Tue, 27 Jan 2026 16:22:51 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, 
	Michael Paquier <michael@paquier.xyz>, Noah Misch <noah@leadboat.com>, 
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
Message-ID: <202601271449.xxtnvk7kbbrt@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CADzfLwWZjWqeX6fF5=iKq_PJiw7G+k01CBu5xB8X_Z+nN1gqqA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/202601271449.xxtnvk7kbbrt%40alvherre.pgsql>
Precedence: bulk

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)