From 7ad55a17f9f7f6bbf7d445193a7e9e50d48756a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Tue, 6 Feb 2024 16:53:28 +0100 Subject: [PATCH 01/25] Allow to execute SQL migrations on complete by default, SQL migrations run on start. This change allows for special SQL migrations that run only on the complete step. --- pkg/migrations/op_raw_sql.go | 19 ++++++++++++++-- pkg/migrations/op_raw_sql_test.go | 37 +++++++++++++++++++++++++++++++ pkg/migrations/types.go | 3 +++ schema.json | 4 ++++ 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index 8d3a70b2..3742f8b6 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -12,14 +12,18 @@ import ( var _ Operation = (*OpRawSQL)(nil) func (o *OpRawSQL) Start(ctx context.Context, conn *sql.DB, stateSchema string, s *schema.Schema, cbs ...CallbackFn) error { - _, err := conn.ExecContext(ctx, o.Up) - if err != nil { + if !o.onComplete() { + _, err := conn.ExecContext(ctx, o.Up) return err } return nil } func (o *OpRawSQL) Complete(ctx context.Context, conn *sql.DB, s *schema.Schema) error { + if o.onComplete() { + _, err := conn.ExecContext(ctx, o.Up) + return err + } return nil } @@ -36,9 +40,20 @@ func (o *OpRawSQL) Validate(ctx context.Context, s *schema.Schema) error { return EmptyMigrationError{} } + if o.onComplete() && o.Down != "" { + return InvalidMigrationError{Reason: "down is not allowed with onComplete"} + } + return nil } +func (o *OpRawSQL) onComplete() bool { + if o.OnComplete != nil { + return *o.OnComplete + } + return false +} + // this operation is isolated, cannot be executed with other operations func (o *OpRawSQL) IsIsolated() {} diff --git a/pkg/migrations/op_raw_sql_test.go b/pkg/migrations/op_raw_sql_test.go index 5931a58d..35c6de2f 100644 --- a/pkg/migrations/op_raw_sql_test.go +++ b/pkg/migrations/op_raw_sql_test.go @@ -11,6 +11,7 @@ import ( func TestRawSQL(t *testing.T) { t.Parallel() + vTrue := true ExecuteTests(t, TestCases{ { @@ -53,6 +54,42 @@ func TestRawSQL(t *testing.T) { }) }, }, + { + name: "raw SQL with onComplete", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpRawSQL{ + OnComplete: &vTrue, + Up: ` + CREATE TABLE test_table ( + id serial, + name text + ) + `, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // SQL didn't run yet + TableMustNotExist(t, db, schema, "test_table") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // table is dropped after rollback + TableMustNotExist(t, db, schema, "test_table") + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // table can be accessed after start + ViewMustExist(t, db, schema, "01_create_table", "test_table") + + // inserts work + MustInsert(t, db, schema, "01_create_table", "test_table", map[string]string{ + "name": "foo", + }) + }, + }, { name: "migration on top of raw SQL", migrations: []migrations.Migration{ diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index aaaaaebc..290220a6 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -171,6 +171,9 @@ type OpRawSQL struct { // SQL expression for down migration Down string `json:"down,omitempty"` + // SQL expression will run on complete step (rather than on start) + OnComplete *bool `json:"onComplete,omitempty"` + // SQL expression for up migration Up string `json:"up"` } diff --git a/schema.json b/schema.json index 5b532a3b..e954014d 100644 --- a/schema.json +++ b/schema.json @@ -312,6 +312,10 @@ "up": { "description": "SQL expression for up migration", "type": "string" + }, + "onComplete": { + "description": "SQL expression will run on complete step (rather than on start)", + "type": "boolean" } }, "required": ["up"], From 24f6822d540ee737e25b3503193f677cb6f82613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Wed, 7 Feb 2024 17:15:14 +0100 Subject: [PATCH 02/25] Allow raw sql op next to other ops --- pkg/migrations/migrations.go | 15 ------------ pkg/migrations/migrations_test.go | 40 ------------------------------- pkg/migrations/op_raw_sql.go | 3 --- 3 files changed, 58 deletions(-) delete mode 100644 pkg/migrations/migrations_test.go diff --git a/pkg/migrations/migrations.go b/pkg/migrations/migrations.go index 2ea276e8..b7f7326d 100644 --- a/pkg/migrations/migrations.go +++ b/pkg/migrations/migrations.go @@ -5,7 +5,6 @@ package migrations import ( "context" "database/sql" - "fmt" _ "github.com/lib/pq" "github.com/xataio/pgroll/pkg/schema" @@ -32,12 +31,6 @@ type Operation interface { Validate(ctx context.Context, s *schema.Schema) error } -// IsolatedOperation is an operation that cannot be executed with other operations -// in the same migration -type IsolatedOperation interface { - IsIsolated() -} - // RequiresSchemaRefreshOperation is an operation that requires the resulting schema to be refreshed type RequiresSchemaRefreshOperation interface { RequiresSchemaRefresh() @@ -55,14 +48,6 @@ type ( // Validate will check that the migration can be applied to the given schema // returns a descriptive error if the migration is invalid func (m *Migration) Validate(ctx context.Context, s *schema.Schema) error { - for _, op := range m.Operations { - if _, ok := op.(IsolatedOperation); ok { - if len(m.Operations) > 1 { - return InvalidMigrationError{Reason: fmt.Sprintf("operation %q cannot be executed with other operations", OperationName(op))} - } - } - } - for _, op := range m.Operations { err := op.Validate(ctx, s) if err != nil { diff --git a/pkg/migrations/migrations_test.go b/pkg/migrations/migrations_test.go deleted file mode 100644 index 80676770..00000000 --- a/pkg/migrations/migrations_test.go +++ /dev/null @@ -1,40 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -package migrations - -import ( - "context" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/xataio/pgroll/pkg/schema" -) - -func TestMigrationsIsolated(t *testing.T) { - migration := Migration{ - Name: "sql", - Operations: Operations{ - &OpRawSQL{ - Up: `foo`, - }, - &OpRenameColumn{}, - }, - } - - err := migration.Validate(context.TODO(), schema.New()) - var wantErr InvalidMigrationError - assert.ErrorAs(t, err, &wantErr) -} - -func TestMigrationsIsolatedValid(t *testing.T) { - migration := Migration{ - Name: "sql", - Operations: Operations{ - &OpRawSQL{ - Up: `foo`, - }, - }, - } - err := migration.Validate(context.TODO(), schema.New()) - assert.NoError(t, err) -} diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index 3742f8b6..baa8ad6c 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -54,8 +54,5 @@ func (o *OpRawSQL) onComplete() bool { return false } -// this operation is isolated, cannot be executed with other operations -func (o *OpRawSQL) IsIsolated() {} - // this operation requires the resulting schema to be refreshed func (o *OpRawSQL) RequiresSchemaRefresh() {} From 4252c03af3682d5e1e3866b5cb0aaee2ec890768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Wed, 7 Feb 2024 17:15:33 +0100 Subject: [PATCH 03/25] update docs on `onComplete` --- docs/README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/README.md b/docs/README.md index b059ce2a..1673c287 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1072,6 +1072,19 @@ Example **raw SQL** migrations: * [05_sql.json](../examples/05_sql.json) +For convenience, `sql` migration allows to run the `up` expression on the complete phase (instead of the default, which is to run it on the start phase) by setting the `onComplete` flag: + +```json +{ + "sql": { + "up": "SQL expression", + "onComplete": true + } +} +``` + +`onComplete` flag is incompatible with `down` expression, as `pgroll` does not support running rollback after complete was executed. + ### Rename table A rename table operation renames a table. From 2ae739320e2cecbd37d96eac1b3d248d4d4e7c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Wed, 7 Feb 2024 17:51:41 +0100 Subject: [PATCH 04/25] Revert "Allow raw sql op next to other ops" This reverts commit 24f6822d540ee737e25b3503193f677cb6f82613. --- pkg/migrations/migrations.go | 15 ++++++++++++ pkg/migrations/migrations_test.go | 40 +++++++++++++++++++++++++++++++ pkg/migrations/op_raw_sql.go | 3 +++ 3 files changed, 58 insertions(+) create mode 100644 pkg/migrations/migrations_test.go diff --git a/pkg/migrations/migrations.go b/pkg/migrations/migrations.go index b7f7326d..2ea276e8 100644 --- a/pkg/migrations/migrations.go +++ b/pkg/migrations/migrations.go @@ -5,6 +5,7 @@ package migrations import ( "context" "database/sql" + "fmt" _ "github.com/lib/pq" "github.com/xataio/pgroll/pkg/schema" @@ -31,6 +32,12 @@ type Operation interface { Validate(ctx context.Context, s *schema.Schema) error } +// IsolatedOperation is an operation that cannot be executed with other operations +// in the same migration +type IsolatedOperation interface { + IsIsolated() +} + // RequiresSchemaRefreshOperation is an operation that requires the resulting schema to be refreshed type RequiresSchemaRefreshOperation interface { RequiresSchemaRefresh() @@ -48,6 +55,14 @@ type ( // Validate will check that the migration can be applied to the given schema // returns a descriptive error if the migration is invalid func (m *Migration) Validate(ctx context.Context, s *schema.Schema) error { + for _, op := range m.Operations { + if _, ok := op.(IsolatedOperation); ok { + if len(m.Operations) > 1 { + return InvalidMigrationError{Reason: fmt.Sprintf("operation %q cannot be executed with other operations", OperationName(op))} + } + } + } + for _, op := range m.Operations { err := op.Validate(ctx, s) if err != nil { diff --git a/pkg/migrations/migrations_test.go b/pkg/migrations/migrations_test.go new file mode 100644 index 00000000..80676770 --- /dev/null +++ b/pkg/migrations/migrations_test.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 + +package migrations + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/xataio/pgroll/pkg/schema" +) + +func TestMigrationsIsolated(t *testing.T) { + migration := Migration{ + Name: "sql", + Operations: Operations{ + &OpRawSQL{ + Up: `foo`, + }, + &OpRenameColumn{}, + }, + } + + err := migration.Validate(context.TODO(), schema.New()) + var wantErr InvalidMigrationError + assert.ErrorAs(t, err, &wantErr) +} + +func TestMigrationsIsolatedValid(t *testing.T) { + migration := Migration{ + Name: "sql", + Operations: Operations{ + &OpRawSQL{ + Up: `foo`, + }, + }, + } + err := migration.Validate(context.TODO(), schema.New()) + assert.NoError(t, err) +} diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index baa8ad6c..3742f8b6 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -54,5 +54,8 @@ func (o *OpRawSQL) onComplete() bool { return false } +// this operation is isolated, cannot be executed with other operations +func (o *OpRawSQL) IsIsolated() {} + // this operation requires the resulting schema to be refreshed func (o *OpRawSQL) RequiresSchemaRefresh() {} From e3f17953416613ce75538b3907ce9f71ca76e517 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Thu, 8 Feb 2024 09:39:47 +0100 Subject: [PATCH 05/25] fix isolation & refresh checks --- pkg/migrations/migrations.go | 8 ++++---- pkg/migrations/op_raw_sql.go | 12 ++++++++---- pkg/migrations/op_raw_sql_test.go | 4 ++-- pkg/roll/execute.go | 15 ++++++++++----- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pkg/migrations/migrations.go b/pkg/migrations/migrations.go index 2ea276e8..949a0894 100644 --- a/pkg/migrations/migrations.go +++ b/pkg/migrations/migrations.go @@ -35,12 +35,12 @@ type Operation interface { // IsolatedOperation is an operation that cannot be executed with other operations // in the same migration type IsolatedOperation interface { - IsIsolated() + IsIsolated() bool } // RequiresSchemaRefreshOperation is an operation that requires the resulting schema to be refreshed type RequiresSchemaRefreshOperation interface { - RequiresSchemaRefresh() + RequiresSchemaRefresh() bool } type ( @@ -56,8 +56,8 @@ type ( // returns a descriptive error if the migration is invalid func (m *Migration) Validate(ctx context.Context, s *schema.Schema) error { for _, op := range m.Operations { - if _, ok := op.(IsolatedOperation); ok { - if len(m.Operations) > 1 { + if isolatedOp, ok := op.(IsolatedOperation); ok { + if isolatedOp.IsIsolated() && len(m.Operations) > 1 { return InvalidMigrationError{Reason: fmt.Sprintf("operation %q cannot be executed with other operations", OperationName(op))} } } diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index 3742f8b6..bdee2e3d 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -54,8 +54,12 @@ func (o *OpRawSQL) onComplete() bool { return false } -// this operation is isolated, cannot be executed with other operations -func (o *OpRawSQL) IsIsolated() {} +// this operation is isolated when executed on start, cannot be executed with other operations +func (o *OpRawSQL) IsIsolated() bool { + return !o.onComplete() +} -// this operation requires the resulting schema to be refreshed -func (o *OpRawSQL) RequiresSchemaRefresh() {} +// this operation requires the resulting schema to be refreshed when executed on start +func (o *OpRawSQL) RequiresSchemaRefresh() bool { + return !o.onComplete() +} diff --git a/pkg/migrations/op_raw_sql_test.go b/pkg/migrations/op_raw_sql_test.go index 35c6de2f..53b8f88b 100644 --- a/pkg/migrations/op_raw_sql_test.go +++ b/pkg/migrations/op_raw_sql_test.go @@ -82,10 +82,10 @@ func TestRawSQL(t *testing.T) { }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { // table can be accessed after start - ViewMustExist(t, db, schema, "01_create_table", "test_table") + TableMustExist(t, db, schema, "test_table") // inserts work - MustInsert(t, db, schema, "01_create_table", "test_table", map[string]string{ + MustInsert(t, db, schema, "", "test_table", map[string]string{ "name": "foo", }) }, diff --git a/pkg/roll/execute.go b/pkg/roll/execute.go index 95d30820..54d92588 100644 --- a/pkg/roll/execute.go +++ b/pkg/roll/execute.go @@ -50,11 +50,13 @@ func (m *Roll) Start(ctx context.Context, migration *migrations.Migration, cbs . errRollback) } - if _, ok := op.(migrations.RequiresSchemaRefreshOperation); ok { - // refresh schema - newSchema, err = m.state.ReadSchema(ctx, m.schema) - if err != nil { - return fmt.Errorf("unable to refresh schema: %w", err) + if refreshOp, ok := op.(migrations.RequiresSchemaRefreshOperation); ok { + if refreshOp.RequiresSchemaRefresh() { + // refresh schema + newSchema, err = m.state.ReadSchema(ctx, m.schema) + if err != nil { + return fmt.Errorf("unable to refresh schema: %w", err) + } } } } @@ -192,5 +194,8 @@ func (m *Roll) createView(ctx context.Context, version, name string, table schem } func VersionedSchemaName(schema string, version string) string { + if version == "" { + return schema + } return schema + "_" + version } From fe251821c28a32360f762740cd7f1bed81761aa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Thu, 8 Feb 2024 16:26:47 +0100 Subject: [PATCH 06/25] fix views after complete --- pkg/roll/execute.go | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/pkg/roll/execute.go b/pkg/roll/execute.go index 54d92588..bc1af834 100644 --- a/pkg/roll/execute.go +++ b/pkg/roll/execute.go @@ -6,11 +6,13 @@ import ( "context" "errors" "fmt" + "sort" "strings" "github.com/lib/pq" "github.com/xataio/pgroll/pkg/migrations" "github.com/xataio/pgroll/pkg/schema" + "golang.org/x/exp/maps" ) // Start will apply the required changes to enable supporting the new schema version @@ -66,16 +68,21 @@ func (m *Roll) Start(ctx context.Context, migration *migrations.Migration, cbs . return nil } + // create views for the new version + return m.ensureViews(ctx, newSchema, migration.Name) +} + +func (m *Roll) ensureViews(ctx context.Context, schema *schema.Schema, version string) error { // create schema for the new version - versionSchema := VersionedSchemaName(m.schema, migration.Name) - _, err = m.pgConn.ExecContext(ctx, fmt.Sprintf("CREATE SCHEMA IF NOT EXISTS %s", pq.QuoteIdentifier(versionSchema))) + versionSchema := VersionedSchemaName(m.schema, version) + _, err := m.pgConn.ExecContext(ctx, fmt.Sprintf("CREATE SCHEMA IF NOT EXISTS %s", pq.QuoteIdentifier(versionSchema))) if err != nil { return err } // create views in the new schema - for name, table := range newSchema.Tables { - err = m.createView(ctx, migration.Name, name, table) + for name, table := range schema.Tables { + err = m.ensureView(ctx, version, name, table) if err != nil { return fmt.Errorf("unable to create view: %w", err) } @@ -121,6 +128,19 @@ func (m *Roll) Complete(ctx context.Context) error { } } + // recreate views for the new version (if some operations require it, ie SQL) + if !m.disableVersionSchemas { + schema, err = m.state.ReadSchema(ctx, m.schema) + if err != nil { + return fmt.Errorf("unable to read schema: %w", err) + } + + err = m.ensureViews(ctx, schema, migration.Name) + if err != nil { + return err + } + } + // mark as completed err = m.state.Complete(ctx, m.schema, migration.Name) if err != nil { @@ -164,10 +184,16 @@ func (m *Roll) Rollback(ctx context.Context) error { } // create view creates a view for the new version of the schema -func (m *Roll) createView(ctx context.Context, version, name string, table schema.Table) error { +func (m *Roll) ensureView(ctx context.Context, version, name string, table schema.Table) error { columns := make([]string, 0, len(table.Columns)) - for k, v := range table.Columns { - columns = append(columns, fmt.Sprintf("%s AS %s", pq.QuoteIdentifier(v.Name), pq.QuoteIdentifier(k))) + // iterate over all columns, sort them alphabetically to ensure consistency when recreating views + // get column names: + names := maps.Keys(table.Columns) + sort.Strings(names) + + for _, name := range names { + v := table.Columns[name] + columns = append(columns, fmt.Sprintf("%s AS %s", pq.QuoteIdentifier(v.Name), pq.QuoteIdentifier(name))) } // Create view with security_invoker option for PG 15+ From 477a83e5d0001f215dbae5d9af30ce4fb9ede8ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Thu, 8 Feb 2024 16:53:15 +0100 Subject: [PATCH 07/25] add more tests --- pkg/migrations/migrations_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/pkg/migrations/migrations_test.go b/pkg/migrations/migrations_test.go index 80676770..507eaf81 100644 --- a/pkg/migrations/migrations_test.go +++ b/pkg/migrations/migrations_test.go @@ -17,7 +17,7 @@ func TestMigrationsIsolated(t *testing.T) { &OpRawSQL{ Up: `foo`, }, - &OpRenameColumn{}, + &OpCreateTable{Name: "foo"}, }, } @@ -37,4 +37,18 @@ func TestMigrationsIsolatedValid(t *testing.T) { } err := migration.Validate(context.TODO(), schema.New()) assert.NoError(t, err) + + // Test onComplete + migration = Migration{ + Name: "sql", + Operations: Operations{ + &OpRawSQL{ + Up: `foo`, + OnComplete: &[]bool{true}[0], + }, + &OpCreateTable{Name: "foo"}, + }, + } + err = migration.Validate(context.TODO(), schema.New()) + assert.NoError(t, err) } From e2abb7f5d798f99e1950bbe20902bbb2e4be6c50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Thu, 8 Feb 2024 17:23:25 +0100 Subject: [PATCH 08/25] fix the logic --- pkg/migrations/op_raw_sql.go | 2 +- pkg/migrations/op_raw_sql_test.go | 43 +++++++++++++++++++++++++++++++ pkg/roll/execute.go | 34 ++++++++++++------------ 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index bdee2e3d..87b70a15 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -61,5 +61,5 @@ func (o *OpRawSQL) IsIsolated() bool { // this operation requires the resulting schema to be refreshed when executed on start func (o *OpRawSQL) RequiresSchemaRefresh() bool { - return !o.onComplete() + return true } diff --git a/pkg/migrations/op_raw_sql_test.go b/pkg/migrations/op_raw_sql_test.go index 53b8f88b..5755d227 100644 --- a/pkg/migrations/op_raw_sql_test.go +++ b/pkg/migrations/op_raw_sql_test.go @@ -90,6 +90,49 @@ func TestRawSQL(t *testing.T) { }) }, }, + { + name: "raw SQL after a migration with onComplete", + migrations: []migrations.Migration{ + { + Name: "01_create_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "test_table", + Columns: []migrations.Column{ + {Name: "id", Type: "serial"}, + {Name: "name", Type: "text"}, + }, + }, + &migrations.OpRawSQL{ + OnComplete: &vTrue, + Up: ` + ALTER TABLE test_table ADD COLUMN age int + `, + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // SQL didn't run yet + ViewMustExist(t, db, schema, "01_create_table", "test_table") + ColumnMustNotExist(t, db, schema, "test_table", "age") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // table is dropped after rollback + TableMustNotExist(t, db, schema, "test_table") + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // table can be accessed after start + TableMustExist(t, db, schema, "test_table") + ColumnMustExist(t, db, schema, "test_table", "age") + + // inserts work + MustInsert(t, db, schema, "01_create_table", "test_table", map[string]string{ + "name": "foo", + "age": "42", + }) + }, + }, { name: "migration on top of raw SQL", migrations: []migrations.Migration{ diff --git a/pkg/roll/execute.go b/pkg/roll/execute.go index bc1af834..73a1277f 100644 --- a/pkg/roll/execute.go +++ b/pkg/roll/execute.go @@ -6,13 +6,11 @@ import ( "context" "errors" "fmt" - "sort" "strings" "github.com/lib/pq" "github.com/xataio/pgroll/pkg/migrations" "github.com/xataio/pgroll/pkg/schema" - "golang.org/x/exp/maps" ) // Start will apply the required changes to enable supporting the new schema version @@ -51,10 +49,11 @@ func (m *Roll) Start(ctx context.Context, migration *migrations.Migration, cbs . fmt.Errorf("unable to execute start operation: %w", err), errRollback) } - - if refreshOp, ok := op.(migrations.RequiresSchemaRefreshOperation); ok { - if refreshOp.RequiresSchemaRefresh() { - // refresh schema + // refresh schema when the op is isolated and requires a refresh (for example raw sql) + // we don't want to refresh the schema if the operation is not isolated as it would + // override changes made by other operations + if refreshOp, ok := op.(migrations.RequiresSchemaRefreshOperation); ok && refreshOp.RequiresSchemaRefresh() { + if isolatedOp, ok := op.(migrations.IsolatedOperation); ok && isolatedOp.IsIsolated() { newSchema, err = m.state.ReadSchema(ctx, m.schema) if err != nil { return fmt.Errorf("unable to refresh schema: %w", err) @@ -121,15 +120,22 @@ func (m *Roll) Complete(ctx context.Context) error { } // execute operations + refreshViews := false for _, op := range migration.Operations { err := op.Complete(ctx, m.pgConn, schema) if err != nil { return fmt.Errorf("unable to execute complete operation: %w", err) } + + if refreshOp, ok := op.(migrations.RequiresSchemaRefreshOperation); ok { + if refreshOp.RequiresSchemaRefresh() { + refreshViews = true + } + } } // recreate views for the new version (if some operations require it, ie SQL) - if !m.disableVersionSchemas { + if refreshViews && !m.disableVersionSchemas { schema, err = m.state.ReadSchema(ctx, m.schema) if err != nil { return fmt.Errorf("unable to read schema: %w", err) @@ -186,14 +192,8 @@ func (m *Roll) Rollback(ctx context.Context) error { // create view creates a view for the new version of the schema func (m *Roll) ensureView(ctx context.Context, version, name string, table schema.Table) error { columns := make([]string, 0, len(table.Columns)) - // iterate over all columns, sort them alphabetically to ensure consistency when recreating views - // get column names: - names := maps.Keys(table.Columns) - sort.Strings(names) - - for _, name := range names { - v := table.Columns[name] - columns = append(columns, fmt.Sprintf("%s AS %s", pq.QuoteIdentifier(v.Name), pq.QuoteIdentifier(name))) + for k, v := range table.Columns { + columns = append(columns, fmt.Sprintf("%s AS %s", pq.QuoteIdentifier(v.Name), pq.QuoteIdentifier(k))) } // Create view with security_invoker option for PG 15+ @@ -207,7 +207,9 @@ func (m *Roll) ensureView(ctx context.Context, version, name string, table schem } _, err := m.pgConn.ExecContext(ctx, - fmt.Sprintf("CREATE OR REPLACE VIEW %s.%s %s AS SELECT %s FROM %s", + fmt.Sprintf("BEGIN; DROP VIEW IF EXISTS %s.%s; CREATE VIEW %s.%s %s AS SELECT %s FROM %s; COMMIT", + pq.QuoteIdentifier(VersionedSchemaName(m.schema, version)), + pq.QuoteIdentifier(name), pq.QuoteIdentifier(VersionedSchemaName(m.schema, version)), pq.QuoteIdentifier(name), withOptions, From 6350f1850339f01cc04b46da244381a7cbd222b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Thu, 8 Feb 2024 17:28:08 +0100 Subject: [PATCH 09/25] update docs --- docs/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index 1673c287..8c271bc8 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1072,7 +1072,7 @@ Example **raw SQL** migrations: * [05_sql.json](../examples/05_sql.json) -For convenience, `sql` migration allows to run the `up` expression on the complete phase (instead of the default, which is to run it on the start phase) by setting the `onComplete` flag: +By default, `sql` operation cannot run together with other operations in the same migration. This is to ensure pgroll can correctly track the state of the database. However, it is possible to run `sql` operation together with other operations by setting the `onComplete` flag to `true`: ```json { @@ -1083,6 +1083,8 @@ For convenience, `sql` migration allows to run the `up` expression on the comple } ``` +The `onComplete` flag will make this operation run the `up` expression on the complete phase (instead of the default, which is to run it on the start phase). + `onComplete` flag is incompatible with `down` expression, as `pgroll` does not support running rollback after complete was executed. ### Rename table From 659dbe3a5b9d129ca3274a7a75a45a1373e10871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Thu, 8 Feb 2024 17:32:35 +0100 Subject: [PATCH 10/25] remove uneeded change --- pkg/migrations/op_raw_sql_test.go | 2 +- pkg/roll/execute.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/migrations/op_raw_sql_test.go b/pkg/migrations/op_raw_sql_test.go index 5755d227..fe851c71 100644 --- a/pkg/migrations/op_raw_sql_test.go +++ b/pkg/migrations/op_raw_sql_test.go @@ -85,7 +85,7 @@ func TestRawSQL(t *testing.T) { TableMustExist(t, db, schema, "test_table") // inserts work - MustInsert(t, db, schema, "", "test_table", map[string]string{ + MustInsert(t, db, schema, "01_create_table", "test_table", map[string]string{ "name": "foo", }) }, diff --git a/pkg/roll/execute.go b/pkg/roll/execute.go index 73a1277f..09e28783 100644 --- a/pkg/roll/execute.go +++ b/pkg/roll/execute.go @@ -222,8 +222,5 @@ func (m *Roll) ensureView(ctx context.Context, version, name string, table schem } func VersionedSchemaName(schema string, version string) string { - if version == "" { - return schema - } return schema + "_" + version } From 159062d4b14168d6e5ade5216f20ed9080b0e806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:29:35 +0100 Subject: [PATCH 11/25] update docs, add example --- docs/README.md | 11 +++++++---- examples/32_sql_onComplete.json | 11 +++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 examples/32_sql_onComplete.json diff --git a/docs/README.md b/docs/README.md index 8c271bc8..26e09dad 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1068,11 +1068,17 @@ A raw SQL operation runs arbitrary SQL against the database. This is intended as } ``` +By default, `sql` operation cannot run together with other operations in the same migration. This is to ensure pgroll can correctly track the state of the database. However, it is possible to run `sql` operation together with other operations by setting the `onComplete` flag to `true`. + +The `onComplete` flag will make this operation run the `up` expression on the complete phase (instead of the default, which is to run it on the start phase). + +`onComplete` flag is incompatible with `down` expression, as `pgroll` does not support running rollback after complete was executed. + Example **raw SQL** migrations: * [05_sql.json](../examples/05_sql.json) +* [32_sql_onComplete.json](../examples/32_sql_onComplete.json) -By default, `sql` operation cannot run together with other operations in the same migration. This is to ensure pgroll can correctly track the state of the database. However, it is possible to run `sql` operation together with other operations by setting the `onComplete` flag to `true`: ```json { @@ -1083,9 +1089,6 @@ By default, `sql` operation cannot run together with other operations in the sam } ``` -The `onComplete` flag will make this operation run the `up` expression on the complete phase (instead of the default, which is to run it on the start phase). - -`onComplete` flag is incompatible with `down` expression, as `pgroll` does not support running rollback after complete was executed. ### Rename table diff --git a/examples/32_sql_onComplete.json b/examples/32_sql_onComplete.json new file mode 100644 index 00000000..6a39e7b3 --- /dev/null +++ b/examples/32_sql_onComplete.json @@ -0,0 +1,11 @@ +{ + "name": "32_sql_onComplete", + "operations": [ + { + "sql": { + "up": "ALTER TABLE people ADD COLUMN age INTEGER", + "onComplete": true + } + } + ] +} From f3cdf227d7d3555ee2bdb9bd98c92c4de1f7a493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:31:54 +0100 Subject: [PATCH 12/25] update RequiresSchemaRefresh signature --- pkg/migrations/migrations.go | 2 +- pkg/migrations/op_raw_sql.go | 4 +--- pkg/roll/execute.go | 8 +++----- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/pkg/migrations/migrations.go b/pkg/migrations/migrations.go index 949a0894..cb2c11b1 100644 --- a/pkg/migrations/migrations.go +++ b/pkg/migrations/migrations.go @@ -40,7 +40,7 @@ type IsolatedOperation interface { // RequiresSchemaRefreshOperation is an operation that requires the resulting schema to be refreshed type RequiresSchemaRefreshOperation interface { - RequiresSchemaRefresh() bool + RequiresSchemaRefresh() } type ( diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index 87b70a15..c913a28f 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -60,6 +60,4 @@ func (o *OpRawSQL) IsIsolated() bool { } // this operation requires the resulting schema to be refreshed when executed on start -func (o *OpRawSQL) RequiresSchemaRefresh() bool { - return true -} +func (o *OpRawSQL) RequiresSchemaRefresh() {} diff --git a/pkg/roll/execute.go b/pkg/roll/execute.go index 09e28783..03374246 100644 --- a/pkg/roll/execute.go +++ b/pkg/roll/execute.go @@ -52,7 +52,7 @@ func (m *Roll) Start(ctx context.Context, migration *migrations.Migration, cbs . // refresh schema when the op is isolated and requires a refresh (for example raw sql) // we don't want to refresh the schema if the operation is not isolated as it would // override changes made by other operations - if refreshOp, ok := op.(migrations.RequiresSchemaRefreshOperation); ok && refreshOp.RequiresSchemaRefresh() { + if _, ok := op.(migrations.RequiresSchemaRefreshOperation); ok { if isolatedOp, ok := op.(migrations.IsolatedOperation); ok && isolatedOp.IsIsolated() { newSchema, err = m.state.ReadSchema(ctx, m.schema) if err != nil { @@ -127,10 +127,8 @@ func (m *Roll) Complete(ctx context.Context) error { return fmt.Errorf("unable to execute complete operation: %w", err) } - if refreshOp, ok := op.(migrations.RequiresSchemaRefreshOperation); ok { - if refreshOp.RequiresSchemaRefresh() { - refreshViews = true - } + if _, ok := op.(migrations.RequiresSchemaRefreshOperation); ok { + refreshViews = true } } From cc7e3ad017a30c1b82b4dbecf23d36a574ea4cdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:36:49 +0100 Subject: [PATCH 13/25] fix example --- examples/32_sql_onComplete.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/32_sql_onComplete.json b/examples/32_sql_onComplete.json index 6a39e7b3..a9f47a92 100644 --- a/examples/32_sql_onComplete.json +++ b/examples/32_sql_onComplete.json @@ -3,7 +3,7 @@ "operations": [ { "sql": { - "up": "ALTER TABLE people ADD COLUMN age INTEGER", + "up": "ALTER TABLE people ADD COLUMN birth_date timestamp", "onComplete": true } } From b39649176c1de80d0d31ec514f9306d79768cda7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:40:49 +0100 Subject: [PATCH 14/25] review comments on schema & tests --- pkg/migrations/migrations_test.go | 2 +- pkg/migrations/op_raw_sql.go | 15 ++++----------- pkg/migrations/op_raw_sql_test.go | 9 ++------- pkg/migrations/types.go | 2 +- schema.json | 3 ++- 5 files changed, 10 insertions(+), 21 deletions(-) diff --git a/pkg/migrations/migrations_test.go b/pkg/migrations/migrations_test.go index 507eaf81..94499601 100644 --- a/pkg/migrations/migrations_test.go +++ b/pkg/migrations/migrations_test.go @@ -44,7 +44,7 @@ func TestMigrationsIsolatedValid(t *testing.T) { Operations: Operations{ &OpRawSQL{ Up: `foo`, - OnComplete: &[]bool{true}[0], + OnComplete: true, }, &OpCreateTable{Name: "foo"}, }, diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index c913a28f..3ae3ae66 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -12,7 +12,7 @@ import ( var _ Operation = (*OpRawSQL)(nil) func (o *OpRawSQL) Start(ctx context.Context, conn *sql.DB, stateSchema string, s *schema.Schema, cbs ...CallbackFn) error { - if !o.onComplete() { + if !o.OnComplete { _, err := conn.ExecContext(ctx, o.Up) return err } @@ -20,7 +20,7 @@ func (o *OpRawSQL) Start(ctx context.Context, conn *sql.DB, stateSchema string, } func (o *OpRawSQL) Complete(ctx context.Context, conn *sql.DB, s *schema.Schema) error { - if o.onComplete() { + if o.OnComplete { _, err := conn.ExecContext(ctx, o.Up) return err } @@ -40,23 +40,16 @@ func (o *OpRawSQL) Validate(ctx context.Context, s *schema.Schema) error { return EmptyMigrationError{} } - if o.onComplete() && o.Down != "" { + if o.OnComplete && o.Down != "" { return InvalidMigrationError{Reason: "down is not allowed with onComplete"} } return nil } -func (o *OpRawSQL) onComplete() bool { - if o.OnComplete != nil { - return *o.OnComplete - } - return false -} - // this operation is isolated when executed on start, cannot be executed with other operations func (o *OpRawSQL) IsIsolated() bool { - return !o.onComplete() + return !o.OnComplete } // this operation requires the resulting schema to be refreshed when executed on start diff --git a/pkg/migrations/op_raw_sql_test.go b/pkg/migrations/op_raw_sql_test.go index fe851c71..53fc8951 100644 --- a/pkg/migrations/op_raw_sql_test.go +++ b/pkg/migrations/op_raw_sql_test.go @@ -11,7 +11,6 @@ import ( func TestRawSQL(t *testing.T) { t.Parallel() - vTrue := true ExecuteTests(t, TestCases{ { @@ -61,7 +60,7 @@ func TestRawSQL(t *testing.T) { Name: "01_create_table", Operations: migrations.Operations{ &migrations.OpRawSQL{ - OnComplete: &vTrue, + OnComplete: true, Up: ` CREATE TABLE test_table ( id serial, @@ -76,10 +75,6 @@ func TestRawSQL(t *testing.T) { // SQL didn't run yet TableMustNotExist(t, db, schema, "test_table") }, - afterRollback: func(t *testing.T, db *sql.DB, schema string) { - // table is dropped after rollback - TableMustNotExist(t, db, schema, "test_table") - }, afterComplete: func(t *testing.T, db *sql.DB, schema string) { // table can be accessed after start TableMustExist(t, db, schema, "test_table") @@ -104,7 +99,7 @@ func TestRawSQL(t *testing.T) { }, }, &migrations.OpRawSQL{ - OnComplete: &vTrue, + OnComplete: true, Up: ` ALTER TABLE test_table ADD COLUMN age int `, diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 290220a6..cb583208 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -172,7 +172,7 @@ type OpRawSQL struct { Down string `json:"down,omitempty"` // SQL expression will run on complete step (rather than on start) - OnComplete *bool `json:"onComplete,omitempty"` + OnComplete bool `json:"onComplete,omitempty"` // SQL expression for up migration Up string `json:"up"` diff --git a/schema.json b/schema.json index e954014d..1fb1150d 100644 --- a/schema.json +++ b/schema.json @@ -315,7 +315,8 @@ }, "onComplete": { "description": "SQL expression will run on complete step (rather than on start)", - "type": "boolean" + "type": "boolean", + "default": false } }, "required": ["up"], From 047649517aad6b1569dfd2867d3e3a4f524c529c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:42:38 +0100 Subject: [PATCH 15/25] split tests --- pkg/migrations/migrations_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/migrations/migrations_test.go b/pkg/migrations/migrations_test.go index 94499601..a1461352 100644 --- a/pkg/migrations/migrations_test.go +++ b/pkg/migrations/migrations_test.go @@ -37,9 +37,10 @@ func TestMigrationsIsolatedValid(t *testing.T) { } err := migration.Validate(context.TODO(), schema.New()) assert.NoError(t, err) +} - // Test onComplete - migration = Migration{ +func TestOnCompleteSQLMigrationsAreNotIsolated(t *testing.T) { + migration := Migration{ Name: "sql", Operations: Operations{ &OpRawSQL{ @@ -49,6 +50,6 @@ func TestMigrationsIsolatedValid(t *testing.T) { &OpCreateTable{Name: "foo"}, }, } - err = migration.Validate(context.TODO(), schema.New()) + err := migration.Validate(context.TODO(), schema.New()) assert.NoError(t, err) } From bb0dd6ae9f3a930cf588e1443b32daba68b246ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:45:52 +0100 Subject: [PATCH 16/25] update schema.json --- schema.json | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/schema.json b/schema.json index 1fb1150d..b9701607 100644 --- a/schema.json +++ b/schema.json @@ -320,6 +320,20 @@ } }, "required": ["up"], + "oneOf": [ + { + "required": ["up", "down"], + "not": { + "required": ["onComplete"] + } + }, + { + "required": ["up", "onComplete"], + "not": { + "required": ["down"] + } + } + ], "type": "object" }, "OpRenameTable": { From bd835607ba1d393295d799199795ba4c9f6e3653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:46:35 +0100 Subject: [PATCH 17/25] move comments around --- pkg/migrations/migrations.go | 2 ++ pkg/migrations/op_raw_sql.go | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/migrations/migrations.go b/pkg/migrations/migrations.go index cb2c11b1..7182478c 100644 --- a/pkg/migrations/migrations.go +++ b/pkg/migrations/migrations.go @@ -35,11 +35,13 @@ type Operation interface { // IsolatedOperation is an operation that cannot be executed with other operations // in the same migration type IsolatedOperation interface { + // this operation is isolated when executed on start, cannot be executed with other operations IsIsolated() bool } // RequiresSchemaRefreshOperation is an operation that requires the resulting schema to be refreshed type RequiresSchemaRefreshOperation interface { + // this operation requires the resulting schema to be refreshed when executed on start RequiresSchemaRefresh() } diff --git a/pkg/migrations/op_raw_sql.go b/pkg/migrations/op_raw_sql.go index 3ae3ae66..1719ed9b 100644 --- a/pkg/migrations/op_raw_sql.go +++ b/pkg/migrations/op_raw_sql.go @@ -47,10 +47,8 @@ func (o *OpRawSQL) Validate(ctx context.Context, s *schema.Schema) error { return nil } -// this operation is isolated when executed on start, cannot be executed with other operations func (o *OpRawSQL) IsIsolated() bool { return !o.OnComplete } -// this operation requires the resulting schema to be refreshed when executed on start func (o *OpRawSQL) RequiresSchemaRefresh() {} From a665d69ab9dc6fbbe0dfea58f15cc39113dcd9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 12:50:24 +0100 Subject: [PATCH 18/25] fix schema --- schema.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/schema.json b/schema.json index b9701607..f775a338 100644 --- a/schema.json +++ b/schema.json @@ -321,6 +321,11 @@ }, "required": ["up"], "oneOf": [ + { + "not": { + "required": ["down", "onComplete"] + } + }, { "required": ["up", "down"], "not": { From a56a6a594d78ef7f2749ae08c42e572875ec3bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 13:04:43 +0100 Subject: [PATCH 19/25] fix schema.json + add tests --- pkg/jsonschema/testdata/sql-3.txtar | 18 ++++++++++++++++++ pkg/jsonschema/testdata/sql-4.txtar | 19 +++++++++++++++++++ schema.json | 19 ++++++++++--------- 3 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 pkg/jsonschema/testdata/sql-3.txtar create mode 100644 pkg/jsonschema/testdata/sql-4.txtar diff --git a/pkg/jsonschema/testdata/sql-3.txtar b/pkg/jsonschema/testdata/sql-3.txtar new file mode 100644 index 00000000..6c1d6246 --- /dev/null +++ b/pkg/jsonschema/testdata/sql-3.txtar @@ -0,0 +1,18 @@ +This is a valid 'sql' migration. +It specifies `up`, `down` and `on_complete + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "sql": { + "up": "CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT)", + "onComplete": true + } + } + ] +} + +-- valid -- +true diff --git a/pkg/jsonschema/testdata/sql-4.txtar b/pkg/jsonschema/testdata/sql-4.txtar new file mode 100644 index 00000000..44e88de4 --- /dev/null +++ b/pkg/jsonschema/testdata/sql-4.txtar @@ -0,0 +1,19 @@ +This is a valid 'sql' migration. +It specifies `up`, `down` and `on_complete + +-- create_table.json -- +{ + "name": "migration_name", + "operations": [ + { + "sql": { + "up": "CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT)", + "down": "DROP TABLE users", + "onComplete": true + } + } + ] +} + +-- valid -- +false diff --git a/schema.json b/schema.json index f775a338..91f60bbd 100644 --- a/schema.json +++ b/schema.json @@ -322,20 +322,21 @@ "required": ["up"], "oneOf": [ { - "not": { - "required": ["down", "onComplete"] - } + "required": ["down"] }, { - "required": ["up", "down"], - "not": { - "required": ["onComplete"] - } + "required": ["onComplete"] }, { - "required": ["up", "onComplete"], "not": { - "required": ["down"] + "anyOf": [ + { + "required": ["down"] + }, + { + "required": ["onComplete"] + } + ] } } ], From d7f69ba1bce550509f6523920468c27bfcc0a5d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 16:29:49 +0100 Subject: [PATCH 20/25] Update pkg/jsonschema/testdata/sql-3.txtar Co-authored-by: Andrew Farries --- pkg/jsonschema/testdata/sql-3.txtar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/jsonschema/testdata/sql-3.txtar b/pkg/jsonschema/testdata/sql-3.txtar index 6c1d6246..c9b60eeb 100644 --- a/pkg/jsonschema/testdata/sql-3.txtar +++ b/pkg/jsonschema/testdata/sql-3.txtar @@ -1,5 +1,5 @@ This is a valid 'sql' migration. -It specifies `up`, `down` and `on_complete +It specifies `up`, and `on_complete` -- create_table.json -- { From d5dc48e2be5923562db4e977e414ecc3e5bb7434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 16:29:55 +0100 Subject: [PATCH 21/25] Update pkg/jsonschema/testdata/sql-4.txtar Co-authored-by: Andrew Farries --- pkg/jsonschema/testdata/sql-4.txtar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/jsonschema/testdata/sql-4.txtar b/pkg/jsonschema/testdata/sql-4.txtar index 44e88de4..8e9a0c48 100644 --- a/pkg/jsonschema/testdata/sql-4.txtar +++ b/pkg/jsonschema/testdata/sql-4.txtar @@ -1,5 +1,5 @@ This is a valid 'sql' migration. -It specifies `up`, `down` and `on_complete +It specifies `up`, `down` and `on_complete` -- create_table.json -- { From 175610f01685ec5c97d0b4ed1588f61a537bbb16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 16:30:13 +0100 Subject: [PATCH 22/25] Update docs/README.md Co-authored-by: Andrew Farries --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index 26e09dad..58c322ac 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1068,7 +1068,7 @@ A raw SQL operation runs arbitrary SQL against the database. This is intended as } ``` -By default, `sql` operation cannot run together with other operations in the same migration. This is to ensure pgroll can correctly track the state of the database. However, it is possible to run `sql` operation together with other operations by setting the `onComplete` flag to `true`. +By default, a `sql` operation cannot run together with other operations in the same migration. This is to ensure pgroll can correctly track the state of the database. However, it is possible to run a `sql` operation together with other operations by setting the `onComplete` flag to `true`. The `onComplete` flag will make this operation run the `up` expression on the complete phase (instead of the default, which is to run it on the start phase). From 8e1adc8a08fc938798dddc986f85c94f10ca8939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 16:31:20 +0100 Subject: [PATCH 23/25] Update pkg/jsonschema/testdata/sql-4.txtar Co-authored-by: Andrew Farries --- pkg/jsonschema/testdata/sql-4.txtar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/jsonschema/testdata/sql-4.txtar b/pkg/jsonschema/testdata/sql-4.txtar index 8e9a0c48..34a8732f 100644 --- a/pkg/jsonschema/testdata/sql-4.txtar +++ b/pkg/jsonschema/testdata/sql-4.txtar @@ -1,4 +1,4 @@ -This is a valid 'sql' migration. +This is an invalid 'sql' migration. It specifies `up`, `down` and `on_complete` -- create_table.json -- From 5cd080e863efb4da3db5242056d83b9557da7040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 16:31:29 +0100 Subject: [PATCH 24/25] rename example --- docs/README.md | 2 +- examples/{32_sql_onComplete.json => 32_sql_on_complete.json} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename examples/{32_sql_onComplete.json => 32_sql_on_complete.json} (82%) diff --git a/docs/README.md b/docs/README.md index 58c322ac..55b8d680 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1077,7 +1077,7 @@ The `onComplete` flag will make this operation run the `up` expression on the co Example **raw SQL** migrations: * [05_sql.json](../examples/05_sql.json) -* [32_sql_onComplete.json](../examples/32_sql_onComplete.json) +* [32_sql_on_complete.json](../examples/32_sql_on_complete.json) ```json diff --git a/examples/32_sql_onComplete.json b/examples/32_sql_on_complete.json similarity index 82% rename from examples/32_sql_onComplete.json rename to examples/32_sql_on_complete.json index a9f47a92..fb152af5 100644 --- a/examples/32_sql_onComplete.json +++ b/examples/32_sql_on_complete.json @@ -1,5 +1,5 @@ { - "name": "32_sql_onComplete", + "name": "32_sql_on_complete", "operations": [ { "sql": { From c622a73df7604f7eb91dbe14a49b70623911f134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez-Aradros=20Herce?= Date: Mon, 12 Feb 2024 16:32:17 +0100 Subject: [PATCH 25/25] move example block --- docs/README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/README.md b/docs/README.md index 55b8d680..89abb760 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1074,10 +1074,7 @@ The `onComplete` flag will make this operation run the `up` expression on the co `onComplete` flag is incompatible with `down` expression, as `pgroll` does not support running rollback after complete was executed. -Example **raw SQL** migrations: -* [05_sql.json](../examples/05_sql.json) -* [32_sql_on_complete.json](../examples/32_sql_on_complete.json) ```json @@ -1089,6 +1086,11 @@ Example **raw SQL** migrations: } ``` +Example **raw SQL** migrations: + +* [05_sql.json](../examples/05_sql.json) +* [32_sql_on_complete.json](../examples/32_sql_on_complete.json) + ### Rename table