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

Commitf21fada

Browse files
committed
Avoid detoasting failure after COMMIT inside a plpgsql FOR loop.
exec_for_query() normally tries to prefetch a few rows at a timefrom the query being iterated over, so as to reduce executorentry/exit overhead. Unfortunately this is unsafe if we haveCOMMIT or ROLLBACK within the loop, because there might beTOAST references in the data that we prefetched but haven'tyet examined. Immediately after the COMMIT/ROLLBACK, we haveno snapshots in the session, meaning that VACUUM is at libertyto remove recently-deleted TOAST rows.This was originally reported as a case triggering the "no knownsnapshots" error in init_toast_snapshot(), but even if you misshitting that, you can get "missing toast chunk", as illustratedby the added isolation test case.To fix, just disable prefetching in non-atomic contexts. Maybethere will be performance complaints prompting us to work harderlater, but it's not clear at the moment that this really costsmuch, and I doubt we'd want to back-patch any complicated fix.In passing, adjust that error message in init_toast_snapshot()to be a little clearer about the likely cause of the problem.Patch by me, based on earlier investigation by Konstantin Knizhnik.Per bug #15990 from Andreas Wicht. Back-patch to v11 whereintra-procedure COMMIT was added.Discussion:https://postgr.es/m/15990-eee2ac466b11293d@postgresql.org
1 parent4f586fe commitf21fada

File tree

4 files changed

+88
-1
lines changed

4 files changed

+88
-1
lines changed

‎src/backend/access/common/toast_internals.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,8 +638,21 @@ init_toast_snapshot(Snapshot toast_snapshot)
638638
{
639639
Snapshotsnapshot=GetOldestSnapshot();
640640

641+
/*
642+
* GetOldestSnapshot returns NULL if the session has no active snapshots.
643+
* We can get that if, for example, a procedure fetches a toasted value
644+
* into a local variable, commits, and then tries to detoast the value.
645+
* Such coding is unsafe, because once we commit there is nothing to
646+
* prevent the toast data from being deleted. Detoasting *must* happen in
647+
* the same transaction that originally fetched the toast pointer. Hence,
648+
* rather than trying to band-aid over the problem, throw an error. (This
649+
* is not very much protection, because in many scenarios the procedure
650+
* would have already created a new transaction snapshot, preventing us
651+
* from detecting the problem. But it's better than nothing, and for sure
652+
* we shouldn't expend code on masking the problem more.)
653+
*/
641654
if (snapshot==NULL)
642-
elog(ERROR,"no known snapshots");
655+
elog(ERROR,"cannot fetch toast data without an active snapshot");
643656

644657
InitToastSnapshot(*toast_snapshot,snapshot->lsn,snapshot->whenTaken);
645658
}

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5826,6 +5826,17 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
58265826
*/
58275827
PinPortal(portal);
58285828

5829+
/*
5830+
* In a non-atomic context, we dare not prefetch, even if it would
5831+
* otherwise be safe. Aside from any semantic hazards that that might
5832+
* create, if we prefetch toasted data and then the user commits the
5833+
* transaction, the toast references could turn into dangling pointers.
5834+
* (Rows we haven't yet fetched from the cursor are safe, because the
5835+
* PersistHoldablePortal mechanism handles this scenario.)
5836+
*/
5837+
if (!estate->atomic)
5838+
prefetch_ok= false;
5839+
58295840
/*
58305841
* Fetch the initial tuple(s). If prefetching is allowed then we grab a
58315842
* few more rows to avoid multiple trips through executor startup

‎src/test/isolation/expected/plpgsql-toast.out

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,46 @@ pg_advisory_unlock
192192
t
193193
s1: NOTICE: length(r) = 6002
194194
step assign5: <... completed>
195+
196+
starting permutation: lock assign6 vacuum unlock
197+
pg_advisory_unlock_all
198+
199+
200+
pg_advisory_unlock_all
201+
202+
203+
step lock:
204+
SELECT pg_advisory_lock(1);
205+
206+
pg_advisory_lock
207+
208+
209+
step assign6:
210+
do $$
211+
declare
212+
r record;
213+
begin
214+
insert into test1 values (2, repeat('bar', 3000));
215+
insert into test1 values (3, repeat('baz', 4000));
216+
for r in select test1.b from test1 loop
217+
delete from test1;
218+
commit;
219+
perform pg_advisory_lock(1);
220+
raise notice 'length(r) = %', length(r::text);
221+
end loop;
222+
end;
223+
$$;
224+
<waiting ...>
225+
step vacuum:
226+
VACUUM test1;
227+
228+
step unlock:
229+
SELECT pg_advisory_unlock(1);
230+
231+
pg_advisory_unlock
232+
233+
t
234+
s1: NOTICE: length(r) = 6002
235+
s1: NOTICE: length(r) = 9002
236+
s1: NOTICE: length(r) = 12002
237+
step assign6: <... completed>

‎src/test/isolation/specs/plpgsql-toast.spec

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,25 @@ do $$
112112
$$;
113113
}
114114

115+
# FOR loop must not hold any fetched-but-not-detoasted values across commit
116+
step"assign6"
117+
{
118+
do $$
119+
declare
120+
rrecord;
121+
begin
122+
insertintotest1values(2,repeat('bar',3000));
123+
insertintotest1values(3,repeat('baz',4000));
124+
forrinselecttest1.bfromtest1loop
125+
deletefromtest1;
126+
commit;
127+
performpg_advisory_lock(1);
128+
raisenotice'length(r) = %',length(r::text);
129+
endloop;
130+
end;
131+
$$;
132+
}
133+
115134
session"s2"
116135
setup
117136
{
@@ -135,3 +154,4 @@ permutation "lock" "assign2" "vacuum" "unlock"
135154
permutation"lock""assign3""vacuum""unlock"
136155
permutation"lock""assign4""vacuum""unlock"
137156
permutation"lock""assign5""vacuum""unlock"
157+
permutation"lock""assign6""vacuum""unlock"

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp