Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6572bd5

Browse files
committed
Prevent RLS filters on ctid from breaking WHERE CURRENT OF <cursor>.
The executor only supports CurrentOfExpr as the sole tidqual of aTidScan plan node. tidpath.c failed to take any particular care aboutthat, but would just take the first ctid equality qual it could findin the target relation's baserestrictinfo list. Originally that wasfine because the grammar prevents any other WHERE conditions frombeing combined with CURRENT OF <cursor>. However, if the relation hasRLS visibility policies then those would get included in the list.Should such a policy include a condition on ctid, we'd typically grabthe wrong qual and produce a malfunctioning plan.To fix, introduce a simplistic priority ordering scheme for which ctidequality qual to prefer. Real-world cases involving more than onesuch qual are so rare that it doesn't seem worth going to any greattrouble to choose one over another, so I didn't work very hard; butthis code could be extended in future if someone thinks differently.It's extremely difficult to think of a reasonable use-case for an RLSrestriction involving ctid, and certainly we've heard no field reportsof this failure. So this doesn't seem worthy of back-patching, butin the name of cleanliness let's fix it going forward.Patch by me, per report from Robert Haas.Discussion:https://postgr.es/m/3914881.1715038270@sss.pgh.pa.us
1 parent09fe965 commit6572bd5

File tree

3 files changed

+118
-30
lines changed

3 files changed

+118
-30
lines changed

‎src/backend/optimizer/path/tidpath.c

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -225,42 +225,37 @@ IsCurrentOfClause(RestrictInfo *rinfo, RelOptInfo *rel)
225225
}
226226

227227
/*
228-
* Extract a set of CTID conditions from the given RestrictInfo
229-
*
230-
* Returns a List of CTID qual RestrictInfos for the specified rel (with
231-
* implicit OR semantics across the list), or NIL if there are no usable
232-
* conditions.
228+
* Is the RestrictInfo usable as a CTID qual for the specified rel?
233229
*
234230
* This function considers only base cases; AND/OR combination is handled
235-
* below. Therefore the returned List never has more than one element.
236-
* (Using a List may seem a bit weird, but it simplifies the caller.)
231+
* below.
237232
*/
238-
staticList*
239-
TidQualFromRestrictInfo(PlannerInfo*root,RestrictInfo*rinfo,RelOptInfo*rel)
233+
staticbool
234+
RestrictInfoIsTidQual(PlannerInfo*root,RestrictInfo*rinfo,RelOptInfo*rel)
240235
{
241236
/*
242237
* We may ignore pseudoconstant clauses (they can't contain Vars, so could
243238
* not match anyway).
244239
*/
245240
if (rinfo->pseudoconstant)
246-
returnNIL;
241+
returnfalse;
247242

248243
/*
249244
* If clause must wait till after some lower-security-level restriction
250245
* clause, reject it.
251246
*/
252247
if (!restriction_is_securely_promotable(rinfo,rel))
253-
returnNIL;
248+
returnfalse;
254249

255250
/*
256-
* Check all base cases. If we get a match, return the clause.
251+
* Check all base cases.
257252
*/
258253
if (IsTidEqualClause(rinfo,rel)||
259254
IsTidEqualAnyClause(root,rinfo,rel)||
260255
IsCurrentOfClause(rinfo,rel))
261-
returnlist_make1(rinfo);
256+
returntrue;
262257

263-
returnNIL;
258+
returnfalse;
264259
}
265260

