Remove support for the name field in migrations (#852)

Remove support for specifying a `name` field in migration files:

```yaml
name: 01_some_migration # <-- no longer supported
operations:
  - create_table:
      name: products
      columns:
        - name: id
          type: serial
          pk: true
        - name: name
          type: varchar(255)
```

The migration filename is the source of truth for the migration name.
The `name` field was made optional and its use discouraged in the
documentation in #744. This PR removes support for it entirely.

⚠️ **Warning**

**This is a breaking change:** migration files that set the `name` field
will not be runnable by `pgroll` until the `name` field is removed.

Because of #812 `pgroll pull` and
`pgroll migrate` will still work with migration histories that contain
migrations with `name` fields.

The easiest way to remove the `name` fields from all migrations in the
history is to `pgroll pull` the migration history; this will serialize
all migrations in the remote history, removing the any `name` fields.

---

Closes #715 and #850

Author: andrew-farries

docs/operations/README.md

@@ -5,15 +5,13 @@
 YAML migration:
 
 ```yaml
-name: "0x_migration_name"
 operations: [...]
 ```
 
 JSON migration:
 
 ```json
 {
-  "name": "0x_migration_name",
   "operations": [...]
 }
 ```

docs/tutorial.md

@@ -42,7 +42,6 @@ With `pgroll` initialized, let's run our first migration. Here is a migration to
 
 <YamlJsonTabs>
 ```yaml
-name: "01_create_users_table"
 operations:
   - create_table:
       name: users
@@ -59,7 +58,6 @@ operations:
 ```
 ```json
 {
-  "name": "01_create_users_table",
   "operations": [
     {
       "create_table": {
@@ -90,8 +88,6 @@ operations:
 
 Take this file and save it as `sql/01_create_users_table.yaml`.
 
-> Note: The `name` field is optional. If not provided, `pgroll` will use the filename (without the `.yaml` extension) as the migration name. In this example, the file is saved as `sql/01_create_users_table.yaml`, so the name would be `01_create_users_table` if not explicitly provided. It's recommended to write migrations without an explicit `name` field and allow the filename to determine the name of the migration.
-
 The migration will create a `users` table with three columns. It is equivalent to the following SQL DDL statement:
 
 ```sql
@@ -152,7 +148,6 @@ Here is the `pgroll` migration that will perform the migration to make the `desc
 
 <YamlJsonTabs>
 ```yaml
-name: "02_user_description_set_nullable"
 operations:
   - alter_column:
       table: users
@@ -163,7 +158,6 @@ operations:
 ```
 ```json
 {
-  "name": "02_user_description_set_nullable",
   "operations": [
     {
       "alter_column": {
@@ -179,8 +173,6 @@ operations:
 ```
 </YamlJsonTabs>
 
-> As mentioned earlier, the `name` field is optional. If the file is saved as `sql/02_user_description_set_nullable.yaml`, `pgroll` will use `02_user_description_set_nullable` as the migration name if the name field is not provided.
-
 Save this migration as `sql/02_user_description_set_nullable.yaml` and start the migration:
 
 ```
@@ -478,7 +470,6 @@ Looking at the second of these items, rollbacks, let's see how to roll back a `p
 
 <YamlJsonTabs>
 ```yaml
-name: "03_add_is_active_column"
 operations:
   - add_column:
       table: users
@@ -490,7 +481,6 @@ operations:
 ```
 ```json
 {
-  "name": "03_add_is_active_column",
   "operations": [
     {
       "add_column": {
@@ -508,7 +498,7 @@ operations:
 ```
 </YamlJsonTabs>
 
-> Again, the `name` field is optional. The filename will be used as the default name if not specified.
+Create this migration and save it as `sql/03_add_is_active_column.yaml`.
 
 (the misspelling of `is_active` is intentional!)
 

pkg/migrations/migrations.go

@@ -68,11 +68,11 @@ type RequiresSchemaRefreshOperation interface {
 type (
 	Operations []Operation
 	Migration  struct {
-		Name       string     `json:"name,omitempty"`
+		Name       string     `json:"-"`
 		Operations Operations `json:"operations"`
 	}
 	RawMigration struct {
-		Name       string          `json:"name"`
+		Name       string          `json:"-"`
 		Operations json.RawMessage `json:"operations"`
 	}
 )

pkg/migrations/op_common.go

@@ -118,10 +118,8 @@ func ReadRawMigration(dir fs.FS, filename string) (*RawMigration, error) {
 		return nil, fmt.Errorf("reading migration file: %w", err)
 	}
 
-	if mig.Name == "" {
-		// Extract base filename without extension as the default migration name
-		mig.Name = strings.TrimSuffix(filepath.Base(filename), filepath.Ext(filename))
-	}
+	// Extract base filename wiahout extension as the migration name
+	mig.Name = strings.TrimSuffix(filepath.Base(filename), filepath.Ext(filename))
 
 	return &mig, nil
 }

pkg/roll/unapplied_test.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
-	"fmt"
 	"maps"
 	"slices"
 	"testing"
@@ -54,12 +53,11 @@ func TestUnappliedMigrations(t *testing.T) {
 			ctx := context.Background()
 
 			// Unmarshal the first migration from the migrations directory
-			var migration migrations.Migration
-			err := json.Unmarshal(fs["01_migration_1.json"].Data, &migration)
+			migration, err := migrations.ReadMigration(fs, "01_migration_1.json")
 			require.NoError(t, err)
 
 			// Apply the first migration
-			err = roll.Start(ctx, &migration, backfill.NewConfig())
+			err = roll.Start(ctx, migration, backfill.NewConfig())
 			require.NoError(t, err)
 			err = roll.Complete(ctx)
 			require.NoError(t, err)
@@ -87,11 +85,10 @@ func TestUnappliedMigrations(t *testing.T) {
 
 			// Unmarshal and apply all migrations from the migrations directory
 			for _, filename := range slices.Sorted(maps.Keys(fs)) {
-				var migration migrations.Migration
-				err := json.Unmarshal(fs[filename].Data, &migration)
+				migration, err := migrations.ReadMigration(fs, filename)
 				require.NoError(t, err)
 
-				err = roll.Start(ctx, &migration, backfill.NewConfig())
+				err = roll.Start(ctx, migration, backfill.NewConfig())
 				require.NoError(t, err)
 				err = roll.Complete(ctx)
 				require.NoError(t, err)
@@ -284,12 +281,11 @@ func TestUnappliedMigrationsWithBaselines(t *testing.T) {
 			require.NoError(t, err)
 
 			// Unmarshal the migration that comes after the baseline version
-			var migration migrations.Migration
-			err = json.Unmarshal(fs["03_migration_3.json"].Data, &migration)
+			migration, err := migrations.ReadMigration(fs, "03_migration_3.json")
 			require.NoError(t, err)
 
 			// Apply the migration
-			err = roll.Start(ctx, &migration, backfill.NewConfig())
+			err = roll.Start(ctx, migration, backfill.NewConfig())
 			require.NoError(t, err)
 			err = roll.Complete(ctx)
 			require.NoError(t, err)
@@ -353,10 +349,9 @@ func TestUnappliedMigrationsWithBaselines(t *testing.T) {
 			require.NoError(t, err)
 
 			// Apply a migration after the first baseline
-			var migration migrations.Migration
-			err = json.Unmarshal(fs["03_migration_3.json"].Data, &migration)
+			migration, err := migrations.ReadMigration(fs, "03_migration_3.json")
 			require.NoError(t, err)
-			err = roll.Start(ctx, &migration, backfill.NewConfig())
+			err = roll.Start(ctx, migration, backfill.NewConfig())
 			require.NoError(t, err)
 			err = roll.Complete(ctx)
 			require.NoError(t, err)
@@ -412,12 +407,11 @@ func TestUnappliedMigrationsWithOldMigrationFormats(t *testing.T) {
 			ctx := context.Background()
 
 			// Unmarshal the first migration from the migrations directory
-			var migration migrations.Migration
-			err := json.Unmarshal(fs["01_migration_1.json"].Data, &migration)
+			migration, err := migrations.ReadMigration(fs, "01_migration_1.json")
 			require.NoError(t, err)
 
 			// Apply the first migration
-			err = roll.Start(ctx, &migration, backfill.NewConfig())
+			err = roll.Start(ctx, migration, backfill.NewConfig())
 			require.NoError(t, err)
 			err = roll.Complete(ctx)
 			require.NoError(t, err)
@@ -465,17 +459,15 @@ func exampleMigration(t *testing.T, name string) []byte {
 func unDeserializableMigration(t *testing.T, name string) []byte {
 	t.Helper()
 
-	migJSON := fmt.Sprintf(`{
-		"name": "%s",
+	migJSON := `{
 		"operations": [
 			{
 				"sql": {
 					"upxxx": "SELECT 1"
 				}
 			}
 		]
-	}
-	`, name)
+	}`
 
 	return []byte(migJSON)
 }

pkg/state/history.go

@@ -62,6 +62,7 @@ func (s *State) SchemaHistory(ctx context.Context, schema string) ([]HistoryEntr
 		if err != nil {
 			return nil, err
 		}
+		mig.Name = name
 
 		entries = append(entries, HistoryEntry{
 			Migration: mig,

pkg/state/state.go

@@ -168,6 +168,7 @@ func (s *State) GetActiveMigration(ctx context.Context, schema string) (*migrati
 	if err != nil {
 		return nil, fmt.Errorf("unable to unmarshal migration: %w", err)
 	}
+	migration.Name = name
 
 	return &migration, nil
 }

pkg/state/state_test.go

@@ -7,7 +7,6 @@ import (
 	"database/sql"
 	"encoding/json"
 	"fmt"
-	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -260,10 +259,6 @@ func TestInferredMigration(t *testing.T) {
 				for i, wantMigration := range tt.wantMigrations {
 					gotMigration := gotMigrations[i]
 
-					// test there is a name for the migration, then remove it for the comparison
-					assert.True(t, strings.HasPrefix(gotMigration.Name, "sql_") && len(gotMigration.Name) > 10)
-					gotMigration.Name = ""
-
 					assert.Equal(t, wantMigration, gotMigration)
 				}
 			})