Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: fix statement generation when ensuring roles #135634

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rafiss
Copy link
Collaborator

@rafiss rafiss commented Nov 18, 2024

Since we started skipping over non-existent roles in 35f723e, we need to make sure we only add commas at the appropriate point.

The test update demonstrates that there was a bug before this patch, as it would fail with:

expected:
ok defaultdb

found:
ERROR: LDAP authorization: error assigning roles to user ldap_user: EnsureUserOnlyBelongsToRoles-grant: at or near ",": syntax error (SQLSTATE 42601)
HINT: try \h GRANT
DETAIL: source SQL:
GRANT , "ldap-parent-synced" TO ldap_user

informs #133779
Release note: None

Since we started skipping over non-existent roles in
35f723e, we need to make sure we only
add commas at the appropriate point.

The test update demonstrates that there was a bug before this patch, as
it would fail with:
```
expected:
ok defaultdb

found:
ERROR: LDAP authorization: error assigning roles to user ldap_user: EnsureUserOnlyBelongsToRoles-grant: at or near ",": syntax error (SQLSTATE 42601)
HINT: try \h GRANT
DETAIL: source SQL:
GRANT , "ldap-parent-synced" TO ldap_user
```

Release note: None
@rafiss rafiss added the backport-24.3.x Flags PRs that need to be backported to 24.3 label Nov 18, 2024
@rafiss rafiss requested review from a team as code owners November 18, 2024 21:34
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-24.3.x Flags PRs that need to be backported to 24.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants