catalog/lease: handle lease timestamps below GC interval

[fqazi]

Previously, when locked leasing timestamps were used, it was possible for the lease time to fall below the GC interval in some synthetic scenarios during name resolution. To address this, this patch updates the name resolution logic in the lease manager to treat GC errors as missing descriptors at a given timestamp. This forces the system to use the read timestamp correctly.

Fixes: #161220

Release note: None

[cockroach-teamcity]

This change is 

[github-actions]

Potential Bug(s) Detected

The three-stage Claude Code analysis has identified potential bug(s) in this PR that may warrant investigation.

Next Steps:
Please review the detailed findings in the workflow run.

Note: When viewing the workflow output, scroll to the bottom to find the Final Analysis Summary.

After you review the findings, please tag the issue as follows:

• If the detected issue is real or was helpful in any way, please tag the issue with O-AI-Review-Real-Issue-Found
• If the detected issue was not helpful in any way, please tag the issue with O-AI-Review-Not-Helpful

[rafiss]

this fix looks good, just had two nits.

@rafiss made 3 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @fqazi).

---

pkg/sql/catalog/lease/lease.go line 2093 at r1 (raw file):

	var err error
	id, err := m.resolveName(ctx, timestamp.GetTimestamp(), parentID, parentSchemaID, name)
	if err != nil &&

nit: can you simplify this logic so that everything is inside of one top-level if err != nil { ... }. i think this would improve readability.

---

pkg/sql/catalog/lease/lease_test.go line 4144 at r1 (raw file):

// TestLeaseBeforeGC validates if a lease timestamp for some reason is before
// a GC interval, we correctly fallback to the read timestamp from the txn.
func TestLeaseBeforeGC(t *testing.T) {

we should also unskip TestRestoreAsOfSystemTimeGCBounds as part of this PR, since this is closing the linked issue #161220

(verify the test passes under stress first, see repro notes at #161220 (comment))

[fqazi]

@fqazi made 1 comment and resolved 1 discussion.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss).

---

pkg/sql/catalog/lease/lease_test.go line 4144 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

we should also unskip TestRestoreAsOfSystemTimeGCBounds as part of this PR, since this is closing the linked issue #161220

(verify the test passes under stress first, see repro notes at #161220 (comment))

The test was already unskipped via: #161795.

I did confirm after reverting Andrew's PR (and unskipping the test) the issue no longer reproduces under stress.

[rafiss]

@rafiss made 1 comment.
Reviewable status: complete! 1 of 0 LGTMs obtained.

[fqazi]

@rafiss TFTR!

bors r+

[craig]

Build succeeded:

• local_roachtest_fips
• lint
• local_roachtest
• linux_amd64_fips_build
• linux_amd64_build
• docker_image_amd64
• acceptance
• examples_orms
• unit_tests
• check_generated_code

[blathers-crl]

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.

---

Issue #161220: branch-release-26.1.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}