Merge branch 'main' into bplunkett/no-concurrent-index

Author: bplunkett-stripe

.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 -->

.sqlfluff

@@ -1,6 +1,7 @@
 [sqlfluff]
 dialect = postgres
 templater = placeholder
+large_file_skip_byte_limit = 0
 
 [sqlfluff:templater:placeholder]
 param_regex = sqlc.arg\(.*\)

README.md

@@ -29,19 +29,28 @@ index cc3a14b..cf4b32d 100644
 The generated plan (*queries using `message_idx` will always have an index backing them, even while the new index is being built*):
 ```
 $ pg-schema-diff plan --from-dsn "postgres://postgres:postgres@localhost:5432/postgres" --to-dir ./schema
-################################ Generated plan ################################
-1. ALTER INDEX "message_idx" RENAME TO "pgschemadiff_tmpidx_message_idx_IiaKzkvPQtyA7ob9piVqiQ";
-        -- Statement Timeout: 3s
-
-2. CREATE INDEX CONCURRENTLY message_idx ON public.foobar USING btree (message, created_at);
-        -- Statement Timeout: 20m0s
-        -- Lock Timeout: 3s
-        -- Hazard 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.
-
-3. DROP INDEX CONCURRENTLY "pgschemadiff_tmpidx_message_idx_IiaKzkvPQtyA7ob9piVqiQ";
-        -- Statement Timeout: 20m0s
-        -- Lock Timeout: 3s
-        -- Hazard INDEX_DROPPED: Dropping this index means queries that use this index might perform worse because they will no longer will be able to leverage it.
+/*
+Statement 0
+*/
+SET SESSION statement_timeout = 3000;
+SET SESSION lock_timeout = 3000;
+ALTER INDEX "public"."message_idx" RENAME TO "pgschemadiff_tmpidx_message_idx_cWw18w4cQ_i7MKzfy7ek9g";
+
+/*
+Statement 1
+  - 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.
+*/
+SET SESSION statement_timeout = 1200000;
+SET SESSION lock_timeout = 3000;
+CREATE INDEX CONCURRENTLY message_idx ON public.foobar USING btree (message, created_at);
+
+/*
+Statement 2
+  - INDEX_DROPPED: Dropping this index means queries that use this index might perform worse because they will no longer will be able to leverage it.
+*/
+SET SESSION statement_timeout = 1200000;
+SET SESSION lock_timeout = 3000;
+DROP INDEX CONCURRENTLY "public"."pgschemadiff_tmpidx_message_idx_cWw18w4cQ_i7MKzfy7ek9g";
 ```
 ### Online `NOT NULL` constraint creation
 Your project's diff:
@@ -61,18 +70,33 @@ index cc3a14b..5a1cec2 100644
 The generated plan (*leverages check constraints to eliminate the need for a long-lived access-exclusive lock on the table*):
 ```
 $ pg-schema-diff plan --from-dsn "postgres://postgres:postgres@localhost:5432/postgres" --to-dir ./schema
-################################ Generated plan ################################
-1. ALTER TABLE "public"."foobar" ADD CONSTRAINT "pgschemadiff_tmpnn_BCOxMXqAQwaXlKPCRXoMMg" CHECK("created_at" IS NOT NULL) NOT VALID;
-        -- Statement Timeout: 3s
-
-2. ALTER TABLE "public"."foobar" VALIDATE CONSTRAINT "pgschemadiff_tmpnn_BCOxMXqAQwaXlKPCRXoMMg";
-        -- Statement Timeout: 3s
-
-3. ALTER TABLE "public"."foobar" ALTER COLUMN "created_at" SET NOT NULL;
-        -- Statement Timeout: 3s
-
-4. ALTER TABLE "public"."foobar" DROP CONSTRAINT "pgschemadiff_tmpnn_BCOxMXqAQwaXlKPCRXoMMg";
-        -- Statement Timeout: 3s
+/*
+Statement 0
+*/
+SET SESSION statement_timeout = 3000;
+SET SESSION lock_timeout = 3000;
+ALTER TABLE "public"."foobar" ADD CONSTRAINT "pgschemadiff_tmpnn_GimngG1rRkODhKvgjhGfNA" CHECK("created_at" IS NOT NULL) NOT VALID;
+
+/*
+Statement 1
+*/
+SET SESSION statement_timeout = 3000;
+SET SESSION lock_timeout = 3000;
+ALTER TABLE "public"."foobar" VALIDATE CONSTRAINT "pgschemadiff_tmpnn_GimngG1rRkODhKvgjhGfNA";
+
+/*
+Statement 2
+*/
+SET SESSION statement_timeout = 3000;
+SET SESSION lock_timeout = 3000;
+ALTER TABLE "public"."foobar" ALTER COLUMN "created_at" SET NOT NULL;
+
+/*
+Statement 3
+*/
+SET SESSION statement_timeout = 3000;
+SET SESSION lock_timeout = 3000;
+ALTER TABLE "public"."foobar" DROP CONSTRAINT "pgschemadiff_tmpnn_GimngG1rRkODhKvgjhGfNA";
 ```
 
 # Key features
@@ -90,10 +114,14 @@ $ pg-schema-diff plan --from-dsn "postgres://postgres:postgres@localhost:5432/po
 * Strong support of partitions
 # Install
 ## CLI
+Brew:
+```bash
+brew install pg-schema-diff
+```
+Go toolchain:
 ```bash
 go install github.com/stripe/pg-schema-diff/cmd/pg-schema-diff@latest
 ```
-
 ## Library
 ```bash
 go get -u github.com/stripe/pg-schema-diff@latest
@@ -171,12 +199,11 @@ for _, stmt := range plan.Statements {
 ```
 
 # Supported Postgres versions
-Supported: 14, 15, 16, 17  
+Supported: 14, 15, 16, 17
 Unsupported: <= 13  are not supported. Use at your own risk.
 
 # Unsupported migrations
 An abridged list of unsupported migrations:
-- Views (Planned)
 - Privileges (Planned)
 - Types (Only enums are currently supported)
 - Renaming. The diffing library relies on names to identify the old and new versions of a table, index, etc. If you rename

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/datastructs.go

@@ -3,6 +3,8 @@ package main
 import (
 	"fmt"
 	"sort"
+
+	"github.com/stripe/pg-schema-diff/internal/util"
 )
 
 func mustGetAndDeleteKey(m map[string]string, key string) (string, error) {
@@ -15,10 +17,7 @@ func mustGetAndDeleteKey(m map[string]string, key string) (string, error) {
 }
 
 func keys(m map[string]string) []string {
-	var vals []string
-	for k := range m {
-		vals = append(vals, k)
-	}
+	vals := util.Keys(m)
 	sort.Strings(vals)
 	return vals
 }

cmd/pg-schema-diff/datastructs_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/main_test.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()
@@ -163,17 +163,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,
 	}
@@ -590,10 +596,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()
+}