Skip to content

Commit 8955599

Browse files
ethanndicksonethanndickson
authored
fix: bump sqlc fork to v1.31.1 merge, strip pg_dump meta-commands (#25105)
Closes coder/internal#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](coder/sqlc@337309b), the merge of upstream v1.31.1 (coder/sqlc#6). v1.31.1 includes [sqlc-dev/sqlc#4390](sqlc-dev/sqlc#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](sqlc-dev/sqlc#4383): don't shadow the input parameter when scanning a single-column return).
1 parent eedde58 commit 8955599

9 files changed

Lines changed: 67 additions & 33 deletions

File tree

‎.github/actions/setup-sqlc/action.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ runs:
1414
# - https://github.com/sqlc-dev/sqlc/pull/4159
1515
shell: bash
1616
run: |
17-
./.github/scripts/retry.sh -- env CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@aab4e865a51df0c43e1839f81a9d349b41d14f05
17+
./.github/scripts/retry.sh -- env CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@337309bfb9524f38466a5090e310040fc7af0203

‎coderd/database/dbtestutil/db.go‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os/exec"
1111
"path/filepath"
1212
"regexp"
13+
"strconv"
1314
"strings"
1415
"testing"
1516
"time"
@@ -240,31 +241,26 @@ func PGDump(dbURL string) ([]byte, error) {
240241
return stdout.Bytes(), nil
241242
}
242243

243-
const (
244-
minimumPostgreSQLVersion = 13
245-
postgresImageSha = "sha256:467e7f2fb97b2f29d616e0be1d02218a7bbdfb94eb3cda7461fd80165edfd1f7"
246-
)
244+
const minimumPostgreSQLVersion = 13
247245

248246
// PGDumpSchemaOnly is for use by gen/dump only.
249247
// It runs pg_dump against dbURL and sets a consistent timezone and encoding.
250248
func PGDumpSchemaOnly(dbURL string) ([]byte, error) {
251249
hasPGDump := false
252-
// TODO: Temporarily pin pg_dump to the docker image until
253-
// https://github.com/sqlc-dev/sqlc/issues/4065 is resolved.
254-
// if _, err := exec.LookPath("pg_dump"); err == nil {
255-
// out, err := exec.Command("pg_dump", "--version").Output()
256-
// if err == nil {
257-
// // Parse output:
258-
// // pg_dump (PostgreSQL) 14.5 (Ubuntu 14.5-0ubuntu0.22.04.1)
259-
// parts := strings.Split(string(out), " ")
260-
// if len(parts) > 2 {
261-
// version, err := strconv.Atoi(strings.Split(parts[2], ".")[0])
262-
// if err == nil && version >= minimumPostgreSQLVersion {
263-
// hasPGDump = true
264-
// }
265-
// }
266-
// }
267-
// }
250+
if _, err := exec.LookPath("pg_dump"); err == nil {
251+
out, err := exec.Command("pg_dump", "--version").Output()
252+
if err == nil {
253+
// Parse output:
254+
// pg_dump (PostgreSQL) 14.5 (Ubuntu 14.5-0ubuntu0.22.04.1)
255+
parts := strings.Split(string(out), " ")
256+
if len(parts) > 2 {
257+
version, err := strconv.Atoi(strings.Split(parts[2], ".")[0])
258+
if err == nil && version >= minimumPostgreSQLVersion {
259+
hasPGDump = true
260+
}
261+
}
262+
}
263+
}
268264

269265
cmdArgs := []string{
270266
"pg_dump",
@@ -289,7 +285,7 @@ func PGDumpSchemaOnly(dbURL string) ([]byte, error) {
289285
"run",
290286
"--rm",
291287
"--network=host",
292-
fmt.Sprintf("%s:%d@%s", postgresImage, minimumPostgreSQLVersion, postgresImageSha),
288+
fmt.Sprintf("%s:%d", postgresImage, minimumPostgreSQLVersion),
293289
}, cmdArgs...)
294290
}
295291
cmd := exec.Command(cmdArgs[0], cmdArgs[1:]...) //#nosec
@@ -310,6 +306,11 @@ func PGDumpSchemaOnly(dbURL string) ([]byte, error) {
310306
func normalizeDump(schema []byte) []byte {
311307
// Remove all comments.
312308
schema = regexp.MustCompile(`(?im)^(--.*)$`).ReplaceAll(schema, []byte{})
309+
// Strip psql meta-commands (\restrict / \unrestrict) emitted by pg_dump
310+
// 13.22+ / 14.19+ / 15.14+ / 16.10+ / 17.6+. The token in these lines is
311+
// randomized per run, so we drop them entirely. See
312+
// https://github.com/coder/internal/issues/965.
313+
schema = regexp.MustCompile(`(?im)^\\(restrict|unrestrict).*$`).ReplaceAll(schema, []byte{})
313314
// Public is implicit in the schema.
314315
schema = regexp.MustCompile(`(?im)( |::|'|\()public\.`).ReplaceAll(schema, []byte(`$1`))
315316
// Remove database settings.

‎coderd/database/dbtestutil/db_internal_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package dbtestutil
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
// Recent pg_dump versions (13.22+ / 14.19+ / 15.14+ / 16.10+ / 17.6+) emit
10+
// psql meta-commands at the head and tail of the dump that aren't valid SQL.
11+
// normalizeDump is expected to strip them so downstream consumers (sqlc,
12+
// schema-equality checks in scripts/migrate-test) don't have to.
13+
//
14+
// See https://github.com/coder/internal/issues/965.
15+
func TestNormalizeDumpStripsRestrict(t *testing.T) {
16+
t.Parallel()
17+
18+
// Raw string literals (backticks) make backslashes literal, so the
19+
// meta-command here matches what pg_dump actually emits.
20+
input := []byte(`-- header
21+
\restrict XYZ
22+
23+
CREATE TABLE foo;
24+
25+
\unrestrict XYZ
26+
`)
27+
28+
out := string(normalizeDump(input))
29+
require.NotContains(t, out, `\restrict`, `normalizeDump must strip \restrict psql meta-command`)
30+
require.NotContains(t, out, `\unrestrict`, `normalizeDump must strip \unrestrict psql meta-command`)
31+
require.Contains(t, out, "CREATE TABLE foo;", "normalizeDump must preserve real SQL between the meta-commands")
32+
}

‎coderd/database/models.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go‎

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dogfood/coder/ubuntu-22.04/Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ RUN apt-get update && \
5151
# Switched to coder/sqlc fork to fix ambiguous column bug, see:
5252
# - https://github.com/coder/sqlc/pull/1
5353
# - https://github.com/sqlc-dev/sqlc/pull/4159
54-
(CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@aab4e865a51df0c43e1839f81a9d349b41d14f05) && \
54+
(CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@337309bfb9524f38466a5090e310040fc7af0203) && \
5555
# ruleguard for checking custom rules, without needing to run all of
5656
# golangci-lint. Check the go.mod in the release of golangci-lint that
5757
# we're using for the version of go-critic that it embeds, then check

‎dogfood/coder/ubuntu-26.04/Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ RUN apt-get update && \
5151
# Switched to coder/sqlc fork to fix ambiguous column bug, see:
5252
# - https://github.com/coder/sqlc/pull/1
5353
# - https://github.com/sqlc-dev/sqlc/pull/4159
54-
(CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@aab4e865a51df0c43e1839f81a9d349b41d14f05) && \
54+
(CGO_ENABLED=1 go install github.com/coder/sqlc/cmd/sqlc@337309bfb9524f38466a5090e310040fc7af0203) && \
5555
# ruleguard for checking custom rules, without needing to run all of
5656
# golangci-lint. Check the go.mod in the release of golangci-lint that
5757
# we're using for the version of go-critic that it embeds, then check

‎flake.nix‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,17 +96,17 @@
9696
# 5. Update the vendorHash
9797
sqlc-custom = unstablePkgs.buildGo126Module {
9898
pname = "sqlc";
99-
version = "coder-fork-aab4e865a51df0c43e1839f81a9d349b41d14f05";
99+
version = "coder-fork-337309bfb9524f38466a5090e310040fc7af0203";
100100

101101
src = pkgs.fetchFromGitHub {
102102
owner = "coder";
103103
repo = "sqlc";
104-
rev = "aab4e865a51df0c43e1839f81a9d349b41d14f05";
105-
sha256 = "sha256-zXjTypEFWDOkoZMKHMMRtAz2coNHSCkQ+nuZ8rOnzZ8=";
104+
rev = "337309bfb9524f38466a5090e310040fc7af0203";
105+
sha256 = "sha256-i8hZaaMlNJyW0hUWYcuNqUcwRdQU747055OknZsJ9Es=";
106106
};
107107

108108
subPackages = [ "cmd/sqlc" ];
109-
vendorHash = "sha256-69kg3qkvEWyCAzjaCSr3a73MNonub9sZTYyGaCW+UTI=";
109+
vendorHash = "sha256-4Cb15MhKyhRvYVKfMqBwuC3WBBIJE6AinJt02+TSMVY=";
110110
};
111111

112112
# Keep Terraform aligned with provisioner/terraform/testdata/version.txt

0 commit comments

Comments
 (0)