Skip to content

release-26.1: sql/unsafesql: fix panic in SafeFormatQuery with nil annotations#166176

Open
angles-n-daemons wants to merge 1 commit intocockroachdb:release-26.1from
angles-n-daemons:backport26.1-166057
Open

release-26.1: sql/unsafesql: fix panic in SafeFormatQuery with nil annotations#166176
angles-n-daemons wants to merge 1 commit intocockroachdb:release-26.1from
angles-n-daemons:backport26.1-166057

Conversation

@angles-n-daemons
Copy link
Contributor

Backport 1/1 commits from #166057.

/cc @cockroachdb/release


Summary

  • FormatAstAsRedactableString unconditionally set FmtAlwaysQualifyTableNames, which requires annotation lookups to resolve fully qualified table names. When SafeFormatQuery was called with nil annotations (as happens in the unsafesql logging path), the formatter panicked on a nil pointer dereference inside Annotations.Get().
  • The panic was caught by SafeFormatQuery's recover handler, replacing the query text with <panicked query format> in log output and losing useful diagnostic information.
  • Now FmtAlwaysQualifyTableNames is only set when annotations are non-nil. Without annotations, table names are printed unqualified (e.g. namespace instead of system.public.namespace), which is strictly better than a panic placeholder.

Resolves: #165998
Epic: CRDB-61709

Release note: None
Release justification: Prevents a panic from occurring during unsafe sql gate formatting.

FormatAstAsRedactableString unconditionally set FmtAlwaysQualifyTableNames,
which requires annotation lookups to resolve fully qualified table names.
When SafeFormatQuery was called with nil annotations (as happens in the
unsafesql logging path), the formatter panicked on a nil pointer
dereference inside Annotations.Get().

The panic was caught by SafeFormatQuery's recover handler, but this meant
the query text was replaced with "<panicked query format>" in log output,
losing useful diagnostic information.

Now FmtAlwaysQualifyTableNames is only set when annotations are non-nil.
Without annotations, table names are printed unqualified (e.g. "namespace"
instead of "system.public.namespace"), which is strictly better than a
panic placeholder.

Resolves: cockroachdb#165998
Epic: CRDB-61709

Release note: None
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
@blathers-crl
Copy link

blathers-crl bot commented Mar 19, 2026

Thanks for opening a backport.

Before merging, please confirm that the change does not break backwards compatibility and otherwise complies with the backport policy. Include a brief release justification in the PR description explaining why the backport is appropriate. All backports must be reviewed by the TL for the owning area. While the stricter LTS policy does not yet apply, please exercise judgment and consider gating non-critical changes behind a disabled-by-default feature flag when appropriate.

@blathers-crl blathers-crl bot added backport Label PR's that are backports to older release branches T-observability labels Mar 19, 2026
@angles-n-daemons angles-n-daemons marked this pull request as ready for review March 19, 2026 15:20
@angles-n-daemons angles-n-daemons requested a review from a team as a code owner March 19, 2026 15:20
@angles-n-daemons angles-n-daemons requested review from DrewKimball and removed request for a team March 19, 2026 15:20
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@angles-n-daemons angles-n-daemons requested a review from rafiss March 19, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches T-observability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants