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

Commit22e44e8

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 parentff43b3e commit22e44e8

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
@@ -273,7 +273,6 @@ tts_virtual_copy_heap_tuple(TupleTableSlot *slot)
273273
returnheap_form_tuple(slot->tts_tupleDescriptor,
274274
slot->tts_values,
275275
slot->tts_isnull);
276-
277276
}
278277

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

337+
Assert(!TTS_EMPTY(slot));
338+
338339
returnheap_getsysattr(hslot->tuple,attnum,
339340
slot->tts_tupleDescriptor,isnull);
340341
}
@@ -347,14 +348,19 @@ tts_heap_materialize(TupleTableSlot *slot)
347348

348349
Assert(!TTS_EMPTY(slot));
349350

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

354-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
355-
356355
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
357356

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

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

379380
MemoryContextSwitchTo(oldContext);
380381
}
@@ -437,7 +438,7 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree)
437438
slot->tts_nvalid=0;
438439
hslot->tuple=tuple;
439440
hslot->off=0;
440-
slot->tts_flags &= ~TTS_FLAG_EMPTY;
441+
slot->tts_flags &= ~(TTS_FLAG_EMPTY |TTS_FLAG_SHOULDFREE);
441442
slot->tts_tid=tuple->t_self;
442443

443444
if (shouldFree)
@@ -510,13 +511,19 @@ tts_minimal_materialize(TupleTableSlot *slot)
510511

511512
Assert(!TTS_EMPTY(slot));
512513

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

517-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
518518
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
519519

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

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

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

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

551553
staticvoid
@@ -616,8 +618,6 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree
616618

617619
if (shouldFree)
618620
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
619-
else
620-
Assert(!TTS_SHOULDFREE(slot));
621621
}
622622

623623

@@ -652,8 +652,6 @@ tts_buffer_heap_clear(TupleTableSlot *slot)
652652

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

659657
if (BufferIsValid(bslot->buffer))
@@ -682,6 +680,8 @@ tts_buffer_heap_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
682680
{
683681
BufferHeapTupleTableSlot*bslot= (BufferHeapTupleTableSlot*)slot;
684682

683+
Assert(!TTS_EMPTY(slot));
684+
685685
returnheap_getsysattr(bslot->base.tuple,attnum,
686686
slot->tts_tupleDescriptor,isnull);
687687
}
@@ -694,14 +694,19 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
694694

695695
Assert(!TTS_EMPTY(slot));
696696

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

701-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
702-
703701
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
704702

703+
/*
704+
* Have to deform from scratch, otherwise tts_values[] entries could point
705+
* into the non-materialized tuple (which might be gone when accessed).
706+
*/
707+
bslot->base.off=0;
708+
slot->tts_nvalid=0;
709+
705710
if (!bslot->base.tuple)
706711
{
707712
/*
@@ -714,7 +719,6 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
714719
bslot->base.tuple=heap_form_tuple(slot->tts_tupleDescriptor,
715720
slot->tts_values,
716721
slot->tts_isnull);
717-
718722
}
719723
else
720724
{
@@ -724,19 +728,21 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
724728
* A heap tuple stored in a BufferHeapTupleTableSlot should have a
725729
* buffer associated with it, unless it's materialized or virtual.
726730
*/
727-
Assert(BufferIsValid(bslot->buffer));
728731
if (likely(BufferIsValid(bslot->buffer)))
729732
ReleaseBuffer(bslot->buffer);
730733
bslot->buffer=InvalidBuffer;
731734
}
732-
MemoryContextSwitchTo(oldContext);
733735

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

742748
staticvoid
@@ -757,10 +763,10 @@ tts_buffer_heap_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
757763
MemoryContextoldContext;
758764

759765
ExecClearTuple(dstslot);
760-
dstslot->tts_flags |=TTS_FLAG_SHOULDFREE;
761766
dstslot->tts_flags &= ~TTS_FLAG_EMPTY;
762767
oldContext=MemoryContextSwitchTo(dstslot->tts_mcxt);
763768
bdstslot->base.tuple=ExecCopySlotHeapTuple(srcslot);
769+
dstslot->tts_flags |=TTS_FLAG_SHOULDFREE;
764770
MemoryContextSwitchTo(oldContext);
765771
}
766772
else
@@ -1445,10 +1451,10 @@ ExecForceStoreHeapTuple(HeapTuple tuple,
14451451
BufferHeapTupleTableSlot*bslot= (BufferHeapTupleTableSlot*)slot;
14461452

14471453
ExecClearTuple(slot);
1448-
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
14491454
slot->tts_flags &= ~TTS_FLAG_EMPTY;
14501455
oldContext=MemoryContextSwitchTo(slot->tts_mcxt);
14511456
bslot->base.tuple=heap_copytuple(tuple);
1457+
slot->tts_flags |=TTS_FLAG_SHOULDFREE;
14521458
MemoryContextSwitchTo(oldContext);
14531459

14541460
if (shouldFree)
@@ -1857,7 +1863,6 @@ slot_getmissingattrs(TupleTableSlot *slot, int startAttNum, int lastAttNum)
18571863
slot->tts_values[missattnum]=attrmiss[missattnum].am_value;
18581864
slot->tts_isnull[missattnum]= !attrmiss[missattnum].am_present;
18591865
}
1860-
18611866
}
18621867
}
18631868

‎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