Add option to disable concurrent index build/drops (#242)

* Add --disable-concurrent-index-ops command-line flag

---------

Co-authored-by: geeknonerd <[email protected]>

Author: bplunkett-stripe

cmd/pg-schema-diff/plan_cmd.go

@@ -114,6 +114,7 @@ type (
 
 		dataPackNewTables     bool
 		disablePlanValidation bool
+		noConcurrentIndexOps  bool
 
 		statementTimeoutModifiers []string
 		lockTimeoutModifiers      []string
@@ -222,6 +223,8 @@ func createPlanOptionsFlags(cmd *cobra.Command) *planOptionsFlags {
 	cmd.Flags().BoolVar(&flags.dataPackNewTables, "data-pack-new-tables", true, "If set, will data pack new tables in the plan to minimize table size (re-arranges columns).")
 	cmd.Flags().BoolVar(&flags.disablePlanValidation, "disable-plan-validation", false, "If set, will disable plan validation. Plan validation runs the migration against a temporary"+
 		"database with an identical schema to the original, asserting that the generated plan actually migrates the schema to the desired target.")
+	cmd.Flags().BoolVar(&flags.noConcurrentIndexOps, "no-concurrent-index-ops", false, "If set, will disable the use of CONCURRENTLY in CREATE INDEX and DROP INDEX statements. "+
+		"This may result in longer lock times and potential downtime during migrations.")
 
 	timeoutModifierFlagVar(cmd, &flags.statementTimeoutModifiers, "statement", "t")
 	timeoutModifierFlagVar(cmd, &flags.lockTimeoutModifiers, "lock", "l")
@@ -321,6 +324,9 @@ func parsePlanOptions(p planOptionsFlags) (planOptions, error) {
 	if p.disablePlanValidation {
 		opts = append(opts, diff.WithDoNotValidatePlan())
 	}
+	if p.noConcurrentIndexOps {
+		opts = append(opts, diff.WithNoConcurrentIndexOps())
+	}
 
 	var statementTimeoutModifiers []timeoutModifier
 	for _, s := range p.statementTimeoutModifiers {

internal/migration_acceptance_tests/index_no_concurrent_cases_test.go

@@ -0,0 +1,63 @@
+package migration_acceptance_tests
+
+import (
+	"testing"
+
+	"github.com/stripe/pg-schema-diff/pkg/diff"
+)
+
+var indexNoConcurrentAcceptanceTestCases = []acceptanceTestCase{
+	{
+		name: "Add",
+		oldSchemaDDL: []string{
+			`
+            CREATE TABLE "Foobar"(
+                id INT PRIMARY KEY,
+                "Foo" VARCHAR(255)
+            );
+			`,
+		},
+		newSchemaDDL: []string{
+			`
+            CREATE TABLE "Foobar"(
+                id INT PRIMARY KEY,
+                "Foo" VARCHAR(255)
+            );
+            CREATE INDEX "Some_idx" ON "Foobar"(id, "Foo");
+			`,
+		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresShareLock,
+		},
+		planOpts: []diff.PlanOpt{diff.WithNoConcurrentIndexOps()},
+	},
+	{
+		name: "Delete",
+		oldSchemaDDL: []string{
+			`
+            CREATE TABLE "Foobar"(
+                id INT PRIMARY KEY,
+                "Foo" VARCHAR(255)
+            );
+            CREATE INDEX "Some_idx" ON "Foobar"(id, "Foo");
+			`,
+		},
+		newSchemaDDL: []string{
+			`
+            CREATE TABLE "Foobar"(
+                id INT PRIMARY KEY,
+                "Foo" VARCHAR(255) NOT NULL
+            );
+			`,
+		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+			diff.MigrationHazardTypeIndexDropped,
+		},
+		planOpts: []diff.PlanOpt{diff.WithNoConcurrentIndexOps()},
+	},
+}
+
+func TestIndexNoConcurrentTestCases(t *testing.T) {
+	runTestCases(t, indexNoConcurrentAcceptanceTestCases)
+}

internal/pgengine/engine.go

@@ -64,6 +64,7 @@ const (
 var (
 	defaultServerConfiguration = map[string]string{
 		"log_checkpoints": "false",
+		"max_connections": "1000",
 	}
 )
 

pkg/diff/plan_generator.go

@@ -35,6 +35,7 @@ type (
 		validatePlan            bool
 		getSchemaOpts           []schema.GetSchemaOpt
 		randReader              io.Reader
+		noConcurrentIndexOps    bool
 	}
 
 	PlanOpt func(opts *planOptions)
@@ -102,6 +103,16 @@ func WithRandReader(randReader io.Reader) PlanOpt {
 	}
 }
 
+// WithNoConcurrentIndexOps disables the use of CONCURRENTLY in CREATE INDEX and DROP INDEX statements.
+// This can be useful when you need simpler DDL statements or when working in environments that don't support
+// concurrent index operations. Note that disabling concurrent operations may result in longer lock times
+// and potential downtime during migrations.
+func WithNoConcurrentIndexOps() PlanOpt {
+	return func(opts *planOptions) {
+		opts.noConcurrentIndexOps = true
+	}
+}
+
 // Generate generates a migration plan to migrate the database to the target schema
 //
 // Parameters:
@@ -187,7 +198,7 @@ func generateMigrationStatements(oldSchema, newSchema schema.Schema, planOptions
 		diff = removeChangesToColumnOrdering(diff)
 	}
 
-	statements, err := newSchemaSQLGenerator(planOptions.randReader).Alter(diff)
+	statements, err := newSchemaSQLGenerator(planOptions.randReader, planOptions).Alter(diff)
 	if err != nil {
 		return nil, fmt.Errorf("generating migration statements: %w", err)
 	}

pkg/diff/schema_migration_plan_test.go

@@ -370,7 +370,7 @@ func TestSchemaMigrationPlanTest(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 			}
-			stmts, err := newSchemaSQLGenerator(randReader).Alter(schemaDiff)
+			stmts, err := newSchemaSQLGenerator(randReader, &planOptions{}).Alter(schemaDiff)
 			require.NoError(t, err)
 			assert.Equal(t, testCase.expectedStatements, stmts, "actual:\n %# v", pretty.Formatter(stmts))
 		})