From 8955599bd0b5006478a53ce4c3c465407846fd6c Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Wed, 13 May 2026 18:55:24 +1000 Subject: [PATCH] fix: bump sqlc fork to v1.31.1 merge, strip pg_dump meta-commands (#25105) Closes https://github.com/coder/internal/issues/965 Recent `pg_dump` patch releases (13.22+ / 14.19+ / 15.14+ / 16.10+ / 17.6+) emit `\restrict` / `\unrestrict` psql meta-commands at the head and tail of schema dumps. These broke both `sqlc` and our `scripts/migrate-test` schema-equality check. PR #19696 worked around it by pinning `pg_dump` to a Docker image. This change unpins the workaround now that `sqlc` handles the meta-commands: * Bumps the coder/sqlc fork pin to [`337309b` on coder/sqlc:main](https://github.com/coder/sqlc/commit/337309bfb9524f38466a5090e310040fc7af0203), the merge of upstream v1.31.1 (coder/sqlc#6). v1.31.1 includes [sqlc-dev/sqlc#4390](https://github.com/sqlc-dev/sqlc/pull/4390), the upstream `\restrict` / `\unrestrict` parser fix. Updated in three places that pin the fork SHA: `flake.nix` (`sqlc-custom`), `.github/actions/setup-sqlc/action.yaml`, and the `dogfood/coder/ubuntu-{22,26}.04` Dockerfiles. The flake's `sha256` / `vendorHash` are reset to `pkgs.lib.fakeSha256`; Nix will surface the real hashes on first build, per the existing comment block. * Reverts #19696's Docker pin in `coderd/database/dbtestutil/db.go`. Local `pg_dump` (13+) and the `postgres:13` Docker fallback both work again. * Strips `\restrict` / `\unrestrict` lines in `normalizeDump` so `scripts/migrate-test`'s schema comparison is stable across `pg_dump` versions (the token in those lines is randomized per run). `TestNormalizeDumpStripsRestrict` locks the behavior in. * Regenerates with v1.31.1, picking up the version stamp and one upstream correctness fix in `DeleteLicense` ([sqlc-dev/sqlc#4383](https://github.com/sqlc-dev/sqlc/pull/4383): don't shadow the input parameter when scanning a single-column return). --- .github/actions/setup-sqlc/action.yaml | 2 +- coderd/database/dbtestutil/db.go | 43 ++++++++++--------- .../database/dbtestutil/db_internal_test.go | 32 ++++++++++++++ coderd/database/models.go | 2 +- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 7 +-- dogfood/coder/ubuntu-22.04/Dockerfile | 2 +- dogfood/coder/ubuntu-26.04/Dockerfile | 2 +- flake.nix | 8 ++-- 9 files changed, 67 insertions(+), 33 deletions(-) create mode 100644 coderd/database/dbtestutil/db_internal_test.go diff --git a/.github/actions/setup-sqlc/action.yaml b/.github/actions/setup-sqlc/action.yaml index 10d9fd5239..029a3f5fe4 100644 --- a/.github/actions/setup-sqlc/action.yaml +++ b/.github/actions/setup-sqlc/action.yaml @@ -14,4 +14,4 @@ runs: # - https://github.com/sqlc-dev/sqlc/pull/4159 shell: bash run: | - ./.github/scripts/retry.sh -- env CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@aab4e865a51df0c43e1839f81a9d349b41d14f05 + ./.github/scripts/retry.sh -- env CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@337309bfb9524f38466a5090e310040fc7af0203 diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index 6179b26ead..d25f2508e4 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -10,6 +10,7 @@ import ( "os/exec" "path/filepath" "regexp" + "strconv" "strings" "testing" "time" @@ -240,31 +241,26 @@ func PGDump(dbURL string) ([]byte, error) { return stdout.Bytes(), nil } -const ( - minimumPostgreSQLVersion = 13 - postgresImageSha = "sha256:467e7f2fb97b2f29d616e0be1d02218a7bbdfb94eb3cda7461fd80165edfd1f7" -) +const minimumPostgreSQLVersion = 13 // PGDumpSchemaOnly is for use by gen/dump only. // It runs pg_dump against dbURL and sets a consistent timezone and encoding. func PGDumpSchemaOnly(dbURL string) ([]byte, error) { hasPGDump := false - // TODO: Temporarily pin pg_dump to the docker image until - // https://github.com/sqlc-dev/sqlc/issues/4065 is resolved. - // if _, err := exec.LookPath("pg_dump"); err == nil { - // out, err := exec.Command("pg_dump", "--version").Output() - // if err == nil { - // // Parse output: - // // pg_dump (PostgreSQL) 14.5 (Ubuntu 14.5-0ubuntu0.22.04.1) - // parts := strings.Split(string(out), " ") - // if len(parts) > 2 { - // version, err := strconv.Atoi(strings.Split(parts[2], ".")[0]) - // if err == nil && version >= minimumPostgreSQLVersion { - // hasPGDump = true - // } - // } - // } - // } + if _, err := exec.LookPath("pg_dump"); err == nil { + out, err := exec.Command("pg_dump", "--version").Output() + if err == nil { + // Parse output: + // pg_dump (PostgreSQL) 14.5 (Ubuntu 14.5-0ubuntu0.22.04.1) + parts := strings.Split(string(out), " ") + if len(parts) > 2 { + version, err := strconv.Atoi(strings.Split(parts[2], ".")[0]) + if err == nil && version >= minimumPostgreSQLVersion { + hasPGDump = true + } + } + } + } cmdArgs := []string{ "pg_dump", @@ -289,7 +285,7 @@ func PGDumpSchemaOnly(dbURL string) ([]byte, error) { "run", "--rm", "--network=host", - fmt.Sprintf("%s:%d@%s", postgresImage, minimumPostgreSQLVersion, postgresImageSha), + fmt.Sprintf("%s:%d", postgresImage, minimumPostgreSQLVersion), }, cmdArgs...) } cmd := exec.Command(cmdArgs[0], cmdArgs[1:]...) //#nosec @@ -310,6 +306,11 @@ func PGDumpSchemaOnly(dbURL string) ([]byte, error) { func normalizeDump(schema []byte) []byte { // Remove all comments. schema = regexp.MustCompile(`(?im)^(--.*)$`).ReplaceAll(schema, []byte{}) + // Strip psql meta-commands (\restrict / \unrestrict) emitted by pg_dump + // 13.22+ / 14.19+ / 15.14+ / 16.10+ / 17.6+. The token in these lines is + // randomized per run, so we drop them entirely. See + // https://github.com/coder/internal/issues/965. + schema = regexp.MustCompile(`(?im)^\\(restrict|unrestrict).*$`).ReplaceAll(schema, []byte{}) // Public is implicit in the schema. schema = regexp.MustCompile(`(?im)( |::|'|\()public\.`).ReplaceAll(schema, []byte(`$1`)) // Remove database settings. diff --git a/coderd/database/dbtestutil/db_internal_test.go b/coderd/database/dbtestutil/db_internal_test.go new file mode 100644 index 0000000000..fb4d71b565 --- /dev/null +++ b/coderd/database/dbtestutil/db_internal_test.go @@ -0,0 +1,32 @@ +package dbtestutil + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// Recent pg_dump versions (13.22+ / 14.19+ / 15.14+ / 16.10+ / 17.6+) emit +// psql meta-commands at the head and tail of the dump that aren't valid SQL. +// normalizeDump is expected to strip them so downstream consumers (sqlc, +// schema-equality checks in scripts/migrate-test) don't have to. +// +// See https://github.com/coder/internal/issues/965. +func TestNormalizeDumpStripsRestrict(t *testing.T) { + t.Parallel() + + // Raw string literals (backticks) make backslashes literal, so the + // meta-command here matches what pg_dump actually emits. + input := []byte(`-- header +\restrict XYZ + +CREATE TABLE foo; + +\unrestrict XYZ +`) + + out := string(normalizeDump(input)) + require.NotContains(t, out, `\restrict`, `normalizeDump must strip \restrict psql meta-command`) + require.NotContains(t, out, `\unrestrict`, `normalizeDump must strip \unrestrict psql meta-command`) + require.Contains(t, out, "CREATE TABLE foo;", "normalizeDump must preserve real SQL between the meta-commands") +} diff --git a/coderd/database/models.go b/coderd/database/models.go index 6c90c0499a..2c05309c87 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.30.0 +// sqlc v1.31.1 package database diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 03737e00aa..67cbadc88f 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.30.0 +// sqlc v1.31.1 package database diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8995b8e4ed..1a3dae3838 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.30.0 +// sqlc v1.31.1 package database @@ -12997,8 +12997,9 @@ RETURNING id func (q *sqlQuerier) DeleteLicense(ctx context.Context, id int32) (int32, error) { row := q.db.QueryRowContext(ctx, deleteLicense, id) - err := row.Scan(&id) - return id, err + var id_2 int32 + err := row.Scan(&id_2) + return id_2, err } const getLicenseByID = `-- name: GetLicenseByID :one diff --git a/dogfood/coder/ubuntu-22.04/Dockerfile b/dogfood/coder/ubuntu-22.04/Dockerfile index 0258e51e28..336a0e755b 100644 --- a/dogfood/coder/ubuntu-22.04/Dockerfile +++ b/dogfood/coder/ubuntu-22.04/Dockerfile @@ -51,7 +51,7 @@ RUN apt-get update && \ # Switched to coder/sqlc fork to fix ambiguous column bug, see: # - https://github.com/coder/sqlc/pull/1 # - https://github.com/sqlc-dev/sqlc/pull/4159 - (CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@aab4e865a51df0c43e1839f81a9d349b41d14f05) && \ + (CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@337309bfb9524f38466a5090e310040fc7af0203) && \ # ruleguard for checking custom rules, without needing to run all of # golangci-lint. Check the go.mod in the release of golangci-lint that # we're using for the version of go-critic that it embeds, then check diff --git a/dogfood/coder/ubuntu-26.04/Dockerfile b/dogfood/coder/ubuntu-26.04/Dockerfile index 1e44341d92..869922bac5 100644 --- a/dogfood/coder/ubuntu-26.04/Dockerfile +++ b/dogfood/coder/ubuntu-26.04/Dockerfile @@ -51,7 +51,7 @@ RUN apt-get update && \ # Switched to coder/sqlc fork to fix ambiguous column bug, see: # - https://github.com/coder/sqlc/pull/1 # - https://github.com/sqlc-dev/sqlc/pull/4159 - (CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@aab4e865a51df0c43e1839f81a9d349b41d14f05) && \ + (CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@337309bfb9524f38466a5090e310040fc7af0203) && \ # ruleguard for checking custom rules, without needing to run all of # golangci-lint. Check the go.mod in the release of golangci-lint that # we're using for the version of go-critic that it embeds, then check diff --git a/flake.nix b/flake.nix index f8e8568769..204d91e579 100644 --- a/flake.nix +++ b/flake.nix @@ -96,17 +96,17 @@ # 5. Update the vendorHash sqlc-custom = unstablePkgs.buildGo126Module { pname = "sqlc"; - version = "coder-fork-aab4e865a51df0c43e1839f81a9d349b41d14f05"; + version = "coder-fork-337309bfb9524f38466a5090e310040fc7af0203"; src = pkgs.fetchFromGitHub { owner = "coder"; repo = "sqlc"; - rev = "aab4e865a51df0c43e1839f81a9d349b41d14f05"; - sha256 = "sha256-zXjTypEFWDOkoZMKHMMRtAz2coNHSCkQ+nuZ8rOnzZ8="; + rev = "337309bfb9524f38466a5090e310040fc7af0203"; + sha256 = "sha256-i8hZaaMlNJyW0hUWYcuNqUcwRdQU747055OknZsJ9Es="; }; subPackages = [ "cmd/sqlc" ]; - vendorHash = "sha256-69kg3qkvEWyCAzjaCSr3a73MNonub9sZTYyGaCW+UTI="; + vendorHash = "sha256-4Cb15MhKyhRvYVKfMqBwuC3WBBIJE6AinJt02+TSMVY="; }; # Keep Terraform aligned with provisioner/terraform/testdata/version.txt