SQL format default (#239)

Author: bplunkett-stripe

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,11 +162,6 @@ type (
 )
 
 var (
-	outputFormatPretty = outputFormat{
-		identifier:            "pretty",
-		convertToOutputString: planToPrettyS,
-	}
-
 	outputFormatJson = outputFormat{
 		identifier:            "json",
 		convertToOutputString: planToJsonS,
@@ -177,10 +172,15 @@ var (
 		convertToOutputString: planToSql,
 	}
 
+	outputFormatPretty = outputFormat{
+		identifier:            "pretty",
+		convertToOutputString: planToPrettyS,
+	}
+
 	outputFormats = []outputFormat{
+		outputFormatSql,
 		outputFormatPretty,
 		outputFormatJson,
-		outputFormatSql,
 	}
 
 	outputFormatStrings = func() []string {
@@ -603,11 +603,14 @@ func planToJsonS(plan diff.Plan) string {
 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("-- Hazard %s\n", hazardToPrettyS(hazard)))
+				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))

cmd/pg-schema-diff/plan_cmd_test.go

@@ -113,7 +113,7 @@ func (suite *cmdTestSuite) TestPlanCmd() {
 					CREATE INDEX fizzbuzz_idx ON foobar(fizzbuzz);
 				`}),
 			},
-			outputEquals: "-- 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.\nSET SESSION statement_timeout = 1200000;\nSET SESSION lock_timeout = 3000;\nCREATE INDEX CONCURRENTLY bar_idx ON public.foobar USING btree (bar);\n\n-- 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.\nSET SESSION statement_timeout = 1200000;\nSET SESSION lock_timeout = 3000;\nCREATE INDEX CONCURRENTLY fizzbuzz_idx ON public.foobar USING btree (fizzbuzz);\n",
+			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",

internal/migration_acceptance_tests/backwards_compat_cases_test.go

@@ -15,7 +15,6 @@ import (
 	externalschema "github.com/stripe/pg-schema-diff/pkg/schema"
 
 	"github.com/stripe/pg-schema-diff/pkg/log"
-	"github.com/stripe/pg-schema-diff/pkg/sqldb"
 	"github.com/stripe/pg-schema-diff/pkg/tempdb"
 )
 
@@ -103,24 +102,6 @@ func WithRandReader(randReader io.Reader) PlanOpt {
 	}
 }
 
-// deprecated: GeneratePlan generates a migration plan to migrate the database to the target schema. This function only
-// diffs the public schemas.
-//
-// Use Generate instead with the DDLSchemaSource(newDDL) and WithIncludeSchemas("public") and WithTempDbFactory options.
-//
-// Parameters:
-// queryable: 	The target database to generate the diff for. It is recommended to pass in *sql.DB of the db you
-// wish to migrate. If using a connection pool, it is RECOMMENDED to set a maximum number of connections.
-// tempDbFactory:  	used to create a temporary database instance to extract the schema from the new DDL and validate the
-// migration plan. It is recommended to use tempdb.NewOnInstanceFactory, or you can provide your own.
-// newDDL:  		DDL encoding the new schema
-// opts:  			Additional options to configure the plan generation
-func GeneratePlan(ctx context.Context, queryable sqldb.Queryable, tempdbFactory tempdb.Factory, newDDL []string, opts ...PlanOpt) (Plan, error) {
-	schemaSource := DBSchemaSource(queryable)
-
-	return Generate(ctx, schemaSource, DDLSchemaSource(newDDL), append(opts, WithTempDbFactory(tempdbFactory), WithIncludeSchemas("public"))...)
-}
-
 // Generate generates a migration plan to migrate the database to the target schema
 //
 // Parameters: