Skip to content

Commit 07d8d08

Browse files
Ensure lex ordering of inferred migration names with respect to pgroll migrations (#899)
Ensure that inferred migrations are named appropriately with respect to user created migrations to ensure correct lexicographical ordering of migration files on disk. ## Reproduction Before this PR, the problem with inferred migration naming was reproducible as follows: Create the following migration as `01_first_migration.yaml`: ```yaml operations: - sql: up: SELECT 1 ``` Apply the migration with `pgroll start --complete` . On the target database, run some DDL: ```sql CREATE TABLE foo(a int PRIMARY KEY); ``` Create another migration, `02_second_migration.yaml`: ```yaml operations: - sql: up: SELECT 1 ``` Apply the migration with `pgroll start --complete` . Now pull the migration history into a temporary directory: ```yaml $ pgroll pull tmp/ ``` The migration history is not in the correct lexicographical order; the inferred migration is ordered after the user-created migrations: ``` ├──  01_first_migration.yaml ├──  02_second_migration.yaml └──  sql_1fb6e25eba9678.yaml ``` ## New behaviour This PR fixes the problem by naming inferred migrations so that they appear in the correct lexicographical order when pulled to disk with `pgroll pull`: The same steps as in the reproduction now result in these files on disk: ``` ├──  01_first_migration.yaml ├──  01_first_migration_20250613120930931268.yaml └──  02_second_migration.yaml ``` The inferred migration is now lex-ordered correctly between the two user migrations by appending a fixed width timestamp suffix to the previous migration name. The inferred migration has its `version_schema` field set: `cat 01_first_migration_20250613120930931268.yaml`: ``` version_schema: sql_e842123c operations: - sql: up: create table foo(id serial primary key) ``` This is to ensure that the version schema for the inferred migration does not risk exceeding the Postgres 63 character limit for schema names. --- Fixes #882
1 parent 4971378 commit 07d8d08

File tree

2 files changed

+92
-6
lines changed

2 files changed

+92
-6
lines changed

pkg/state/init.sql

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -471,12 +471,36 @@ BEGIN
471471
AND migration_type = 'inferred'
472472
AND migration -> 'operations' -> 0 -> 'sql' ->> 'up' = current_query();
473473
-- Someone did a schema change without pgroll, include it in the history
474-
SELECT
475-
INTO migration_id pg_catalog.format('sql_%s', pg_catalog.substr(pg_catalog.md5(pg_catalog.random()::text), 0, 15));
474+
-- Get the latest non-inferred migration name with microsecond timestamp for ordering
475+
WITH latest_non_inferred AS (
476+
SELECT
477+
name
478+
FROM
479+
placeholder.migrations
480+
WHERE
481+
SCHEMA = schemaname
482+
AND migration_type != 'inferred'
483+
ORDER BY
484+
created_at DESC
485+
LIMIT 1
486+
)
487+
SELECT
488+
INTO migration_id CASE WHEN EXISTS (
489+
SELECT
490+
1
491+
FROM
492+
latest_non_inferred) THEN
493+
pg_catalog.format('%s_%s', (
494+
SELECT
495+
name
496+
FROM latest_non_inferred), pg_catalog.to_char(pg_catalog.clock_timestamp(), 'YYYYMMDDHH24MISSUS'))
497+
ELSE
498+
pg_catalog.format('00000_initial_%s', pg_catalog.to_char(pg_catalog.clock_timestamp(), 'YYYYMMDDHH24MISSUS'))
499+
END;
476500
INSERT INTO placeholder.migrations (schema, name, migration, resulting_schema, done, parent, migration_type, created_at, updated_at)
477-
VALUES (schemaname, migration_id, pg_catalog.json_build_object('operations', (
478-
SELECT
479-
pg_catalog.json_agg(pg_catalog.json_build_object('sql', pg_catalog.json_build_object('up', pg_catalog.current_query()))))),
501+
VALUES (schemaname, migration_id, pg_catalog.json_build_object('version_schema', 'sql_' || substring(md5(random()::text), 1, 8), 'operations', (
502+
SELECT
503+
pg_catalog.json_agg(pg_catalog.json_build_object('sql', pg_catalog.json_build_object('up', pg_catalog.current_query()))))),
480504
placeholder.read_schema (schemaname),
481505
TRUE,
482506
placeholder.latest_migration (schemaname),

pkg/state/state_test.go

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import (
1515
"github.com/stretchr/testify/require"
1616

1717
"github.com/xataio/pgroll/internal/testutils"
18+
"github.com/xataio/pgroll/pkg/backfill"
1819
"github.com/xataio/pgroll/pkg/migrations"
20+
"github.com/xataio/pgroll/pkg/roll"
1921
"github.com/xataio/pgroll/pkg/schema"
2022
"github.com/xataio/pgroll/pkg/state"
2123
)
@@ -263,13 +265,73 @@ func TestInferredMigration(t *testing.T) {
263265
for i, wantMigration := range tt.wantMigrations {
264266
gotMigration := gotMigrations[i]
265267

266-
assert.Equal(t, wantMigration, gotMigration)
268+
// Ensure that the operations are equal; we don't care about the
269+
// randomly generated `VersionSchema` field for the migration.
270+
assert.Equal(t, wantMigration.Operations, gotMigration.Operations)
267271
}
268272
})
269273
}
270274
})
271275
}
272276

