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

Commit9dbc6d2

Browse files
committed
Minor code review for tuple slot rewrite.
Avoid creating transiently-inconsistent slot states where possible,by not setting TTS_FLAG_SHOULDFREE until after the slot actually hasa free'able tuple pointer, and by making sure that we reset tts_nvalidand related derived state before we replace the tuple contents. Thiswould only matter if something were to examine the slot after we'dsuffered some kind of error (e.g. out of memory) while manipulatingthe slot. We typically don't do that, so these changes might just becosmetic --- but even if so, it seems like good future-proofing.Also remove some redundant Asserts, and add a couple for consistency.Back-patch to v12 where all this code was rewritten.Discussion:https://postgr.es/m/16095-c3ff2e5283b8dba5@postgresql.org
1 parentd4d0cd6 commit9dbc6d2

File tree

2 files changed

+45
-41
lines changed

2 files changed

+45
-41
lines changed

‎src/backend/executor/execTuples.c

Lines changed: 43 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
*At ExecutorStart()
2121
*----------------
22-
22+
*
2323
*- ExecInitSeqScan() calls ExecInitScanTupleSlot() to construct a
2424
* TupleTableSlots for the tuples returned by the access method, and
2525
* ExecInitResultTypeTL() to define the node's return
@@ -272,7 +272,6 @@ tts_virtual_copy_heap_tuple(TupleTableSlot *slot)
272272
returnheap_form_tuple(slot->tts_tupleDescriptor,
273273
slot->tts_values,
274274
slot->tts_isnull);
275-
276275
}
277276

278277
staticMinimalTuple
@@ -334,6 +333,8 @@ tts_heap_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
334333
{
335334
HeapTupleTableSlot*hslot= (HeapTupleTableSlot*)slot;
336335

336+
Assert(!TTS_EMPTY(slot));
337+
337338
returnheap_getsysattr(hslot->tuple,attnum,
338339
slot->tts_tupleDescriptor,isnull);
339340
}
@@ -346,14 +347,19 @@ tts_heap_materialize(TupleTableSlot *slot)
346347

347348
Assert(!TTS_EMPTY(slot));
348349

349-
/*This slot hasit's tuple already materialized. Nothing to do. */
350+
/*If slot hasits tuple already materialized, nothing to do. */
350351
if (TTS_SHOULDFREE(slot))
351352
return;
352353

353-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
354-
355354
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
356355

356+
/*
357+
* Have to deform from scratch, otherwise tts_values[] entries could point
358+
* into the non-materialized tuple (which might be gone when accessed).
359+
*/
360+
slot->tts_nvalid=0;
361+
hslot->off=0;
362+
357363
if (!hslot->tuple)
358364
hslot->tuple=heap_form_tuple(slot->tts_tupleDescriptor,
359365
slot->tts_values,
@@ -368,12 +374,7 @@ tts_heap_materialize(TupleTableSlot *slot)
368374
hslot->tuple=heap_copytuple(hslot->tuple);
369375
}
370376

371-
/*
372-
* Have to deform from scratch, otherwise tts_values[] entries could point
373-
* into the non-materialized tuple (which might be gone when accessed).
374-
*/
375-
slot->tts_nvalid=0;
376-
hslot->off=0;
377+
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
377378

378379
MemoryContextSwitchTo(oldContext);
379380
}
@@ -436,7 +437,7 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree)
436437
slot->tts_nvalid=0;
437438
hslot->tuple=tuple;
438439
hslot->off=0;
439-
slot->tts_flags &= ~TTS_FLAG_EMPTY;
440+
slot->tts_flags &= ~(TTS_FLAG_EMPTY |TTS_FLAG_SHOULDFREE);
440441
slot->tts_tid=tuple->t_self;
441442

442443
if (shouldFree)
@@ -509,13 +510,19 @@ tts_minimal_materialize(TupleTableSlot *slot)
509510

510511
Assert(!TTS_EMPTY(slot));
511512

512-
/*This slot hasit's tuple already materialized. Nothing to do. */
513+
/*If slot hasits tuple already materialized, nothing to do. */
513514
if (TTS_SHOULDFREE(slot))
514515
return;
515516

516-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
517517
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
518518

519+
/*
520+
* Have to deform from scratch, otherwise tts_values[] entries could point
521+
* into the non-materialized tuple (which might be gone when accessed).
522+
*/
523+
slot->tts_nvalid=0;
524+
mslot->off=0;
525+
519526
if (!mslot->mintuple)
520527
{
521528
mslot->mintuple=heap_form_minimal_tuple(slot->tts_tupleDescriptor,
@@ -532,19 +539,14 @@ tts_minimal_materialize(TupleTableSlot *slot)
532539
mslot->mintuple=heap_copy_minimal_tuple(mslot->mintuple);
533540
}
534541

542+
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
543+
535544
Assert(mslot->tuple==&mslot->minhdr);
536545

537546
mslot->minhdr.t_len=mslot->mintuple->t_len+MINIMAL_TUPLE_OFFSET;
538547
mslot->minhdr.t_data= (HeapTupleHeader) ((char*)mslot->mintuple-MINIMAL_TUPLE_OFFSET);
539548

540549
MemoryContextSwitchTo(oldContext);
541-
542-
/*
543-
* Have to deform from scratch, otherwise tts_values[] entries could point
544-
* into the non-materialized tuple (which might be gone when accessed).
545-
*/
546-
slot->tts_nvalid=0;
547-
mslot->off=0;
548550
}
549551

550552
staticvoid
@@ -615,8 +617,6 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree
615617

