Repository navigation
fix(deps): pin the pgx fork at its master tip - #21862
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9688dc6033
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59fa54e587
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
88edae5 to
0d1fdaf
Compare
d-bytebase
changed the title
The pin named d3c6cdda, the v5.12.0 rebase, which is an ancestor of the fork's master rather than its tip. Naming the tip keeps the pin on the default branch, so a later rebase starts from the commit Bytebase actually ships. This changes no code. The module trees at the two pseudo-versions are identical, and both fork patches are present: ColumnTypePrecisionScale and the timestamptz session TimeZone conversion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0d1fdaf to
ee8d62e
Compare
d-bytebase
changed the title
Dependency hygiene only. Two lines of
go.mod/go.sum, no code change.The pin named
d3c6cdda, the v5.12.0 rebase, which is an ancestor of the fork'smasterrather than its tip. Pinning an ancestor still resolves, but it means thedefault branch and the shipped commit can drift: a later rebase would start from
whatever
masterhappens to be, not from what Bytebase runs. This points the pin atmaster's tip so those are the same commit.This changes no code
diff -rbetween the two module trees in the module cache is empty.ColumnTypePrecisionScale(
stdlib/sql.go:704) and the timestamptz sessionTimeZoneconversion(
stdlib/sql.go:857).TestConvertValueTimestampFromDriverpasses unchanged, including itsZone == "CST"/Offset == 28800assertions. Nothing in the test suite moved.The fork's
masterreached this state via bytebase/pgx#4, which revertedbytebase/pgx#3 so that
mastermatches the shipped commit again.Not in this PR
The v5.11 → v5.12 upgrade itself already landed in #21840. This PR adds no new
dependency version.
Removing the fork's time zone patch — and with it, deciding where
timestamptzshould be rendered — is deliberately not here. That design question is open, and
the POSIX
TimeZonefailure behind it is real and still unfixed: an instance whosesession
TimeZoneis+08:00orEST5EDT,M3.2.0/2,M11.1.0/2fails everytimestamptzread, because the patch callstime.LoadLocationon that string. It ispre-existing, not a regression, and it belongs with the time zone work.
Verification
gofmtclean;golangci-lintv2.15.0 →0 issues; server build OK../backend/store/...and./backend/plugin/db/{pg,redshift,cockroachdb}pass.backend/testssample-instance tests pass.🤖 Generated with Claude Code