On 03.04.22 20:50, David G. Johnston wrote: > However, tables having an identity sequence seem to be unaddressed in > this patch. The existing (and unchanged) pg_dump.c code results in:
It is addressed. For example, run this in PG14:
ALTER TABLE public.t1 ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY ( SEQUENCE NAME public.t1_a_seq START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE CACHE 1 ); ALTER SEQUENCE public.t1_a_seq SET LOGGED;
OK, I do see the new code for this and see how my prior email was confusing/wrong. I do still have the v14 dump file restoration concern but that actually isn't something pg_dump.c has to (or even can) worry about. Ensuring that a v15+ dump represents the existing state correctly is basically a given which is why I wasn't seeing how my comments would be interpreted relative to that.
For the patch I'm still thinking we want to add [UN]LOGGED to sequence_options. Even if pg_dump doesn't utilize it, though aside from potential code cleanliness I don't see why it wouldn't. If absent, the default behavior shown here (sequence matches table, as per "+ seqstmt->sequence->relpersistence = cxt->relation->relpersistence;" would take effect) applies, otherwise the newly created sequence is as requested.
From this, in the current patch, a pg_dump v14- produced dump file restoration will change the persistence of owned sequences on an unlogged table to unlogged from logged during restoration into v15+ (since the alter sequence will not be present after the alter table). A v15+ pg_dump produced dump file will retain the logged persistence mode for the sequence. The only way to avoid this discrepancy is to have sequence_options taken on a [UN]LOGGED option that defaults to LOGGED. This then correctly reflects historical behavior and will produce a consistently restored dump file.
