Skip to content

sql/rls: refactor rls filter injection #147364

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

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

Conversation

spilchen
Copy link
Contributor

This does a refactor of the logic that adds the filter for row-level security (RLS) policies. The intent is to match what we did for the check constraint. Namely a single function (addRowLevelSecurityFilter) that you can quickly see at a glance what policies are applied to the various policy command scope.

There is a slight update to the optbuilder tests as we include redundant clauses that didn't exist before. These will get optimized out.

During the refactor, I fixed a bug where the RLS state mishandled applied policies: if a SELECT policy existed but no DELETE or UPDATE policy was present, we incorrectly treated it as if no policy applied.

Epic: none
Release note: none

This does a refactor of the logic that adds the filter for row-level
security (RLS) policies. The intent is to match what we did for the
check constraint. Namely a single function (addRowLevelSecurityFilter)
that you can quickly see at a glance what policies are applied to the
various policy command scope.

There is a slight update to the optbuilder tests as we include redundant
clauses that didn't exist before. These will get optimized out.

Epic: none
Release note: none
@spilchen spilchen self-assigned this May 27, 2025
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@spilchen spilchen requested a review from mgartner May 27, 2025 23:28
@spilchen spilchen marked this pull request as ready for review May 27, 2025 23:28
@spilchen spilchen requested a review from a team as a code owner May 27, 2025 23:28
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.

2 participants