Skip to content

Conversation

@geeknonerd
Copy link
Contributor

Description

This PR introduces a new --output-format sql option to the pg-schema-diff plan command, allowing users to generate clean, executable SQL output without
formatting headers and numbering. The SQL format produces migration statements that can be directly saved to .sql files and executed against databases.

Motivation

Users frequently need to save migration plans as executable SQL files for manual review, version control, or execution in different environments. The
existing pretty format includes decorative elements like #### headers and 1. numbering that make the output unsuitable for direct database execution.
The new SQL format addresses this by providing clean, semicolon-terminated SQL statements without any formatting artifacts.

Changes

  • Added outputFormatSql variable and planToSqlS function in plan_cmd.go
  • Integrated SQL format into the existing outputFormats array alongside pretty and json
  • The SQL format strips all formatting elements and outputs pure SQL statements with proper semicolon termination
  • Updated help documentation to include the new sql option

Output Comparison

Pretty Format (Default)

################################ Generated plan ################################

  1. CREATE TABLE "public"."conversation_pin" (
    "id" bigint NOT NULL GENERATED BY DEFAULT AS IDENTITY,
    "user_id" bigint NOT NULL,
    "conversation_id" bigint NOT NULL
    );

SQL Format (New)

CREATE TABLE "public"."conversation_pin" (
"id" bigint NOT NULL GENERATED BY DEFAULT AS IDENTITY,
"user_id" bigint NOT NULL,
"conversation_id" bigint NOT NULL
);

Testing

  • Unit Tests: Added comprehensive tests for planToSqlS function covering empty plans, single statements, multiple statements, and edge cases
  • Integration Tests: Added 4 new test cases in TestPlanCmd to verify SQL output format works across different schema source combinations (DSN-to-DSN,
    DSN-to-Dir, etc.)
  • Input Validation: Added TestOutputFormatValidation to ensure proper error handling for invalid format values
  • Format Verification: Added TestSqlFormatDoesNotContainHeaders to confirm SQL output excludes formatting elements
  • End-to-End Testing: Added TestSqlOutputExecutable to verify generated SQL can be successfully executed against real databases
  • Regression Testing: Confirmed all existing tests pass without issues - no impact on existing pretty and json formats
  • Help Integration: Verified the new sql option appears correctly in command help output

Usage

Generate clean SQL output for direct execution
pg-schema-diff plan
--from-dsn "postgres://user:pass@host/db1"
--to-dsn "postgres://user:pass@host/db2"
--output-format sql > migration.sql

Save to file and execute
pg-schema-diff plan --from-dir ./old --to-dir ./new --output-format sql > migration.sql
psql -f migration.sql

Copy link
Collaborator

@Navbryce Navbryce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks solid! Thank you for this change. A few small comments.

-- Doing this from my cell phone, so excuse any weird formatting in my comments.

var stmtStrs []string
for _, stmt := range plan.Statements {
ddl := strings.TrimSpace(stmt.DDL)
if !strings.HasSuffix(ddl, ";") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not consistent across the outputted SQL? I would prefer to ensure are output is consistent inside the diff package than handling normalization in the CLI.

return string(jsonData)
}

func planToSqlS(plan diff.Plan) string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we materialize the warnings and timeouts in the plan as well?

The warnings could be added as comments. The timeouts could be set via set statement timeout command?

@Navbryce
Copy link
Collaborator

Navbryce commented Aug 5, 2025

Can we update this to actually set the statement timeouts?

@geeknonerd geeknonerd requested a review from Navbryce August 5, 2025 23:54
@geeknonerd
Copy link
Contributor Author

Can we update this to actually set the statement timeouts?

Should be possible

@bplunkett-stripe
Copy link
Collaborator

bplunkett-stripe commented Aug 10, 2025

Can we update this to actually set the statement timeouts?

Should be possible

Can we do it in this PR? I think it would be nice just to have it fully fleshed out! Appreciate the work here!

@bplunkett-stripe
Copy link
Collaborator

Getting this into a mergable state here!. Your code authorship should still appear among contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants