DoDiODev opened a new pull request, #9031:
URL: https://github.com/apache/devlake/pull/9031
## 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/pq` →
`github.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` |
### 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](https://github.com/advisories/GHSA-9jj7-4m8r-rfcm) |
**CRITICAL** |
| [GHSA-xgrm-4fwx-7qm8](https://github.com/advisories/GHSA-xgrm-4fwx-7qm8) |
**CRITICAL** |
| [GHSA-j88v-2chj-qfwx](https://github.com/advisories/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):** 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
30918493865](https://github.com/DoDiODev/devlake/actions/runs/30918493865),
**8/8 jobs successful**: builder image, lint (go), unit-test, e2e (mysql),
migration-script-lint, config-ui, ASF license header, grafana dashboards.
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
