Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow REPLICA IDENTITY to be set on an index that's not (yet) valid.

  1. BUG #17756: Invalid replica indentity set order in a dump

    PG Bug reporting form <noreply@postgresql.org> — 2023-01-20T13:41:07Z

    The following bug has been logged on the website:

Bug reference:      17756
Logged by:          Sergey Belyashov
Email address:      sergey.belyashov@gmail.com
PostgreSQL version: 14.5
Operating system:   Debian Linux x86_64
Description:        

Some database have a partitioned table with unique index used as REPLICA
IDENTITY. pg_dump places ALTER TABLE tbl REPLICA IDENTITY USING INDEX
some_idx after partial index creation. But Postgresql fails to restore such
dump because replica identity cannot be set on invalid index some_idx:
partition indices are not created.

  2. Re: BUG #17756: Invalid replica indentity set order in a dump

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-20T15:42:10Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> Some database have a partitioned table with unique index used as REPLICA
> IDENTITY. pg_dump places ALTER TABLE tbl REPLICA IDENTITY USING INDEX
> some_idx after partial index creation. But Postgresql fails to restore such
> dump because replica identity cannot be set on invalid index some_idx:
> partition indices are not created.

Please provide a concrete example, preferably a SQL script to create
a database that triggers the problem.  There are enough variables
here that nobody is likely to be excited about trying to reverse-
engineer a test case from only this amount of detail.

			regards, tom lane

  3. Re: BUG #17756: Invalid replica indentity set order in a dump

    Sergey Belyashov <sergey.belyashov@gmail.com> — 2023-01-20T17:45:57Z

    SQL:
create database testdb;
\c testdb
create table tbl (id integer not null primary key) partition by list (id);
create table tbl_1 partition of tbl for values in (1);
alter table tbl replica identity using index tbl_pkey;

Next do:
$ pg_dump testdb >testdb.sql
$ psql testdb -c "drop table tbl"
$ psql testdb <testdb.sql

result:
...
ALTER TABLE
ERROR:  cannot use invalid index "tbl_pkey" as replica identity
...

Reproduced in Postgresql 15.1 too.

Sergey Belyashov

пт, 20 янв. 2023 г. в 18:42, Tom Lane <tgl@sss.pgh.pa.us>:
>
> PG Bug reporting form <noreply@postgresql.org> writes:
> > Some database have a partitioned table with unique index used as REPLICA
> > IDENTITY. pg_dump places ALTER TABLE tbl REPLICA IDENTITY USING INDEX
> > some_idx after partial index creation. But Postgresql fails to restore such
> > dump because replica identity cannot be set on invalid index some_idx:
> > partition indices are not created.
>
> Please provide a concrete example, preferably a SQL script to create
> a database that triggers the problem.  There are enough variables
> here that nobody is likely to be excited about trying to reverse-
> engineer a test case from only this amount of detail.
>
>                         regards, tom lane

  4. Re: BUG #17756: Invalid replica indentity set order in a dump

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-20T18:14:01Z

    Sergey Belyashov <sergey.belyashov@gmail.com> writes:
> SQL:
> create database testdb;
> \c testdb
> create table tbl (id integer not null primary key) partition by list (id);
> create table tbl_1 partition of tbl for values in (1);
> alter table tbl replica identity using index tbl_pkey;

> Next do:
> $ pg_dump testdb >testdb.sql
> $ psql testdb -c "drop table tbl"
> $ psql testdb <testdb.sql

> result:
> ...
> ALTER TABLE
> ERROR:  cannot use invalid index "tbl_pkey" as replica identity

Thanks for the test case.  So the problem occurs because pg_dump dumps
the commands in this order:

    ALTER TABLE ONLY public.tbl
        ADD CONSTRAINT tbl_pkey PRIMARY KEY (id);
    
    ALTER TABLE ONLY public.tbl REPLICA IDENTITY USING INDEX tbl_pkey;
    
    ALTER TABLE ONLY public.tbl_1
        ADD CONSTRAINT tbl_1_pkey PRIMARY KEY (id);
    
    ALTER INDEX public.tbl_pkey ATTACH PARTITION public.tbl_1_pkey;

but the backend won't take the ALTER REPLICA IDENTITY command
until after the ATTACH PARTITION.

We could probably make pg_dump emit things in the order that works,
but it'd be a significant amount of extra complication there
(the ALTER REPLICA IDENTITY command couldn't be treated as just
part of the index definition).

I wonder why it is that the backend rejects this sequence.
I see that you can do this:

regression=# create table tbl (id integer not null primary key) partition by list (id);
CREATE TABLE
regression=# alter table tbl replica identity using index tbl_pkey;
ALTER TABLE

and it doesn't seem like the partitioned index is notably more
valid in this state than in the one that pg_dump has created.
So I think it might be better to fix the backend to allow this
sequence of operations.

			regards, tom lane

  5. Re: BUG #17756: Invalid replica indentity set order in a dump

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-20T19:54:25Z

    I wrote:
> I wonder why it is that the backend rejects this sequence.
> I see that you can do this:
> regression=# create table tbl (id integer not null primary key) partition by list (id);
> CREATE TABLE
> regression=# alter table tbl replica identity using index tbl_pkey;
> ALTER TABLE
> and it doesn't seem like the partitioned index is notably more
> valid in this state than in the one that pg_dump has created.
> So I think it might be better to fix the backend to allow this
> sequence of operations.

I looked into this, and I think we basically could just drop the
restriction in ATExecReplicaIdentity that rejects marking an invalid
index as replica identity.  If it is invalid, then the relcache won't
treat it as being the live replica identity (cf RelationGetIndexList),
so nothing will happen until it becomes valid; but there's no reason
why we can't mark it as identity in advance of that.  We support
not-yet-valid primary keys, so why not replica identity?

In checking this, I found a comment in index.c claiming

             * ALTER TABLE assumes that indisreplident cannot be set for
             * invalid indexes.

which was added by Michael's 9511fb37a.  The back story for that
indicates that Michael was worried about multiple indexes of a
relation becoming marked with indisreplident.  However, as far
as I can see the only actual issue there is that
relation_mark_replica_identity is too trusting, and that looks
like either sloppy coding or premature optimization.  I think
we could just rip out its early-exit path and fix it to positively
guarantee that no more than one index is marked indisreplident.

In short, I propose the attached.  This'd need to be back-patched,
which 9511fb37a wasn't, so maybe we should also back-patch 9511fb37a?
I don't think it's really necessary though.

			regards, tom lane