Skip to content

Commit

Permalink
Merge #85996
Browse files Browse the repository at this point in the history
85996: sql: don't panic on auth check with unopen txn r=ajwerner,RichardJCai a=rafiss

Relates to #80764 and #82034

Ever since c00ea84 was merged, the KV
layer has disallowed use of an aborted txn. Therefore, the checks here
are no longer necessary.

This should actually help with debugging, since now if the aborted txn
is used, we should get back an error that has the reason for the abort
(or restart), instead of a panic that does not have that info.

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
craig[bot] and rafiss committed Aug 12, 2022
2 parents 9394d64 + 9fff27f commit 5151d97
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions pkg/sql/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (p *planner) CheckPrivilegeForUser(
// Verify that the txn is valid in any case, so that
// we don't get the risk to say "OK" to root requests
// with an invalid API usage.
if p.txn == nil || !p.txn.IsOpen() {
if p.txn == nil {
return errors.AssertionFailedf("cannot use CheckPrivilege without a txn")
}

Expand Down Expand Up @@ -326,7 +326,7 @@ func (p *planner) CheckAnyPrivilege(
// Verify that the txn is valid in any case, so that
// we don't get the risk to say "OK" to root requests
// with an invalid API usage.
if p.txn == nil || !p.txn.IsOpen() {
if p.txn == nil {
return errors.AssertionFailedf("cannot use CheckAnyPrivilege without a txn")
}

Expand Down Expand Up @@ -377,7 +377,7 @@ func (p *planner) UserHasAdminRole(ctx context.Context, user username.SQLUsernam
// Verify that the txn is valid in any case, so that
// we don't get the risk to say "OK" to root requests
// with an invalid API usage.
if p.txn == nil || !p.txn.IsOpen() {
if p.txn == nil {
return false, errors.AssertionFailedf("cannot use HasAdminRole without a txn")
}

Expand Down Expand Up @@ -654,7 +654,7 @@ func (p *planner) HasRoleOption(ctx context.Context, roleOption roleoption.Optio
// Verify that the txn is valid in any case, so that
// we don't get the risk to say "OK" to root requests
// with an invalid API usage.
if p.txn == nil || !p.txn.IsOpen() {
if p.txn == nil {
return false, errors.AssertionFailedf("cannot use HasRoleOption without a txn")
}

Expand Down

0 comments on commit 5151d97

Please sign in to comment.