Schema Diff: converting a column between an integer type and SERIAL produces an incomplete script

[dpage]

Bug Description

Schema Diff can tell that a column is a SERIAL/BIGSERIAL/SMALLSERIAL on one side and a plain integer column on the other, but the script it generates does not finish the conversion, so applying it leaves the target column functionally different from the source.

With a source table declared as:

CREATE TABLE public.t (id bigserial NOT NULL, val text);

and a target declared as:

CREATE TABLE public.t (id integer NOT NULL, val text);

the comparison produces the column type change and, from the sequence node, the sequence itself:

ALTER TABLE public.t
    ALTER COLUMN id TYPE bigint;

CREATE SEQUENCE IF NOT EXISTS public.t_id_seq
    INCREMENT 1 START 1 MINVALUE 1 MAXVALUE 9223372036854775807 CACHE 1;

ALTER SEQUENCE public.t_id_seq
    OWNED BY public.t.id;

but never the ALTER COLUMN id SET DEFAULT nextval('public.t_id_seq'::regclass) that makes the column a serial. The sequence is created and owned by the column, yet the column has no default, so inserts that omit id still fail on the target whilst succeeding on the source.

The cause is that Schema Diff compares columns after they have been reprojected onto the SERIAL pseudo-type (get_formatted_columns() with with_serial=True), and the reprojection empties the column's nextval() default because the default is implied by the pseudo-type. Once the two sides genuinely differ in serialness, that emptied default is all the SQL generator has to work from, so the default is simply never written.

The reverse direction (source plain integer, target bigserial) has an ordering problem rather than a missing statement: the column's DROP DEFAULT and the sequence's DROP SEQUENCE both get generated, but a sequence owned by a column whose default still references it cannot be dropped, so the order in which the script emits them matters.

Expected Behaviour

Applying a Schema Diff script for a column that changes between an integer type and a serial should leave the target column equivalent to the source, defaults included.

Context

Noticed whilst fixing #10236 (commit 01e5bf0), which stopped identical serial columns being reported as different and stopped the invalid ALTER COLUMN ... TYPE bigserial being generated for serial columns with genuine differences. That change deliberately left this conversion case alone: it is now valid SQL rather than invalid SQL, but it is still incomplete.

[dpage]

I looked at fixing this and it needs a design decision rather than a patch, so here is what I found.

The missing statement is ALTER COLUMN id SET DEFAULT nextval('<seq>'::regclass), and the problem is that by the time the SQL is generated, nothing in the diff data knows which sequence to name. get_formatted_columns() empties the column's default as part of reprojecting it onto the pseudo-type (the default is implied by bigserial), seqrelid is dropped as one of column_keys_to_ignore, and compare_target_cols() deletes ignored keys from the dictionary outright rather than merely skipping them when comparing, so the sequence's identity is gone by the time _check_for_column_update() runs.

Three ways out, none of them free:

1. Carry the sequence name in the diff data. Add the owned sequence's name to columns/sql/*/properties.sql and keep it through the reprojection. The catch is that anything carried is also compared, and a schema-qualified nextval(...) string differs between two databases whenever the schemas being compared have different names, which reintroduces exactly the false positive Schema Diff: False positive and invalid SQL generated for BIGSERIAL / SERIAL columns #10236 was about. Keeping it out of the comparison means teaching compare_target_cols() to compare copies with the ignored keys removed instead of deleting them from the originals. That is the change I would favour, since a diff dictionary that has had data deleted from it is awkward for any generator, but it changes the shape of what every column-level template receives and wants its own review.

2. Resolve the sequence at apply time with nextval(pg_get_serial_sequence('schema.table', 'column')). No name guessing, and the sequence is created with OWNED BY by the same script. It does not work as written, though: the default expression is stored as given, so the column's default becomes the pg_get_serial_sequence(...) call rather than nextval('seq'::regclass), and the next comparison no longer recognises the column as a serial, so the diff never settles. Wrapping it in a DO block to resolve the name and EXECUTE the ALTER would store the right thing, but a DO block is unlike anything else Schema Diff emits.

3. Guess the sequence name from the table and column, which is what Issue with CREATE script: Schema Diff generates wrong SQL for SERIAL columns after renaming, or when a sequence name accidentally matches #10100 and If sequence with same name exists new generated sequence in CREATE script overwrites the script silently #10101 were about, so no.

There is an ordering aspect too: whichever way the default is emitted, it has to run after the CREATE SEQUENCE ... OWNED BY that the sequence node contributes, and the script is not ordered by dependency at the moment (#10295).

The reverse direction (source a plain integer, target a serial) is not a missing statement but an ordering one: the column's DROP DEFAULT and the sequence's DROP SEQUENCE are both generated, and a sequence owned by a column whose default still references it cannot be dropped, so it only works if they come out in that order.

I have left this alone deliberately: since 01e5bf0 it generates valid but incomplete SQL rather than invalid SQL, which seems a reasonable place to pause until someone takes a view on option 1.