616618
if (shouldFree)
617619
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
618-
else
619-
Assert(!TTS_SHOULDFREE(slot));
620620
}
621621

622622

@@ -651,8 +651,6 @@ tts_buffer_heap_clear(TupleTableSlot *slot)
651651

652652
heap_freetuple(bslot->base.tuple);
653653
slot->tts_flags &= ~TTS_FLAG_SHOULDFREE;
654-
655-
Assert(!BufferIsValid(bslot->buffer));
656654
}
657655

658656
if (BufferIsValid(bslot->buffer))
@@ -681,6 +679,8 @@ tts_buffer_heap_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
681679
{
682680
BufferHeapTupleTableSlot*bslot= (BufferHeapTupleTableSlot*)slot;
683681

682+
Assert(!TTS_EMPTY(slot));
683+
684684
returnheap_getsysattr(bslot->base.tuple,attnum,
685685
slot->tts_tupleDescriptor,isnull);
686686
}
@@ -693,14 +693,19 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
693693

694694
Assert(!TTS_EMPTY(slot));
695695

696-
/* If already materialized nothing to do. */
696+
/* Ifslot has its tuplealready materialized, nothing to do. */
697697
if (TTS_SHOULDFREE(slot))
698698
return;
699699

700-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
701-
702700
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
703701

702+
/*
703+
* Have to deform from scratch, otherwise tts_values[] entries could point
704+
* into the non-materialized tuple (which might be gone when accessed).
705+
*/
706+
bslot->base.off=0;
707+
slot->tts_nvalid=0;
708+
704709
if (!bslot->base.tuple)
705710
{
706711
/*
@@ -713,7 +718,6 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
713718
bslot->base.tuple=heap_form_tuple(slot->tts_tupleDescriptor,
714719
slot->tts_values,
715720
slot->tts_isnull);
716-
717721
}
718722
else
719723
{
@@ -723,19 +727,21 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
723727
* A heap tuple stored in a BufferHeapTupleTableSlot should have a
724728
* buffer associated with it, unless it's materialized or virtual.
725729
*/
726-
Assert(BufferIsValid(bslot->buffer));
727730
if (likely(BufferIsValid(bslot->buffer)))
728731
ReleaseBuffer(bslot->buffer);
729732
bslot->buffer=InvalidBuffer;
730733
}
731-
MemoryContextSwitchTo(oldContext);
732734

733735
/*
734-
* Have to deform from scratch, otherwise tts_values[] entries could point
735-
* into the non-materialized tuple (which might be gone when accessed).
736+
* We don't set TTS_FLAG_SHOULDFREE until after releasing the buffer, if
737+
* any. This avoids having a transient state that would fall foul of our
738+
* assertions that a slot with TTS_FLAG_SHOULDFREE doesn't own a buffer.
739+
* In the unlikely event that ReleaseBuffer() above errors out, we'd
740+
* effectively leak the copied tuple, but that seems fairly harmless.
736741
*/
737-
bslot->base.off=0;
738-
slot->tts_nvalid=0;
742+
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
743+
744+
MemoryContextSwitchTo(oldContext);
739745
}
740746

741747
staticvoid
@@ -756,10 +762,10 @@ tts_buffer_heap_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
756762
MemoryContextoldContext;
757763

758764
ExecClearTuple(dstslot);
759-
dstslot->tts_flags |=TTS_FLAG_SHOULDFREE;
760765
dstslot->tts_flags &= ~TTS_FLAG_EMPTY;
761766
oldContext=MemoryContextSwitchTo(dstslot->tts_mcxt);
762767
bdstslot->base.tuple=ExecCopySlotHeapTuple(srcslot);
768+
dstslot->tts_flags |=TTS_FLAG_SHOULDFREE;
763769
MemoryContextSwitchTo(oldContext);
764770
}
765771
else
@@ -1444,10 +1450,10 @@ ExecForceStoreHeapTuple(HeapTuple tuple,
14441450
BufferHeapTupleTableSlot*bslot= (BufferHeapTupleTableSlot*)slot;
14451451

14461452
ExecClearTuple(slot);
1447-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
14481453
slot->tts_flags &= ~TTS_FLAG_EMPTY;
14491454
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
14501455
bslot->base.tuple=heap_copytuple(tuple);
1456+
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
14511457
MemoryContextSwitchTo(oldContext);
14521458

14531459
if (shouldFree)
@@ -1856,7 +1862,6 @@ slot_getmissingattrs(TupleTableSlot *slot, int startAttNum, int lastAttNum)
18561862
slot->tts_values[missattnum]=attrmiss[missattnum].am_value;
18571863
slot->tts_isnull[missattnum]= !attrmiss[missattnum].am_present;
18581864
}
1859-
18601865
}
18611866
}
18621867

‎src/include/executor/tuptable.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,8 @@ typedef struct BufferHeapTupleTableSlot
261261
/*
262262
* If buffer is not InvalidBuffer, then the slot is holding a pin on the
263263
* indicated buffer page; drop the pin when we release the slot's
264-
* reference to that buffer. (TTS_FLAG_SHOULDFREE should not be set be
265-
* false in such a case, since presumably tts_tuple is pointing at the
266-
* buffer page.)
264+
* reference to that buffer. (TTS_FLAG_SHOULDFREE should not be set in
265+
* such a case, since presumably tts_tuple is pointing into the buffer.)
267266
*/
268267
Bufferbuffer;/* tuple's buffer, or InvalidBuffer */
269268
}BufferHeapTupleTableSlot;

0 commit comments

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