Skip to content

sql/resolver: rename AvoidLeased to AssertNotLeased#108681

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
fqazi:avoidLeasedFix
Aug 14, 2023
Merged

sql/resolver: rename AvoidLeased to AssertNotLeased#108681
craig[bot] merged 1 commit intocockroachdb:masterfrom
fqazi:avoidLeasedFix

Conversation

@fqazi
Copy link
Copy Markdown
Collaborator

@fqazi fqazi commented Aug 13, 2023

Previously, we had a flag to avoid leased descriptors, which did not do anything within look-up contexts. This was because this flag was never really used since skipping leased descriptors happens at a higher level with flags on the resolver. To address this, this
the patch will make this flag more useful by making it an assertion that no leased descriptor is being used in these contexts.

Release note: None

@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Aug 13, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

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

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@fqazi fqazi marked this pull request as ready for review August 14, 2023 14:31
@fqazi fqazi requested a review from a team as a code owner August 14, 2023 14:31
@fqazi fqazi requested a review from a team August 14, 2023 14:31
Copy link
Copy Markdown
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice find! glad this didn't actually cause issues

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @fqazi)


-- commits line 5 at r1:
nit: missing word "which did not ___ anything"

Previously, we had a flag to avoid leased descriptors,
which did not do anything within look-up contexts. This
was because this flag was never really used since skipping
leased descriptors happens at a higher level with flags on
the resolver. To address this, this the patch will make this flag
more useful by making it an assertion that no leased descriptor
is being used in these contexts.

Fixes: cockroachdb#108714
Release note: None
@fqazi
Copy link
Copy Markdown
Collaborator Author

fqazi commented Aug 14, 2023

@rafiss TFTR!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 14, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 14, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 14, 2023

This PR was included in a batch that timed out, it will be automatically retried

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 14, 2023

Build succeeded:

@craig craig bot merged commit 96b4a8e into cockroachdb:master Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants