Retire gp-common-go-libs and sqlx; inline replacements in-repo#29
Open
adam8157 wants to merge 2 commits into
Open
Retire gp-common-go-libs and sqlx; inline replacements in-repo#29adam8157 wants to merge 2 commits into
adam8157 wants to merge 2 commits into
Conversation
Replace the external gp-common-go-libs dependency (and its transitive sqlx dep) with seven top-level packages copied/ported into the repo: cluster/, dbconn/, gplog/, iohelper/, operating/, structmatcher/, testhelper/. The bulk of the file churn is mechanical import-path rewrites; APIs were preserved so call sites didn't need to change. Zero new direct deps; the replace directive pointing at warehouse-pg/gp- common-go-libs is gone. dbconn is rewritten on database/sql + pgx/v5 stdlib driver with a small reflect-based scanner (db: tag mapping, embedded structs, sql.Scanner passthrough so pq.StringArray still works) — sqlx is out. gplog is reimplemented on stdlib log.Logger. Internal behavior changes worth flagging: - dbconn.Get is now strict: returns ErrMultipleRows on >1 row instead of sqlx's silent first-row. Every production call site was audited and is 1-row by construction (scalar functions, SHOW <guc>, PK-unique catalog lookups, cluster invariants). - dbconn.handleConnectionError classifies pgx errors by SQLSTATE (42704/3D000) instead of substring-matching lib/pq error prefixes. - dbconn.Begin uses BeginTx(LevelSerializable); previously BEGIN + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE. - DBConn.ConnPool[i] is *sql.DB (was *sqlx.DB); Tx[i] is *sql.Tx.
- `errors.Errorf(...)` -> `fmt.Errorf(...)`
- `errors.Wrap(err, msg)` -> `fmt.Errorf("%s: %w", msg, err)` so the
wrapped error is still recoverable via stdlib `errors.Unwrap` / `%w`
- `errors.New` / `errors.As` / `errors.Is` -> stdlib `errors`
- Stack capture in `gplog.Fatal` switches from `errors.WithStack` to
`runtime.Callers` + `runtime.CallersFrames` via a new internal
`captureStack()`. The stack still starts at Fatal's caller, matching
the previous behavior.
Only used for `pq.StringArray` while scanning the array columns in
`pg_statistic`. Replaced by an in-repo `sql.Scanner` at `dbconn/array.go`
(~75 lines, with table-driven tests). NULL element handling matches
pq's strict behavior (errors out; does not silently produce "").
Write to the pipe from a goroutine so content larger than the OS pipe
buffer (~64 KB) no longer deadlocks the writer.
`tools.go` deleted; `ginkgo` / `goimports` are now invoked via
`go tool ginkgo` / `go tool goimports`. `golang.org/x/tools` moves to
indirect. Requires Go 1.24+ (we're on 1.25).
- Delete three more methods that gpbackup never called externally
(`Query`, `QueryContext`, `ExecContext`). These were API-parity
carryovers from gp-common-go-libs.
- Rewrite `SelectString` to scan at most two rows from a single
`sql.Rows` iteration instead of materializing the whole result via
`SelectStringSlice`. Semantics unchanged (0 rows -> "", nil; >1 row
-> error).
- `strings.Replace(s, ..., -1)` -> `strings.ReplaceAll`
- `reflect.PtrTo` -> `reflect.PointerTo` (PtrTo is deprecated since 1.18)
- `errors.New(fmt.Sprintf(...))` -> `fmt.Errorf` in `restore/validate.go`
- Update stale comments that still referenced sqlx
Two GetIndex specs created indexes with the same name in two schemas,
then called `OidFromObjectName(conn, "", "simple_table_idx1", TYPE_INDEX)`
to populate `results[0].Oid`. The empty `schemaName` made the lookup
ambiguous (two rows). sqlx silently returned the first; the new strict
`dbconn.Get` errors with `ErrMultipleRows`. Since `Oid` is excluded from
the structmatcher comparison anyway, the lookups were vestigial.
Contributor
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the external gp-common-go-libs dependency (and its transitive sqlx
dep) with seven top-level packages copied/ported into the repo:
cluster/, dbconn/, gplog/, iohelper/, operating/, structmatcher/, testhelper/.
The bulk of the file churn is mechanical import-path rewrites; APIs were
preserved so call sites didn't need to change.
Zero new direct deps; the replace directive pointing at warehouse-pg/gp-
common-go-libs is gone. dbconn is rewritten on database/sql + pgx/v5 stdlib
driver with a small reflect-based scanner (db: tag mapping, embedded
structs, sql.Scanner passthrough so pq.StringArray still works) — sqlx is
out. gplog is reimplemented on stdlib log.Logger.
Internal behavior changes worth flagging:
sqlx's silent first-row. Every production call site was audited and is
1-row by construction (scalar functions, SHOW , PK-unique catalog
lookups, cluster invariants).
(42704/3D000) instead of substring-matching lib/pq error prefixes.
TRANSACTION ISOLATION LEVEL SERIALIZABLE.