277+
func TestInferredMigrationsHaveExpectedNamesAndVersionSchema(t *testing.T) {
278+
t.Parallel()
279+
280+
t.Run("an inferred migration first in the history", func(t *testing.T) {
281+
testutils.WithStateAndConnectionToContainer(t, func(st *state.State, db *sql.DB) {
282+
ctx := context.Background()
283+
284+
// Execute a SQL DDL statement
285+
_, err := db.ExecContext(ctx, "CREATE TABLE table1(id int)")
286+
require.NoError(t, err)
287+
288+
// Get the migration history
289+
history, err := st.SchemaHistory(ctx, "public")
290+
require.NoError(t, err)
291+
292+
// Assert that the history contains one migration with the expected name
293+
require.Len(t, history, 1)
294+
require.Regexp(t, `^00000_initial_\d{20}$`, history[0].Migration.Name)
295+
// Assert that the VersionSchema field matches the expected format
296+
require.Regexp(t, "^sql_[0-9a-f]{8}", history[0].Migration.VersionSchema)
297+
})
298+
})
299+
300+
t.Run("an inferred migration following a pgroll migration", func(t *testing.T) {
301+
testutils.WithMigratorAndConnectionToContainer(t, func(m *roll.Roll, db *sql.DB) {
302+
ctx := context.Background()
303+
304+
// Execute a pgroll migration
305+
err := m.Start(ctx, &migrations.Migration{
306+
Name: "01_initial_migration",
307+
Operations: migrations.Operations{&migrations.OpRawSQL{Up: "SELECT 1"}},
308+
}, backfill.NewConfig())
309+
require.NoError(t, err)
310+
311+
// Complete the migration
312+
err = m.Complete(ctx)
313+
require.NoError(t, err)
314+
315+
// Execute a SQL DDL statement
316+
_, err = db.ExecContext(ctx, "CREATE TABLE table1(id int)")
317+
require.NoError(t, err)
318+
319+
// Get the migration history
320+
history, err := m.State().SchemaHistory(ctx, "public")
321+
require.NoError(t, err)
322+
323+
// Assert that the history contains two migrations with the expected
324+
// names. The inferred migration should have a name that starts with
325+
// the name of the preceding pgroll migration followed by a timestamp.
326+
require.Len(t, history, 2)
327+
require.Equal(t, "01_initial_migration", history[0].Migration.Name)
328+
require.Regexp(t, `^01_initial_migration_\d{20}$`, history[1].Migration.Name)
329+
// Assert that the VersionSchema field matches the expected format
330+
require.Regexp(t, "^sql_[0-9a-f]{8}", history[1].Migration.VersionSchema)
331+
})
332+
})
333+
}
334+
273335
func TestInferredMigrationsInTransactionHaveDifferentTimestamps(t *testing.T) {
274336
ctx := context.Background()
275337

0 commit comments

Comments
 (0)