266261
/*
@@ -270,12 +265,22 @@ TidQualFromRestrictInfo(PlannerInfo *root, RestrictInfo *rinfo, RelOptInfo *rel)
270265
* implicit OR semantics across the list), or NIL if there are no usable
271266
* equality conditions.
272267
*
273-
* This function is just concerned with handling AND/OR recursion.
268+
* This function is mainly concerned with handling AND/OR recursion.
269+
* However, we do have a special rule to enforce: if there is a CurrentOfExpr
270+
* qual, we *must* return that and only that, else the executor may fail.
271+
* Ordinarily a CurrentOfExpr would be all alone anyway because of grammar
272+
* restrictions, but it is possible for RLS quals to appear AND'ed with it.
273+
* It's even possible (if fairly useless) for the RLS quals to be CTID quals.
274+
* So we must scan the whole rlist to see if there's a CurrentOfExpr. Since
275+
* we have to do that, we also apply some very-trivial preference rules about
276+
* which of the other possibilities should be chosen, in the unlikely event
277+
* that there's more than one choice.
274278
*/
275279
staticList*
276280
TidQualFromRestrictInfoList(PlannerInfo*root,List*rlist,RelOptInfo*rel)
277281
{
278-
List*rlst=NIL;
282+
RestrictInfo*tidclause=NULL;/* best simple CTID qual so far */
283+
List*orlist=NIL;/* best OR'ed CTID qual so far */
279284
ListCell*l;
280285

281286
foreach(l,rlist)
@@ -284,6 +289,7 @@ TidQualFromRestrictInfoList(PlannerInfo *root, List *rlist, RelOptInfo *rel)
284289

285290
if (restriction_is_or_clause(rinfo))
286291
{
292+
List*rlst=NIL;
287293
ListCell*j;
288294

289295
/*
@@ -308,7 +314,10 @@ TidQualFromRestrictInfoList(PlannerInfo *root, List *rlist, RelOptInfo *rel)
308314
RestrictInfo*ri=castNode(RestrictInfo,orarg);
309315

310316
Assert(!restriction_is_or_clause(ri));
311-
sublist=TidQualFromRestrictInfo(root,ri,rel);
317+
if (RestrictInfoIsTidQual(root,ri,rel))
318+
sublist=list_make1(ri);
319+
else
320+
sublist=NIL;
312321
}
313322

314323
/*
@@ -326,25 +335,44 @@ TidQualFromRestrictInfoList(PlannerInfo *root, List *rlist, RelOptInfo *rel)
326335
*/
327336
rlst=list_concat(rlst,sublist);
328337
}
338+
339+
if (rlst)
340+
{
341+
/*
342+
* Accept the OR'ed list if it's the first one, or if it's
343+
* shorter than the previous one.
344+
*/
345+
if (orlist==NIL||list_length(rlst)<list_length(orlist))
346+
orlist=rlst;
347+
}
329348
}
330349
else
331350
{
332351
/* Not an OR clause, so handle base cases */
333-
rlst=TidQualFromRestrictInfo(root,rinfo,rel);
334-
}
352+
if (RestrictInfoIsTidQual(root,rinfo,rel))
353+
{
354+
/* We can stop immediately if it's a CurrentOfExpr */
355+
if (IsCurrentOfClause(rinfo,rel))
356+
returnlist_make1(rinfo);
335357

336-
/*
337-
*Stop as soon as we find any usableCTIDcondition.In theory we
338-
*could get CTID equality conditions from different AND'ed clauses,
339-
*in which case we could try to pick the most efficient one. In
340-
* practice, such usage seems very unlikely, so we don't bother; we
341-
* just exit as soon as we find the first candidate.
342-
*/
343-
if (rlst)
344-
break;
358+
/*
359+
*Otherwise, remember the first non-ORCTIDqual.We could
360+
*try to apply some preference order if there's more than
361+
*one, but such usage seems very unlikely, so don't bother.
362+
*/
363+
if (tidclause==NULL)
364+
tidclause=rinfo;
365+
}
366+
}
345367
}
346368

347-
returnrlst;
369+
/*
370+
* Prefer any singleton CTID qual to an OR'ed list. Again, it seems
371+
* unlikely to be worth thinking harder than that.
372+
*/
373+
if (tidclause)
374+
returnlist_make1(tidclause);
375+
returnorlist;
348376
}
349377

