You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "DoDiODev (via GitHub)" <gi...@apache.org> on 2026/08/04 15:29:09 UTC

[PR] refactor(deps): drop lib/pq and bump pgx/v5 to v5.10.0 [devlake]

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: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] refactor(deps): drop lib/pq and bump pgx/v5 to v5.10.0 [devlake]

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh merged PR #9031:
URL: https://github.com/apache/devlake/pull/9031


-- 
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: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org