-
Notifications
You must be signed in to change notification settings - Fork 469
Apply SELECT
policies if RETURNING
has cols
#19720
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
base: main
Are you sure you want to change the base?
Apply SELECT
policies if RETURNING
has cols
#19720
Conversation
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
Files changed: |
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
@@ -66,7 +66,6 @@ The following table shows which policies are applied to which statement types, w | |||
|
|||
Additional considerations include: | |||
|
|||
- `SELECT` evaluation: CockroachDB always evaluates `SELECT` (`USING`) policies for `INSERT`, `UPDATE`, and `DELETE`, even when the statement doesn't reference table columns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we include further details as to when the SELECT policies will be applied during an INSERT/UPDATE/DELETE? SELECT policies are always applied during the fetch of the existing row (for UPDATE or DELETE). If a column is referenced in the WHERE, SET or RETURNING clause, then SELECT policies will also get applied during the write.
I think the postgres docs handled this by adding superscript numbers to the above table as an additional addendum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've taken a shot at addressing this feedback in 8af8a1f
tried to incorporate the info from your comment and add some superscripts and more explanation
PTAL!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During writes, the SELECT policy is always applied to the existing row. For the new row being inserted or updated, it's only applied if that row is referenced in the WHERE, SET, or RETURNING clause, since those clauses require reading from it.
It might be worth adding a column to the chart for this distinction.
The values for chart would be:
Command / SELECT policy - USING (row that already exists) / SELECT POLICY - USING (row being added)
SELECT / ✓ / -
SELECT … FOR UPDATE / ✓ / -
INSERT / - / ✓
INSERT … RETURNING / - / ✓(a)
UPDATE / ✓ / ✓(a)
DELETE / ✓ / -
INSERT … ON CONFLICT / ✓(b) / ✓(b)
UPSERT / ✓(b) / ✓(b)
For UPSERT and it's variants, we have slightly different behaviour. So, I added (b) which means:
- (b) A USING policy failure causes the statement to fail. Normally, USING filters out rows silently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes DOC-13890