Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f16270a

Browse files
committed
Add locking clause for SB views for update/delete
In expand_security_qual(), we were handling locking correctly when a PlanRowMark existed, but not when we were working with the target relation (which doesn't have any PlanRowMarks, but the subquery created for the security barrier quals still needs to lock the rows under it). Noted by Etsuro Fujita when working with the Postgres FDW, which wasn't properly issuing a SELECT ... FOR UPDATE to the remote side under a DELETE. Back-patch to 9.4 where updatable security barrier views were introduced. Per discussion with Etsuro and Dean Rasheed.
1 parent 2164a0d commit f16270a

File tree

2 files changed

+170
-129
lines changed

2 files changed

+170
-129
lines changed

src/backend/optimizer/prep/prepsecurity.c

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ typedef struct
3737
} security_barrier_replace_vars_context;
3838

3939
static void expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
40-
RangeTblEntry *rte, Node *qual);
40+
RangeTblEntry *rte, Node *qual, bool targetRelation);
4141

4242
static void security_barrier_replace_vars(Node *node,
4343
security_barrier_replace_vars_context *context);
@@ -63,6 +63,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
6363
Query *parse = root->parse;
6464
int rt_index;
6565
ListCell *cell;
66+
bool targetRelation = false;
6667

6768
/*
6869
* Process each RTE in the rtable list.
@@ -98,6 +99,15 @@ expand_security_quals(PlannerInfo *root, List *tlist)
9899
{
99100
RangeTblEntry *newrte = copyObject(rte);
100101

102+
/*
103+
* We need to let expand_security_qual know if this is the target
104+
* relation, as it has additional work to do in that case.
105+
*
106+
* Capture that information here as we're about to replace
107+
* parse->resultRelation.
108+
*/
109+
targetRelation = true;
110+
101111
parse->rtable = lappend(parse->rtable, newrte);
102112
parse->resultRelation = list_length(parse->rtable);
103113

@@ -147,7 +157,8 @@ expand_security_quals(PlannerInfo *root, List *tlist)
147157
rte->securityQuals = list_delete_first(rte->securityQuals);
148158

149159
ChangeVarNodes(qual, rt_index, 1, 0);
150-
expand_security_qual(root, tlist, rt_index, rte, qual);
160+
expand_security_qual(root, tlist, rt_index, rte, qual,
161+
targetRelation);
151162
}
152163
}
153164
}
@@ -160,7 +171,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
160171
*/
161172
static void
162173
expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
163-
RangeTblEntry *rte, Node *qual)
174+
RangeTblEntry *rte, Node *qual, bool targetRelation)
164175
{
165176
Query *parse = root->parse;
166177
Oid relid = rte->relid;
@@ -219,10 +230,11 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
219230
* Now deal with any PlanRowMark on this RTE by requesting a lock
220231
* of the same strength on the RTE copied down to the subquery.
221232
*
222-
* Note that we can't push the user-defined quals down since they
223-
* may included untrusted functions and that means that we will
224-
* end up locking all rows which pass the securityQuals, even if
225-
* those rows don't pass the user-defined quals. This is
233+
* Note that we can only push down user-defined quals if they are
234+
* only using leakproof (and therefore trusted) functions and
235+
* operators. As a result, we may end up locking more rows than
236+
* strictly necessary (and, in the worst case, we could end up
237+
* locking all rows which pass the securityQuals). This is
226238
* currently documented behavior, but it'd be nice to come up with
227239
* a better solution some day.
228240
*/
@@ -255,6 +267,15 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
255267
root->rowMarks = list_delete(root->rowMarks, rc);
256268
}
257269

270+
/*
271+
* When we are replacing the target relation with a subquery, we
272+
* need to make sure to add a locking clause explicitly to the
273+
* generated subquery since there won't be any row marks against
274+
* the target relation itself.
275+
*/
276+
if (targetRelation)
277+
applyLockingClause(subquery, 1, LCS_FORUPDATE,
278+
false, false);
258279
/*
259280
* Replace any variables in the outer query that refer to the
260281
* original relation RTE with references to columns that we will

0 commit comments

Comments
 (0)