Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequence migration: nextval('new_seq') = nextval('old_seq') #10

Open
runeksvendsen opened this issue Jan 2, 2021 · 3 comments
Open

Comments

@runeksvendsen
Copy link
Member

I have a table that looks as follows:

data RunT f
    = Run
    { runId         :: C f (SqlSerial Word32)
    , runTimeStart  :: C f UTCTime
    , runTimeEnd    :: C f UTCTime
    }

The current schema looks like this:

, ( TableName { tableName = "runs" }
    , Table
        { tableConstraints =
            fromList
            [ PrimaryKey
                "runs_pkey" (fromList [ ColumnName { columnName = "id" } ])
            ]
        , tableColumns =
            fromList
            [ ( ColumnName { columnName = "id" }
                , Column
                    { columnType = SqlStdType DataTypeInteger
                    , columnConstraints =
                        fromList [ NotNull , Default "nextval('runs_id_seq'::regclass)" ]
                    }
                )
            , ( ColumnName { columnName = "time_end" }
                , Column
                    { columnType = SqlStdType (DataTypeTimeStamp Nothing True)
                    , columnConstraints = fromList [ NotNull ]
                    }
                )
            , ( ColumnName { columnName = "time_start" }
                , Column
                    { columnType = SqlStdType (DataTypeTimeStamp Nothing True)
                    , columnConstraints = fromList [ NotNull ]
                    }
                )
            ]
        }
)

When I let beam-automigrate manage the schema, it suggests the following migrations:

CREATE SEQUENCE runs___id___seq;
ALTER TABLE runs ALTER COLUMN id TYPE NUMERIC(10);
ALTER TABLE runs ALTER COLUMN id SET DEFAULT nextval('runs___id___seq'::regclass);
ALTER TABLE runs ALTER COLUMN id DROP DEFAULT;

So, the migration:

  1. Changes the type of the id column from INT to NUMERIC(10)
  2. Replaces the existing sequence for id (runs_id_seq) with a new sequence (runs___id___seq)

I have not executed the migration yet, but as far as I can see this will not work because nextval for the new sequence will not return the same as nextval for the old sequence -- thus causing a primary key conflict on the next insert into runs with a default value for id.

Secondly, while grepping for nextval in the code base, I came across:

-- If a PrimaryKey is referenced in another table, we do not want to recreate the \"SEQUENCE\" and
-- \"nextval\" default constraint associated with the original field.
which indicates that migrating sequences in this manner isn't even intended in the first place. Is this the case or am I misunderstanding the comment in question?

@runeksvendsen
Copy link
Member Author

I have now had a chance to apply the suggested migration to the database in question.

The result is that the next insert in the runs database results in an error because DEFAULT is translated into null:

ERROR:  null value in column "id" violates not-null constraint
DETAIL:  Failing row contains (null, 2021-01-02 13:58:16.473601+00, 2021-01-02 13:59:00.013016+00).
STATEMENT:  INSERT INTO "runs"("id", "time_start", "time_end") VALUES (DEFAULT, '2021-01-02 13:58:16.473601Z', '2021-01-02 13:59:00.013016Z') RETURNING "id", "time_start", "time_end"

Which I guess is caused by the last DROP DEFAULT statement.

The expected error appears if I remove the DROP DEFAULT statement:

ERROR:  duplicate key value violates unique constraint "runs_pkey"
DETAIL:  Key (id)=(1) already exists.
STATEMENT:  INSERT INTO "runs"("id", "time_start", "time_end") VALUES (DEFAULT, '2021-01-02 14:25:12.027683Z', '2021-01-02 14:25:55.322397Z') RETURNING "id", "time_start", "time_end"

@runeksvendsen
Copy link
Member Author

runeksvendsen commented Jan 3, 2021

It appears that the existing sequences aren't found due to this piece of code:

case T.splitOn "___" seqName of

The existing sequences use a different naming schema (with a single underscore as separator -- e.g. runs_id_seq instead of runs___id___seq), so getSequence does not detect these.

Could the solution perhaps be to change the sequencesQ query to:

SELECT column_default, table_name, column_name 
FROM information_schema.columns 
WHERE column_default LIKE 'nextval(''_%''::regclass)'

and then, inside getSequence, extracting the sequence name from the column_default field using the regular expression nextval\('([_a-z][a-z0-9_]*)'::regclass\) (and the table and column name directly from the table_name and column_name fields)?

This would require something akin to the following changes for sequencesQ and getSequence (of getSchema) in Database.Beam.AutoMigrate.Postgres.

sequencesQ :: Pg.Query
sequencesQ = fromString
  "SELECT column_default, table_name, column_name FROM information_schema.columns WHERE column_default LIKE 'nextval(''%''::regclass)'"
getSequence ::
  Sequences ->
  (Text, Text, Text) ->
  IO Sequences
getSequence allSeqs (columnDefault, tName, cName) =
  case T.stripPrefix "nextval('" columnDefault >>= T.stripSuffix "'::regclass)" of
    Just seqName ->
      pure $ M.insert (SequenceName seqName) (Sequence (TableName tName) (ColumnName cName)) allSeqs
    Nothing -> pure allSeqs

@runeksvendsen
Copy link
Member Author

runeksvendsen commented Jan 3, 2021

Alternatively, the string matching could be done entirely in postgres using the following query.

SELECT c.relname, i.table_name, i.column_name
FROM   information_schema.columns i
JOIN   pg_class c ON EXISTS (
    SELECT *
    FROM   regexp_matches(i.column_default, 'nextval\(''([_a-z][a-z0-9_]*)''::regclass\)') x(y)
    WHERE  y[1] = c.relname
    AND    c.relkind = 'S'
    );

which has the advantage that we make sure the given sequence (extracted from column_default in information_schema.columns) is a sequence according to the pg_class table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant