Skip to content

refactor(deps): drop lib/pq and bump pgx/v5 to v5.10.0 - #9031

Open
DoDiODev wants to merge 3 commits into
apache:mainfrom
DoDiODev:pr/wave4k-pgx
Open

refactor(deps): drop lib/pq and bump pgx/v5 to v5.10.0#9031
DoDiODev wants to merge 3 commits into
apache:mainfrom
DoDiODev:pr/wave4k-pgx

Conversation

@DoDiODev

@DoDiODev DoDiODev commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

PostgreSQL support in this repository already runs entirely on pgx:
backend/core/runner/db.go opens PostgreSQL connections through
gorm.io/driver/postgres, which uses github.com/jackc/pgx/v5 under the hood.
There is no sql.Open("postgres", ...) and no blank import of lib/pq anywhere
in the tree.

github.com/lib/pq was nevertheless still a direct dependency because of a
single call site — pq.Array in the StarRocks plugin's generic row scan.
This PR replaces that call with the pgx equivalent and drops lib/pq.

Because pgx thereby becomes a direct dependency, it is also raised from the
currently vendored v5.6.0 to v5.10.0, which closes two CRITICAL
advisories that already affect the tree today (pgx is shipped transitively).

What changed

File Change
backend/plugins/starrocks/tasks/tasks.go pq.Array(&arr)typeMap.SQLScanner(&arr); import github.com/lib/pqgithub.com/jackc/pgx/v5/pgtype
backend/plugins/starrocks/tasks/tasks_test.go (new) unit test pinning the array-literal parsing behaviour
backend/go.mod github.com/lib/pq v1.10.2 removed; github.com/jackc/pgx/v5 v5.6.0 (indirect) → v5.10.0 (direct)
backend/go.sum regenerated by go mod tidy
backend/scripts/install-go.sh (new) idempotent Go 1.26.2 CI bootstrap with architecture-specific SHA-256 verification
.github/workflows/{golangci-lint,test,test-e2e}.yml runs the bootstrap before the compiling lint, unit, MySQL-E2E and PostgreSQL-E2E paths

Why pgtype.Map.SQLScanner and not something simpler

The PostgreSQL driver does not hand array columns to database/sql as a
typed value. In pgx/v5/stdlib, array OIDs fall through to the default branch
of the DataTypeOID switch and are delivered as the raw array literal
({a,b}) — i.e. a string. Consequences:

  • scanning directly into *[]string fails
    (storing driver.Value type string into type *[]string), and
  • pgtype.FlatArray[string] does not help either, since it implements the
    native pgx interfaces, not sql.Scanner.

(*pgtype.Map).SQLScanner(v) returns an sql.Scanner that accepts both
string and []byte and parses the literal — the 1:1 replacement for
pq.Array.

The pgtype.Map is constructed once per query, outside the row loop,
because building it registers the full default type catalog; doing it per
row/column would be needlessly expensive (pq.Array was essentially free).

Behaviour is unchanged (and now tested)

The array scan path had no test coverage at all, so the new unit test pins
it down. Both implementations parse identically:

literal result
{a,b} ["a","b"]
{} []
{""} [""]
{"a,x","b\"y"} ["a,x","b\"y"]
{ä ö,"x y"} ["ä ö","x y"]

Note for reviewers: {NULL} was already rejected by pq.Array
(cannot convert nil to string) and is still rejected
(cannot scan NULL into *string). Only the error message differs — this is
neither a regression nor a silent behaviour fix.

Security

The pgx version currently resolved on main (v5.6.0) is affected by:

Advisory Severity
GHSA-9jj7-4m8r-rfcm CRITICAL
GHSA-xgrm-4fwx-7qm8 CRITICAL
GHSA-j88v-2chj-qfwx LOW — simple protocol only, not used here

All three are fixed in v5.9.2; this PR moves to the current v5.10.0.

About the go.mod churn

pgx/v5 v5.10.0 requires golang.org/x/text v0.29.0, golang.org/x/sync v0.17.0, github.com/stretchr/testify v1.11.1, puddle/v2 v2.2.2 and
pgservicefile. Minimal version selection propagates that to
golang.org/x/tools, x/mod, x/net, x/crypto and x/sys. These bumps are
not optional — pinning the minimum fixed pgx (v5.9.2) produces exactly the
same requirement set. No source changes were needed for any of them.

gorm.io/gorm and gorm.io/driver/postgres are deliberately untouched.
driver/postgres v1.5.2 declares pgx only as a minimum version, so raising
pgx does not force an ORM bump.

Validation

CI equivalent (all green): after merging the current upstream/main
(including #9028), the identical job matrix was run in a fork workflow
(fork-ci.yml) on a throwaway branch (this branch plus that workflow,
otherwise unchanged) —
run 31478071596,
8/8 jobs successful: builder image, lint (go), unit-test, e2e (mysql),
migration-script-lint, config-ui, ASF license header, grafana dashboards.

The bootstrap was also exercised twice in the unmodified published
mericodev/lake-builder:latest image: it replaced Go 1.20.4 with Go 1.26.2,
verified the release SHA-256, selected /opt/go/1.26.2 as GOROOT, and the
second invocation was a no-op. go test ./plugins/starrocks/tasks passed in
that bootstrapped container.

Locally on macOS (arm64):

  • go build ./...
  • ./scripts/unit-test-go.sh — 65 packages, exit 0, no failures
  • go test ./plugins/starrocks/... — new array test green
  • go mod why -m github.com/lib/pq"main module does not need module
    github.com/lib/pq"
    ; no lib/pq source import left in the tree; only
    transitive /go.mod hashes of unrelated modules remain in go.sum

Out of scope on purpose

  • backend/core/runner/db.go — already on gorm/pgx, unchanged.
  • gorm.io/driver/postgres v1.6.2 / gorm.io/gorm v1.31.2 — a separate,
    MVS-coupled change; not needed for this one.
  • DSN, compose and E2E configuration — postgres://… URLs stay valid.

Rollback

Restore the import and pq.Array(&arr), re-add lib/pq to go.mod, run
go mod tidy, drop the test. No schema, migration or API impact.

CI compatibility with the stale mericodev/lake-builder:latest image

The compiling Go CI jobs (test.yml, golangci-lint.yml and both jobs in
test-e2e.yml) run inside the prebuilt container
mericodev/lake-builder:latest. The published image currently has Go
1.20.4, so its standard library has neither slices nor crypto/pbkdf2.

pgx/v5 v5.10.0 declares go 1.25 and uses crypto/pbkdf2 (Go 1.24+), and the
MVS-required golang.org/x/sys v0.35.0 declares go 1.23, so the jobs fail
before any test runs:

/go/pkg/mod/github.com/jackc/pgx/v5@v5.10.0/pgconn/auth_scram.go:19:2:
  package crypto/pbkdf2 is not in GOROOT (/usr/local/go/src/crypto/pbkdf2)
  note: imported by a module that requires go 1.25

/go/pkg/mod/golang.org/x/sys@v0.35.0/unix/syscall_linux.go:16:2:
  package slices is not in GOROOT (/usr/local/go/src/slices)
  note: imported by a module that requires go 1.23

This PR now bootstraps Go 1.26.2 in exactly those four compiling jobs before
any build or test. The script verifies the architecture-specific release
SHA-256, installs into /opt/go/1.26.2, prepends its bin directory through
GITHUB_PATH, and is idempotent. migration-script-lint.yml is deliberately
unchanged because that job already succeeds with the image's existing Go.

The PR is therefore no longer coupled to a maintainer republish of the image.
Republishing mericodev/lake-builder:latest with the current toolchain remains
the desirable post-merge cleanup.

PostgreSQL support already runs entirely on pgx: `core/runner/db.go` opens
PostgreSQL connections through `gorm.io/driver/postgres`, which uses
`github.com/jackc/pgx/v5` under the hood. There is no `sql.Open("postgres", ...)`
and no blank import of `lib/pq` anywhere in the tree.

The single remaining functional use of `lib/pq` was in the StarRocks plugin,
where `pq.Array` was used to scan PostgreSQL array columns. It is replaced by
`pgtype.Map.SQLScanner`, the pgx equivalent that implements `sql.Scanner` and
therefore works on the `database/sql` code path used by gorm. The `pgtype.Map`
is created once per query, outside the row loop, because building it registers
the full default type catalog.

Shipping a second, effectively unused PostgreSQL driver also meant shipping
`pgx/v5 v5.6.0`, which is affected by two CRITICAL advisories
(GHSA-9jj7-4m8r-rfcm, GHSA-xgrm-4fwx-7qm8) plus a low-severity one
(GHSA-j88v-2chj-qfwx), all fixed in v5.9.2. Since pgx becomes a direct
dependency here, it is bumped to v5.10.0.

`gorm.io/driver/postgres` and `gorm.io/gorm` are deliberately left untouched:
`driver/postgres v1.5.2` only declares a minimum pgx version, so raising pgx
does not require an ORM bump.

Notes for reviewers:
- `pgx/v5` moves from the indirect block to a direct requirement.
- Array literals parse identically to the previous implementation, including
  empty arrays, empty elements, quoted separators and UTF-8 content. This is
  covered by a new unit test; the array scan path had no test coverage before.
- `NULL` elements inside arrays were already rejected by `pq.Array` and are
  still rejected (only the error message differs) — this is not a regression
  and not a silent fix.

Signed-off-by: DoDiODev <DoDiDev@proton.me>
Signed-off-by: DoDiODev <DoDiDev@proton.me>

# Conflicts:
#	backend/go.mod
#	backend/go.sum
Signed-off-by: DoDiODev <DoDiDev@proton.me>
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.

1 participant