Re: BUG #19817: CREATE UNIQUE INDEX on a partitioned table can silently attach an invalid, not-ready child index, le

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Kirill Reshke <reshkekirill(at)gmail(dot)com>
Cc: 303677365(at)qq(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #19817: CREATE UNIQUE INDEX on a partitioned table can silently attach an invalid, not-ready child index, le
Date: 2026-11-02 12:46:11
Message-ID: CAFiTN-usrrPD7JV7vsyW7rwtmu5nN_W7SnrsaVc-8WawWSNZ3w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Mon, Nov 2, 2026 at 8:10 AM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
>
> On Sun, 1 Nov 2026 at 11:23, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Fri, Oct 30, 2026 at 12:46 PM PG Bug reporting form
> > <noreply(at)postgresql(dot)org> wrote:
> > >
> > > The following bug has been logged on the website:
> > >
> > > Bug reference: 19817
> > > Logged by: chunling qin
> > > Email address: 303677365(at)qq(dot)com
> > > PostgreSQL version: 18.6
> > > Operating system: Linux (TencentOS 4.4, x86_64)
> > > Description:
> > >
> > > Hello,
> > >
> > > When CREATE UNIQUE INDEX is run on a partitioned table, DefineIndex()
> > > recurses to each partition and reuses an already-existing index it considers
> > > "equivalent" (src/backend/commands/indexcmds.c, the recursion loop around
> > > lines 1450-1495). That equivalence check only compares the index
> > > definition;
> > > it does not check indisvalid or indisready. As a result, an index left
> > > invalid (and not ready) by a previously failed CREATE INDEX CONCURRENTLY is
> > > silently attached as a child index, the parent index is created with
> > > indisvalid=false, and the whole chain reports success -- while in fact the
> > > UNIQUE constraint is not enforced at all.
> > >
> > > Steps to reproduce (self-contained, no data needed):
> > >
> > > -- a partition with a leftover invalid index from a failed CIC
> > > CREATE TABLE b2_root (a int, b text) PARTITION BY RANGE (a);
> > > CREATE TABLE b2_p1 PARTITION OF b2_root FOR VALUES FROM (0) TO (1000);
> > >
> > > INSERT INTO b2_p1 VALUES (1,'x'), (1,'dup'); -- duplicate key
> > > CREATE UNIQUE INDEX CONCURRENTLY b2_p1_bad ON b2_p1 (a); -- fails,
> > > leaves
> > > -- b2_p1_bad behind with indisvalid=f, indisready=f
> > > DELETE FROM b2_p1 WHERE b = 'dup'; -- table is
> > > clean now
> > >
> > > -- the silent failure:
> > > CREATE UNIQUE INDEX b2_root_ui ON b2_root (a); -- returns
> > > -- CREATE INDEX, no warning
> > >
> > > -- state after: the invalid leftover is attached as a child index
> > > SELECT c.relname, i.indisvalid, i.indisready
> > > FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid
> > > WHERE c.relname IN ('b2_p1_bad','b2_root_ui');
> > > -- b2_p1_bad | f | f (parent: b2_root_ui)
> > > -- b2_root_ui | f | t
> > >
> > > -- consequence a: uniqueness is not enforced, duplicates are accepted
> > > INSERT INTO b2_p1 VALUES (1,'x'), (1,'x'); -- succeeds
> > >
> > > -- consequence b: ON CONFLICT can no longer be used on this table
> > > INSERT INTO b2_p1 VALUES (1,'x') ON CONFLICT (a) DO NOTHING;
> > > -- ERROR: there is no unique or exclusion constraint matching
> > > -- the ON CONFLICT specification
> > >
> > > Expected: the recursion loop should skip (or hard-fail on) child indexes
> > > that are not indisvalid/indisready, at minimum emit a WARNING. For
> > > comparison, the ATTACH path already does a full check:
> > > AttachPartitionEnsureIndexes() (src/backend/commands/tablecmds.c) refuses
> > > in that situation, and the project has recently applied the same standard
> > > to REPACK ("Fail REPACK in presence of invalid indexes", commits 0c5d6269614
> > > and 78142821dfd).
> > >
> > > Actual: CREATE UNIQUE INDEX reports success and silently attaches the
> > > invalid leftover index; the unique constraint enforces nothing; no warning
> > > of any kind is emitted.
> > >
> > > Reproduced on:
> > > - PostgreSQL 20devel (snapshot of 2026-10-27, x86_64-pc-linux-gnu)
> > > - PostgreSQL 18.6 (REL_18_STABLE, self-built)
> > >
> > > The behavior looks the same on both branches, so this appears to be
> > > long-standing rather than a recent regression.
> > >
> > > A draft fix location would be the recursion loop in DefineIndex()
> > > (src/backend/commands/indexcmds.c:1450-1495): before reusing an existing
> > > partition index, require indisvalid && indisready, mirroring
> > > AttachPartitionEnsureIndexes().
> >
> > Thanks for the report. I agree that the DefineIndex() should not reuse
> > invalid indexes on partitions when creating an index on a partitioned
> > table, consistent with how AttachPartitionEnsureIndexes() already
> > skips !indisvalid indexes during ALTER TABLE ... ATTACH PARTITION
> > (commit e90e9275f56). Please find attached a patch that skips invalid
> > partition indexes in DefineIndex() and removes the now-unnecessary
> > parent invalidation logic.
> >
> > --
> > Regards,
> > Dilip Kumar
> > Google
>
> Hi!
> I caught a test failure with v1.
>
> # diff -U3 /home/reshke/pgr/postgres/src/test/regress/expected/indexing.out
> /home/reshke/pgr/postgres/src/test/regress/results/indexing.out
> # --- /home/reshke/pgr/postgres/src/test/regress/expected/indexing.out
> 2026-11-02 02:36:57.687946852 +0000
> # +++ /home/reshke/pgr/postgres/src/test/regress/results/indexing.out
> 2026-11-02 02:38:05.543186622 +0000
> # @@ -1684,6 +1684,7 @@
> # -- Now creating the index on the partitioned table succeeds and creates new
> # -- valid indexes, discarding the invalid index created previously.
> # create index parted_isvalid_idx on parted_isvalid_tab ((a/b));
> # +ERROR: division by zero
> # -- Likewise for a failed concurrent unique index build on a partition:
> # insert into parted_isvalid_tab_11 values (1, 1), (1, 1);
> # create unique index concurrently parted_isvalid_uniq_11 on
> parted_isvalid_tab_11 (a);
> # @@ -1704,21 +1705,16 @@
> # pg_inherits inh on (idx.indexrelid = inh.inhrelid)
> # where indexrelid::regclass::text like 'parted_isvalid%'
> # order by indexrelid::regclass::text collate "C";
> # - indexrelid | indisvalid | indrelid
> | inhparent
> # --------------------------------+------------+-----------------------+------------------------------
> # - parted_isvalid_idx | t | parted_isvalid_tab |
> # - parted_isvalid_idx_11 | f | parted_isvalid_tab_11 |
> # - parted_isvalid_tab_11_a_b_idx | t | parted_isvalid_tab_11
> | parted_isvalid_tab_1_a_b_idx
> # - parted_isvalid_tab_11_a_idx | t | parted_isvalid_tab_11
> | parted_isvalid_tab_1_a_idx
> # - parted_isvalid_tab_12_a_b_idx | t | parted_isvalid_tab_12
> | parted_isvalid_tab_1_a_b_idx
> # - parted_isvalid_tab_12_a_idx | t | parted_isvalid_tab_12
> | parted_isvalid_tab_1_a_idx
> # - parted_isvalid_tab_1_a_b_idx | t | parted_isvalid_tab_1
> | parted_isvalid_idx
> # - parted_isvalid_tab_1_a_idx | t | parted_isvalid_tab_1
> | parted_isvalid_uniq
> # - parted_isvalid_tab_2_a_b_idx | t | parted_isvalid_tab_2
> | parted_isvalid_idx
> # - parted_isvalid_tab_2_a_idx | t | parted_isvalid_tab_2
> | parted_isvalid_uniq
> # - parted_isvalid_uniq | t | parted_isvalid_tab |
> # - parted_isvalid_uniq_11 | f | parted_isvalid_tab_11 |
> # -(12 rows)
> # + indexrelid | indisvalid | indrelid |
> inhparent
> # +-----------------------------+------------+-----------------------+----------------------------
> # + parted_isvalid_idx_11 | f | parted_isvalid_tab_11 |
> # + parted_isvalid_tab_11_a_idx | t | parted_isvalid_tab_11 |
> parted_isvalid_tab_1_a_idx
> # + parted_isvalid_tab_12_a_idx | t | parted_isvalid_tab_12 |
> parted_isvalid_tab_1_a_idx
> # + parted_isvalid_tab_1_a_idx | t | parted_isvalid_tab_1 |
> parted_isvalid_uniq
> # + parted_isvalid_tab_2_a_idx | t | parted_isvalid_tab_2 |
> parted_isvalid_uniq
> # + parted_isvalid_uniq | t | parted_isvalid_tab |
> # + parted_isvalid_uniq_11 | f | parted_isvalid_tab_11 |
> # +(7 rows)
> #
> # drop table parted_isvalid_tab;
> # -- Check state of replica indexes when attaching a partition.
> not ok 220 + indexing 605 ms
>
>
> fails about 1 in 3 runs for me.
>
> (ERROR after delete from index).
> The problem is, for parallel make checks, index creation fails because
> of recently-dead tuples. Index build should always include those.
> Example:
>
> -- psql: CREATE TABLE t (a int, b int)
> -- psql: INSERT INTO t VALUES (1, 0)
>
> -- session A: BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT count(*) FROM t;
> -- (old snapshot held open)
>
> -- psql: CREATE INDEX CONCURRENTLY t_expr_idx ON t ((a/b))
> ERROR: division by zero
>
> -- psql: DELETE FROM t
> -- psql: CREATE INDEX t_expr_idx2 ON t ((a/b))
> ERROR: division by zero

Thanks for catching this. In parallel check, a concurrent session
holding an older snapshot causes heapam_index_build_range_scan() to
see the deleted (1, 0) row as HEAPTUPLE_RECENTLY_DEAD, so
FormIndexDatum() still evaluates (a/b) on it, whereas unique-checking
is skipped for RECENTLY_DEAD tuples, So IMHO we can keep unique
checking test only as that will also test what we intend to test here
and remove the divide by zero test from here?

Attached is v2, which uses the failed concurrent unique index build
(duplicate keys) for that test instead of the division-by-zero
expression index so that RECENTLY_DEAD tuples do not cause spurious
failures.

--
Regards,
Dilip Kumar
Google

Attachment Content-Type Size
v2-0001-Ignore-invalid-indexes-on-partitions-during-CREAT.patch application/octet-stream 18.2 KB

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Heikki Linnakangas 2026-11-02 14:07:59 Lost FOR KEY SHARE lock
Previous Message Heikki Linnakangas 2026-11-02 09:36:47 Re: Server crash on UPDATE after savepoint rollback when the row is locked by the same transaction (Assert HEAP_XMAX_IS_LOCKED_ONLY in heap_update)