Merge branch 'stripe:main' into lmcrean/212

Author: lmcrean

.github/ISSUE_TEMPLATE/bug_report.md

@@ -8,24 +8,24 @@ assignees: ''
 ---
 
 ### Describe the bug**
-[//]: # A clear and concise description of what the bug is.
+<!-- A clear and concise description of what the bug is. -->
 
 ### Expected behavior**
-[//]: # A clear and concise description of what you expected to happen.
+<!-- A clear and concise description of what you expected to happen. -->
 
 ### To Reproduce
-[//]: # Steps to reproduce the behavior:
-[//]: # 1. Go to '...'
-[//]: # 2. Click on '....'
-[//]: # 3. Scroll down to '....'
-[//]: # 4. See error
+<!-- Steps to reproduce the behavior:
+1. Go to '...'
+2. Click on '....'
+3. Scroll down to '....'
+4. See error -->
 
 ### Context 
-[//]: # Fetch from "pg-schema-diff version" if using the CLI or go.mod if using the library
+<!-- Fetch from "pg-schema-diff version" if using the CLI or go.mod if using the library -->
 pg-schema-diff version: Some sha or version
-[//]: # If you are using the CLI or using the library programatically
+<!-- If you are using the CLI or using the library programatically -->
 pg-schema-diff usage: [CLI | LIBRARY]
 Postgres version: [14, 15, 16, 17]
-[//]: # Only if relevant
+<!-- Only if relevant -->
 pg_dump of database: Gist link
 

.github/ISSUE_TEMPLATE/feature_request.md

@@ -8,7 +8,7 @@ assignees: ''
 ---
 
 ### Describe the feature
-[//]: # Provide a thorough description of the feature. If possible, include examples of  the feature and its behavior. For example, if it's a new command flag, provide examples of using the command flag and expected outputs.
+<!-- Provide a thorough description of the feature. If possible, include examples of  the feature and its behavior. For example, if it's a new command flag, provide examples of using the command flag and expected outputs. -->
 
 ### Motivation
-[//]: # Describe why you want this feature. This is important for assessing the priority of the feature and if alternative solutions exist.
+<!-- Describe why you want this feature. This is important for assessing the priority of the feature and if alternative solutions exist. -->

.github/pull_request_template.md

@@ -1,10 +1,10 @@
-[//]: # README: Ensure you've read the CONTRIBUTING.MD and that your commits are signed
+<!-- README: Ensure you've read the CONTRIBUTING.MD and that your commits are signed -->
 
 ### Description
-[//]: # A clear and concise description of the purpose of this Pull Request. What is being changed? Include any relevant background for this change.
+<!-- A clear and concise description of the purpose of this Pull Request. What is being changed? Include any relevant background for this change. -->
 
 ### Motivation
-[//]: # Why you made these changes. Link to any relevant issues.
+<!-- Why you made these changes. Link to any relevant issues. -->
 
 ### Testing
-[//]: # Describe how you tested these changes
+<!-- Describe how you tested these changes -->

cmd/pg-schema-diff/apply_cmd_test.go

@@ -78,9 +78,9 @@ func (suite *cmdTestSuite) TestApplyCmd() {
 			})
 			// The migration should have been successful. Assert it was.
 			expectedDb := tempDbWithSchema(suite.T(), suite.pgEngine, tc.expectedSchemaDDL)
-			expectedDbDump, err := pgdump.GetDump(expectedDb, pgdump.WithSchemaOnly())
+			expectedDbDump, err := pgdump.GetDump(expectedDb, pgdump.WithSchemaOnly(), pgdump.WithRestrictKey(pgdump.FixedRestrictKey))
 			suite.Require().NoError(err)
-			fromDbDump, err := pgdump.GetDump(fromDb, pgdump.WithSchemaOnly())
+			fromDbDump, err := pgdump.GetDump(fromDb, pgdump.WithSchemaOnly(), pgdump.WithRestrictKey(pgdump.FixedRestrictKey))
 			suite.Require().NoError(err)
 
 			suite.Equal(expectedDbDump, fromDbDump)

cmd/pg-schema-diff/main_test.go

@@ -34,6 +34,8 @@ type runCmdWithAssertionsParams struct {
 	// saving schemas to a randomly generated temporary directory.
 	dynamicArgs []dArgGenerator
 
+	// outputEquals is the exact string that stdout should equal.
+	outputEquals string
 	// outputContains is a list of substrings that are expected to be contained in the stdout output of the command.
 	outputContains []string
 	// expectErrContains is a list of substrings that are expected to be contained in the error returned by
@@ -72,6 +74,9 @@ func (suite *cmdTestSuite) runCmdWithAssertions(tc runCmdWithAssertionsParams) {
 			suite.Contains(stdOutStr, o)
 		}
 	}
+	if len(tc.outputEquals) > 0 {
+		suite.Equal(tc.outputEquals, stdOutStr)
+	}
 }
 
 // dArgGenerator generates argument at the run-time of the test case...

cmd/pg-schema-diff/plan_cmd.go

@@ -42,11 +42,11 @@ func buildPlanCmd() *cobra.Command {
 	toSchemaFlags := createSchemaSourceFlags(cmd, "to-")
 	tempDbConnFlags := createConnectionFlags(cmd, "temp-db-", "The temporary database to use for schema extraction. This is optional if diffing to/from a Postgres instance")
 	planOptsFlags := createPlanOptionsFlags(cmd)
-	outputFmt := outputFormatPretty
+	outputFmt := outputFormatSql
 	cmd.Flags().Var(
 		&outputFmt,
 		"output-format",
-		fmt.Sprintf("Change the output format for what is printed. Defaults to pretty-printed human-readable output. (options: %s)", strings.Join(outputFormatStrings(), ", ")),
+		fmt.Sprintf("Change the output format for what is printed. Defaults to %v. (options: %s)", outputFmt.identifier, strings.Join(outputFormatStrings(), ", ")),
 	)
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		logger := log.SimpleLogger()
@@ -162,17 +162,23 @@ type (
 )
 
 var (
-	outputFormatPretty = outputFormat{
-		identifier:            "pretty",
-		convertToOutputString: planToPrettyS,
-	}
-
 	outputFormatJson = outputFormat{
 		identifier:            "json",
 		convertToOutputString: planToJsonS,
 	}
 
+	outputFormatSql = outputFormat{
+		identifier:            "sql",
+		convertToOutputString: planToSql,
+	}
+
+	outputFormatPretty = outputFormat{
+		identifier:            "pretty",
+		convertToOutputString: planToPrettyS,
+	}
+
 	outputFormats = []outputFormat{
+		outputFormatSql,
 		outputFormatPretty,
 		outputFormatJson,
 	}
@@ -584,10 +590,33 @@ func hazardToPrettyS(hazard diff.MigrationHazard) string {
 	}
 }
 
+// planToJsonS converts the plan to JSON.
 func planToJsonS(plan diff.Plan) string {
 	jsonData, err := json.MarshalIndent(plan, "", "  ")
 	if err != nil {
 		panic(err)
 	}
 	return string(jsonData)
 }
+
+// planToSql converts the plan to one large runnable SQL script.
+func planToSql(plan diff.Plan) string {
+	sb := strings.Builder{}
+	for i, stmt := range plan.Statements {
+		sb.WriteString("/*\n")
+		sb.WriteString(fmt.Sprintf("Statement %d\n", i))
+		if len(stmt.Hazards) > 0 {
+			for _, hazard := range stmt.Hazards {
+				sb.WriteString(fmt.Sprintf("  - %s\n", hazardToPrettyS(hazard)))
+			}
+		}
+		sb.WriteString("*/\n")
+		sb.WriteString(fmt.Sprintf("SET SESSION statement_timeout = %d;\n", stmt.Timeout.Milliseconds()))
+		sb.WriteString(fmt.Sprintf("SET SESSION lock_timeout = %d;\n", stmt.LockTimeout.Milliseconds()))
+		sb.WriteString(fmt.Sprintf("%s;", stmt.DDL))
+		if i < len(plan.Statements)-1 {
+			sb.WriteString("\n\n")
+		}
+	}
+	return sb.String()
+}

cmd/pg-schema-diff/plan_cmd_test.go

@@ -12,6 +12,8 @@ func (suite *cmdTestSuite) TestPlanCmd() {
 		args        []string
 		dynamicArgs []dArgGenerator
 
+		// outputEquals is the exact string that stdout should equal.
+		outputEquals string
 		// outputContains is a list of substrings that are expected to be contained in the stdout output of the command.
 		outputContains []string
 		// expectErrContains is a list of substrings that are expected to be contained in the error returned by
@@ -91,11 +93,43 @@ func (suite *cmdTestSuite) TestPlanCmd() {
 			args:              []string{"--from-dir", "some-dir", "--to-dir", "some-other-dir"},
 			expectErrContains: []string{"at least one Postgres server"},
 		},
+		{
+			name: "sql output format",
+			args: []string{"--output-format", "sql"},
+			dynamicArgs: []dArgGenerator{
+				tempDsnDArg(suite.pgEngine, "temp-db-dsn", []string{""}),
+				tempSchemaDirDArg("from-dir", []string{`
+						CREATE TABLE foobar(
+							bar TEXT,
+							fizzbuzz TEXT
+						);
+				`}),
+				tempSchemaDirDArg("to-dir", []string{`
+						CREATE TABLE foobar(
+							bar TEXT,
+							fizzbuzz TEXT
+						);
+					CREATE INDEX bar_idx ON foobar(bar);
+					CREATE INDEX fizzbuzz_idx ON foobar(fizzbuzz);
+				`}),
+			},
+			outputEquals: "/*\nStatement 0\n  - INDEX_BUILD: This might affect database performance. Concurrent index builds require a non-trivial amount of CPU, potentially affecting database performance. They also can take a while but do not lock out writes.\n*/\nSET SESSION statement_timeout = 1200000;\nSET SESSION lock_timeout = 3000;\nCREATE INDEX CONCURRENTLY bar_idx ON public.foobar USING btree (bar);\n\n/*\nStatement 1\n  - INDEX_BUILD: This might affect database performance. Concurrent index builds require a non-trivial amount of CPU, potentially affecting database performance. They also can take a while but do not lock out writes.\n*/\nSET SESSION statement_timeout = 1200000;\nSET SESSION lock_timeout = 3000;\nCREATE INDEX CONCURRENTLY fizzbuzz_idx ON public.foobar USING btree (fizzbuzz);\n",
+		},
+		{
+			name: "invalid output format",
+			args: []string{"--output-format", "invalid"},
+			dynamicArgs: []dArgGenerator{
+				tempDsnDArg(suite.pgEngine, "from-dsn", nil),
+				tempDsnDArg(suite.pgEngine, "to-dsn", []string{"CREATE TABLE foobar()"}),
+			},
+			expectErrContains: []string{"invalid output format"},
+		},
 	} {
 		suite.Run(tc.name, func() {
 			suite.runCmdWithAssertions(runCmdWithAssertionsParams{
 				args:              append([]string{"plan"}, tc.args...),
 				dynamicArgs:       tc.dynamicArgs,
+				outputEquals:      tc.outputEquals,
 				outputContains:    tc.outputContains,
 				expectErrContains: tc.expectErrContains,
 			})

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/go-logfmt/logfmt v0.6.0
 	github.com/google/go-cmp v0.5.9
 	github.com/google/uuid v1.3.0
+	github.com/hashicorp/go-version v1.7.0
 	github.com/jackc/pgx/v4 v4.18.2
 	github.com/kr/pretty v0.3.1
 	github.com/lib/pq v1.10.2

go.sum

@@ -29,6 +29,8 @@ github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKeRZfjY=
+github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
 github.com/jackc/chunkreader v1.0.0/go.mod h1:RT6O25fNZIuasFJRyZ4R/Y2BbhasbmZXF9QQ7T3kePo=

internal/migration_acceptance_tests/acceptance_test.go

@@ -183,7 +183,7 @@ func runTest(t *testing.T, tc acceptanceTestCase) {
 
 	// Make sure the pgdump after running the migration is the same as the
 	// pgdump from a database where we directly run the newSchemaDDL
-	oldDbDump, err := pgdump.GetDump(oldDb, pgdump.WithSchemaOnly())
+	oldDbDump, err := pgdump.GetDump(oldDb, pgdump.WithSchemaOnly(), pgdump.WithRestrictKey(pgdump.FixedRestrictKey))
 	require.NoError(t, err)
 
 	newDbDump := directlyRunDDLAndGetDump(t, engine, tc.expectedDBSchemaDDL)
@@ -221,7 +221,7 @@ func directlyRunDDLAndGetDump(t *testing.T, engine *pgengine.Engine, ddl []strin
 	defer newDb.DropDB()
 	require.NoError(t, applyDDL(newDb, ddl))
 
-	newDbDump, err := pgdump.GetDump(newDb, pgdump.WithSchemaOnly())
+	newDbDump, err := pgdump.GetDump(newDb, pgdump.WithSchemaOnly(), pgdump.WithRestrictKey(pgdump.FixedRestrictKey))
 	require.NoError(t, err)
 	return newDbDump
 }