350378
/*
@@ -405,7 +433,7 @@ BuildParameterizedTidPaths(PlannerInfo *root, RelOptInfo *rel, List *clauses)
405433
* might find a suitable ScalarArrayOpExpr in the rel's joininfo list,
406434
* but it seems unlikely to be worth expending the cycles to check.
407435
* And we definitely won't find a CurrentOfExpr here. Hence, we don't
408-
* useTidQualFromRestrictInfo; but this must match that function
436+
* useRestrictInfoIsTidQual; but this must match that function
409437
* otherwise.
410438
*/
411439
if (rinfo->pseudoconstant||

‎src/test/regress/expected/rowsecurity.out

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3921,6 +3921,47 @@ SELECT * FROM current_check;
39213921
(1 row)
39223922

39233923
COMMIT;
3924+
-- Check that RLS filters that are tidquals don't override WHERE CURRENT OF
3925+
BEGIN;
3926+
CREATE TABLE current_check_2 (a int, b text);
3927+
INSERT INTO current_check_2 VALUES (1, 'Apple');
3928+
ALTER TABLE current_check_2 ENABLE ROW LEVEL SECURITY;
3929+
ALTER TABLE current_check_2 FORCE ROW LEVEL SECURITY;
3930+
-- policy must accept ctid = (InvalidBlockNumber,0) since updates check it
3931+
-- before assigning a ctid to the new row
3932+
CREATE POLICY p1 ON current_check_2 AS PERMISSIVE
3933+
USING (ctid IN ('(0,1)', '(0,2)', '(4294967295,0)'));
3934+
SELECT ctid, * FROM current_check_2;
3935+
ctid | a | b
3936+
-------+---+-------
3937+
(0,1) | 1 | Apple
3938+
(1 row)
3939+
3940+
DECLARE current_check_cursor CURSOR FOR SELECT * FROM current_check_2;
3941+
FETCH FROM current_check_cursor;
3942+
a | b
3943+
---+-------
3944+
1 | Apple
3945+
(1 row)
3946+
3947+
EXPLAIN (COSTS OFF)
3948+
UPDATE current_check_2 SET b = 'Manzana' WHERE CURRENT OF current_check_cursor;
3949+
QUERY PLAN
3950+
----------------------------------------------------------------------------
3951+
Update on current_check_2
3952+
-> Tid Scan on current_check_2
3953+
TID Cond: CURRENT OF current_check_cursor
3954+
Filter: (ctid = ANY ('{"(0,1)","(0,2)","(4294967295,0)"}'::tid[]))
3955+
(4 rows)
3956+
3957+
UPDATE current_check_2 SET b = 'Manzana' WHERE CURRENT OF current_check_cursor;
3958+
SELECT ctid, * FROM current_check_2;
3959+
ctid | a | b
3960+
-------+---+---------
3961+
(0,2) | 1 | Manzana
3962+
(1 row)
3963+
3964+
ROLLBACK;
39243965
--
39253966
-- check pg_stats view filtering
39263967
--

‎src/test/regress/sql/rowsecurity.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,6 +1726,25 @@ SELECT * FROM current_check;
17261726

17271727
COMMIT;
17281728

1729+
-- Check that RLS filters that are tidquals don't override WHERE CURRENT OF
1730+
BEGIN;
1731+
CREATETABLEcurrent_check_2 (aint, btext);
1732+
INSERT INTO current_check_2VALUES (1,'Apple');
1733+
ALTERTABLE current_check_2 ENABLE ROW LEVEL SECURITY;
1734+
ALTERTABLE current_check_2 FORCE ROW LEVEL SECURITY;
1735+
-- policy must accept ctid = (InvalidBlockNumber,0) since updates check it
1736+
-- before assigning a ctid to the new row
1737+
CREATE POLICY p1ON current_check_2AS PERMISSIVE
1738+
USING (ctidIN ('(0,1)','(0,2)','(4294967295,0)'));
1739+
SELECT ctid,*FROM current_check_2;
1740+
DECLARE current_check_cursor CURSOR FORSELECT*FROM current_check_2;
1741+
FETCHFROM current_check_cursor;
1742+
EXPLAIN (COSTS OFF)
1743+
UPDATE current_check_2SET b='Manzana'WHERE CURRENT OF current_check_cursor;
1744+
UPDATE current_check_2SET b='Manzana'WHERE CURRENT OF current_check_cursor;
1745+
SELECT ctid,*FROM current_check_2;
1746+
ROLLBACK;
1747+
17291748
--
17301749
-- check pg_stats view filtering
17311750
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp