Skip to content

Commit e07a8c8

Browse files
craig[bot]yuzefovichrickystewart
committed
143695: tree: include gist into enum comparison assertion r=yuzefovich a=yuzefovich In theory, the plan gist should already be included in all ~assertion failures~sentry reports, but it doesn't happen in practice. Let's include it explicitly (if it is present in the context) into the enum mismatched version comparison error to help with tracking it down. Informs: #143571 Epic: None Release note: None 143749: build/README: add step to upgrade golang.org/x packages r=jlinder a=rickystewart [Internal discussion](https://cockroachlabs.slack.com/archives/CJ0H8Q97C/p1743193919914069) Epic: none Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Ricky Stewart <[email protected]>
3 parents 757ce69 + e300eaf + b33e73d commit e07a8c8

File tree

3 files changed

+19
-3
lines changed

3 files changed

+19
-3
lines changed

build/README.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ do so:
7575
7676
# Updating the golang version
7777
78+
The Go upgrade process is documented [here](https://cockroachlabs.atlassian.net/wiki/spaces/devinf/pages/4193714322/CockroachDB+Go+upgrade+policy+and+process).
79+
This checklist is meant to be followed by the dev-inf team member that
80+
performs the upgrade and constructs the draft PR.
81+
7882
Please copy this checklist into the relevant commit message and perform these
7983
steps:
8084
@@ -87,7 +91,8 @@ steps:
8791
* [ ] Adjust `--@io_bazel_rules_go//go/toolchain:sdk_version` in [.bazelrc](../.bazelrc).
8892
* [ ] Bump the version in `WORKSPACE` under `go_download_sdk`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases). Also edit the filenames listed in `sdks` and update all the hashes to match what you built in the step above.
8993
* [ ] Bump the version in `WORKSPACE` under `go_download_sdk` for the FIPS version of Go (`go_sdk_fips`).
90-
* [ ] Run `./dev generate bazel` to refresh `distdir_files.bzl`, then `bazel fetch @distdir//:archives` to ensure you've updated all hashes to the correct value.
94+
* [ ] Upgrade golang.org/x packages; these are maintained by the Go project and it's reasonable to upgrade them when doing our Go upgrade. Run `grep -e '^\tgolang.org/x' go.mod | grep -v vcs | grep -v image | grep -v typeparams | cut -w -f2 | sed 's/$/@latest/' | xargs go get`. (Note: we don't upgrade certain libraries that are not linked into CRDB, hence the `grep -v`.)
95+
* [ ] Run `./dev generate bazel --mirror`, then `bazel fetch @distdir//:archives` to ensure you've updated all hashes to the correct value.
9196
* [ ] Bump the go version in `go.mod`.
9297
* [ ] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)).
9398
* [ ] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).

pkg/sql/conn_executor.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4854,4 +4854,5 @@ func init() {
48544854
logcrash.RegisterTagFn("gist", func(ctx context.Context) string {
48554855
return planGistFromCtx(ctx)
48564856
})
4857+
tree.PlanGistFromCtx = planGistFromCtx
48574858
}

pkg/sql/sem/tree/datum.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5474,6 +5474,10 @@ func (d *DEnum) ResolvedType() *types.T {
54745474
return d.EnumTyp
54755475
}
54765476

5477+
// PlanCistFromCtx returns the plan gist if it is stored in the context. It is
5478+
// injected from the sql package to avoid import cycle.
5479+
var PlanGistFromCtx func(context.Context) string
5480+
54775481
// Compare implements the Datum interface.
54785482
func (d *DEnum) Compare(ctx context.Context, cmpCtx CompareContext, other Datum) (int, error) {
54795483
if other == DNull {
@@ -5491,10 +5495,16 @@ func (d *DEnum) Compare(ctx context.Context, cmpCtx CompareContext, other Datum)
54915495

54925496
// We should never be comparing two different versions of the same enum.
54935497
if v.EnumTyp.TypeMeta.Version != d.EnumTyp.TypeMeta.Version {
5498+
var gist redact.SafeString
5499+
if PlanGistFromCtx != nil {
5500+
// Plan gist, by construction, doesn't contain any PII, so it's a
5501+
// safe string.
5502+
gist = redact.SafeString(PlanGistFromCtx(ctx))
5503+
}
54945504
panic(errors.AssertionFailedf(
5495-
"comparison of two different versions of enum %s oid %d: versions %d and %d",
5505+
"comparison of two different versions of enum %s oid %d: versions %d and %d, gist %q",
54965506
d.EnumTyp.SQLStringForError(), errors.Safe(d.EnumTyp.Oid()), d.EnumTyp.TypeMeta.Version,
5497-
v.EnumTyp.TypeMeta.Version,
5507+
v.EnumTyp.TypeMeta.Version, gist,
54985508
))
54995509
}
55005510

0 commit comments

Comments
 (0)