public inbox for [email protected]  

help / color / mirror / Atom feed

From: Álvaro Herrera <[email protected]>
To: Mihail Nikalayeu <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
Date: Tue, 27 Jan 2026 16:22:51 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CADzfLwWZjWqeX6fF5=iKq_PJiw7G+k01CBu5xB8X_Z+nN1gqqA@mail.gmail.com>

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)

view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox