Thread

  1. Re: Logical Replication of sequences

    Nisha Moond <nisha.moond412@gmail.com> — 2025-06-30T09:50:57Z

    On Tue, Jun 24, 2025 at 10:37 AM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > >
    > > Thanks for the comment, the attached v20250622 version patch has the
    > > changes for the same.
    > >
    >
    > Thanks for the patches, I am not done with review yet, but please find
    > the feedback so far:
    >
    
    Thanks for the review.
    
    >
    > 1)
    > + if (!OidIsValid(seq_relid))
    > + ereport(ERROR,
    > + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    > + errmsg("sequence \"%s.%s\" does not exist",
    > +    schema_name, sequence_name));
    >
    > ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE might not be a correct error
    > code here. Shall we have ERRCODE_UNDEFINED_OBJECT? Thoughts?
    >
    
    +1
    Updated to ERRCODE_UNDEFINED_OBJECT code in v20250630
    
    >
    > 2)
    > tab-complete here shows correctly:
    >
    > postgres=# CREATE PUBLICATION pub6 FOR ALL
    > SEQUENCES  TABLES
    >
    > But tab-complete for these 2 commands does not show anything:
    >
    > postgres=# CREATE PUBLICATION pub6 FOR ALL TABLES,
    > postgres=# CREATE PUBLICATION pub6 FOR ALL SEQUENCES,
    >
    > We shall specify SEQUENCES/TABLES in above commands. IIUC, we do not
    > support any other combination like (table <name>, tables in schema
    > <name>) once we get ALL clause in command. So it is safe to display
    > tab-complete as either TABLES or SEQUENECS in above.
    >
    
    Tab-completion is not supported after a comma (,) in any other cases.
    For example, the following commands are valid, but tab-completion does
    not work after the comma:
    
    CREATE PUBLICATION pub7 FOR TABLE t1, TABLES IN SCHEMA public;
    CREATE PUBLICATION pub7 FOR TABLES IN SCHEMA public, TABLES IN SCHEMA schema2;
    
    I feel we can keep the behavior consistent in this case too. Thoughts?
    
    > 3)
    > postgres=#  CREATE publication pub1 for sequences;
    > ERROR:  invalid publication object list
    > LINE 1: CREATE publication pub1 for sequences;
    >                                  ^
    > DETAIL:  One of TABLE, TABLES IN SCHEMA or ALL SEQUENCES must be
    > specified before a standalone table or schema name.
    >
    >
    > This message is not correct as we can not have ALL SEQUENCES *before*
    > a standalone table or schema name. The problem is that gram.y is
    > taking *sequences* as a table or schema name. I noticed that it does
    > same with *tables* as well:
    >
    > postgres=#  CREATE publication pub1 for tables;
    > ERROR:  invalid publication object list
    > LINE 1: CREATE publication pub1 for tables;
    >                                   ^
    > DETAIL:  One of TABLE, TABLES IN SCHEMA or ALL SEQUENCES must be
    > specified before a standalone table or schema name.
    >
    >
    > But since gram.y here can not identify an in-between missing keyword
    > *all*, thus it is considering it (tables/sequenecs) as a literal/name
    > instead of keyword. We can revert back to old message in such a case.
    > I am unable to think of a good solution here.
    >
    
    Done, reverted to the original message.
    
    >
    > 4)
    > I think the error here is wrong as we are not trying to specify
    > multiple all-tables entries.
    >
    > postgres=# CREATE PUBLICATION pub6 for all tables, tables in schema public;
    > ERROR:  invalid publication object list
    > LINE 1: CREATE PUBLICATION pub6 for all tables, tables in schema pub...
    >
    >                                                 ^
    > DETAIL:  ALL TABLES can be specified only once.
    >
    
    The parser cannot distinguish between "TABLES" and "TABLES IN SCHEMA"
    while building all_object_list for "FOR ALL ...".
    To address this, the duplicate check has been moved to
    CreatePublication, and a syntax error is now raised for cases
    mentioned above.
    
    >
    > 5)
    > The log messages still has some scope of improvement:
    >
    > 2025-06-24 08:52:17.988 IST [110359] LOG:  logical replication
    > sequence synchronization worker for subscription "sub1" has started
    > 2025-06-24 08:52:18.029 IST [110359] LOG:  logical replication
    > sequence synchronization for subscription "sub1" - total
    > unsynchronized: 3
    > 2025-06-24 08:52:18.090 IST [110359] LOG:  logical replication
    > sequence synchronization for subscription "sub1" - batch #1 = 3
    > attempted, 0 succeeded, 2 mismatched, 1 missing
    > 2025-06-24 08:52:18.091 IST [110359] ERROR:  logical replication
    > sequence synchronization failed for subscription "sub1"
    > 2025-06-24 08:52:18.091 IST [110359] DETAIL:  Sequences
    > ("public.myseq100") are missing on the publisher. Additionally,
    > parameters differ for the remote and local sequences
    > ("public.myseq101", "public.myseq102").
    > 2025-06-24 08:52:18.091 IST [110359] HINT:  Use ALTER SUBSCRIPTION ...
    > REFRESH PUBLICATION or use ALTER SUBSCRIPTION ... REFRESH PUBLICATION
    > SEQUENCES. Alter or re-create local sequences to have the same
    > parameters as the remote sequences
    >
    >
    > a)
    > Sequences ("public.myseq100") are missing on the publisher.
    > Additionally, parameters differ for the remote and local sequences
    > ("public.myseq101", "public.myseq102").
    >
    > Shall we change this to:
    > missing sequence(s) on publisher: ("public.myseq100"); mismatched
    > sequences(s) on subscriber: ("public.myseq101", "public.myseq102")
    >
    > It will then be similar to the previous log pattern ( 3 attempted, 0
    > succeeded etc) instead of being more verbal. Thoughts?
    >
    >
    > b)
    > Hints are a little odd. First line of hint is just saying to use
    > 'ALTER SUBSCRIPTION' without giving any purpose. While the second line
    > of hint is giving the purpose of alter-sequences.
    >
    > Shall we have?
    > For missing sequences, use ALTER SUBSCRIPTION with either REFRESH
    > PUBLICATION or REFRESH PUBLICATION SEQUENCES
    > For mismatched sequences, alter or re-create local sequences to have
    > matching parameters as publishers.
    >
    > Thoughts?
    >
    
    Updated the messages as suggested.
    
    > 6)
    > postgres=# create publication pub1 for table tab1, all sequences;
    > ERROR:  syntax error at or near "all"
    > LINE 1: create publication pub1 for table tab1, all sequences;
    >
    > We can mention in commit msg that this combination is also not
    > supported or what all combinations are supported. Currently it is not
    > clear f
    
    Done.
    ~~~~
    
    Please find the attached v20250630 patch set addressing above comments
    and other comments in [1],[2],[3] and [4].
    
    [1] https://www.postgresql.org/message-id/CABdArM7h1qQLUb_S7i6MrLPEtHXnX%2BY2fPQaSnqhCdHktcQk5Q%40mail.gmail.com
    [2] https://www.postgresql.org/message-id/CANhcyEVbdambw%3DaVVuW0RrhQ7Lkqad%3DCdrvVA8FP6Xb%2BkP_Qzg%40mail.gmail.com
    [3] https://www.postgresql.org/message-id/CABdArM5mwL8WtGWdDdYT98ddYaB%3D3N6cfPBncvnh682X1GfbVQ%40mail.gmail.com
    [4] https://www.postgresql.org/message-id/CANhcyEWKhHWFzpdAF6czbwq76NRDNCecDqQNtN6Bomn26mqHFw%40mail.gmail.com
    
    --
    Thanks,
    Nisha