Skip to content

Commit 075aade

Browse files
feodorfeodor
committed
Adjust INCLUDE index truncation comments and code.
Add several assertions that ensure that we're dealing with a pivot tuple without non-key attributes where that's expected. Also, remove the assertion within _bt_isequal(), restoring the v10 function signature. A similar check will be performed for the page highkey within _bt_moveright() in most cases. Also avoid dropping all objects within regression tests, to increase pg_dump test coverage for INCLUDE indexes. Rather than using infrastructure that's generally intended to be used with reference counted heap tuple descriptors during truncation, use the same function that was introduced to store flat TupleDescs in shared memory (we use a temp palloc'd buffer). This isn't strictly necessary, but seems more future-proof than the old approach. It also lets us avoid including rel.h within indextuple.c, which was arguably a modularity violation. Also, we now call index_deform_tuple() with the truncated TupleDesc, not the source TupleDesc, since that's more robust, and saves a few cycles. In passing, fix a memory leak by pfree'ing truncated pivot tuple memory during CREATE INDEX. Also pfree during a page split, just to be consistent. Refactor _bt_check_natts() to be more readable. Author: Peter Geoghegan with some editorization by me Reviewed by: Alexander Korotkov, Teodor Sigaev Discussion: https://www.postgresql.org/message-id/CAH2-Wz%3DkCWuXeMrBCopC-tFs3FbiVxQNjjgNKdG2sHxZ5k2y3w%40mail.gmail.com
1 parent 5372c2c commit 075aade

File tree

13 files changed

+429
-330
lines changed

13 files changed

+429
-330
lines changed

‎contrib/amcheck/verify_nbtree.c

Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,9 @@ bt_check_level_from_leftmost(BtreeCheckState *state, BtreeLevel level)
698698
* "real" data item on the page to the right (if such a first item is
699699
* available).
700700
*
701+
* - That tuples report that they have the expected number of attributes.
702+
* INCLUDE index pivot tuples should not contain non-key attributes.
703+
*
701704
* Furthermore, when state passed shows ShareLock held, and target page is
702705
* internal page, function also checks:
703706
*
@@ -722,43 +725,35 @@ bt_target_page_check(BtreeCheckState *state)
722725
elog(DEBUG2, "verifying %u items on %s block %u", max,
723726
P_ISLEAF(topaque) ? "leaf" : "internal", state->targetblock);
724727

725-
726-
/* Check the number of attributes in high key if any */
727-
if (!P_RIGHTMOST(topaque))
728+
/*
729+
* Check the number of attributes in high key. Note, rightmost page doesn't
730+
* contain a high key, so nothing to check
731+
*/
732+
if (!P_RIGHTMOST(topaque) &&
733+
!_bt_check_natts(state->rel, state->target, P_HIKEY))
728734
{
729-
if (!_bt_check_natts(state->rel, state->target, P_HIKEY))
730-
{
731-
ItemId itemid;
732-
IndexTuple itup;
733-
char *itid,
734-
*htid;
735+
ItemId itemid;
736+
IndexTuple itup;
735737

736-
itemid = PageGetItemId(state->target, P_HIKEY);
737-
itup = (IndexTuple) PageGetItem(state->target, itemid);
738-
itid = psprintf("(%u,%u)", state->targetblock, P_HIKEY);
739-
htid = psprintf("(%u,%u)",
740-
ItemPointerGetBlockNumberNoCheck(&(itup->t_tid)),
741-
ItemPointerGetOffsetNumberNoCheck(&(itup->t_tid)));
738+
itemid = PageGetItemId(state->target, P_HIKEY);
739+
itup = (IndexTuple) PageGetItem(state->target, itemid);
742740

743-
ereport(ERROR,
744-
(errcode(ERRCODE_INDEX_CORRUPTED),
745-
errmsg("wrong number of index tuple attributes for index \"%s\"",
746-
RelationGetRelationName(state->rel)),
747-
errdetail_internal("Index tid=%s natts=%u points to %s tid=%s page lsn=%X/%X.",
748-
itid,
749-
BTreeTupGetNAtts(itup, state->rel),
750-
P_ISLEAF(topaque) ? "heap" : "index",
751-
htid,
752-
(uint32) (state->targetlsn >> 32),
753-
(uint32) state->targetlsn)));
754-
}
741+
ereport(ERROR,
742+
(errcode(ERRCODE_INDEX_CORRUPTED),
743+
errmsg("wrong number of high key index tuple attributes in index \"%s\"",
744+
RelationGetRelationName(state->rel)),
745+
errdetail_internal("Index block=%u natts=%u block type=%s page lsn=%X/%X.",
746+
state->targetblock,
747+
BTreeTupleGetNAtts(itup, state->rel),
748+
P_ISLEAF(topaque) ? "heap" : "index",
749+
(uint32) (state->targetlsn >> 32),
750+
(uint32) state->targetlsn)));
755751
}
756752

757-
758753
/*
759754
* Loop over page items, starting from first non-highkey item, not high
760-
* key (if any). Also, immediately skip "negative infinity" real item (if
761-
* any).
755+
* key (if any). Most tests are not performed for the "negative infinity"
756+
* real item (if any).
762757
*/
763758
for (offset = P_FIRSTDATAKEY(topaque);
764759
offset <= max;
@@ -791,7 +786,7 @@ bt_target_page_check(BtreeCheckState *state)
791786
tupsize, ItemIdGetLength(itemid),
792787
(uint32) (state->targetlsn >> 32),
793788
(uint32) state->targetlsn),
794-
errhint("This could be a torn page problem")));
789+
errhint("This could be a torn page problem.")));
795790

796791
/* Check the number of index tuple attributes */
797792
if (!_bt_check_natts(state->rel, state->target, offset))
@@ -806,20 +801,20 @@ bt_target_page_check(BtreeCheckState *state)
806801

807802
ereport(ERROR,
808803
(errcode(ERRCODE_INDEX_CORRUPTED),
809-
errmsg("wrong number of index tuple attributes for index \"%s\"",
804+
errmsg("wrong number of index tuple attributes in index \"%s\"",
810805
RelationGetRelationName(state->rel)),
811806
errdetail_internal("Index tid=%s natts=%u points to %s tid=%s page lsn=%X/%X.",
812807
itid,
813-
BTreeTupGetNAtts(itup, state->rel),
808+
BTreeTupleGetNAtts(itup, state->rel),
814809
P_ISLEAF(topaque) ? "heap" : "index",
815810
htid,
816811
(uint32) (state->targetlsn >> 32),
817812
(uint32) state->targetlsn)));
818813
}
819814

820815
/*
821-
* Don't try to generate scankey using "negative infinity" garbage
822-
* data on internal pages
816+
* Don't try to generate scankey using "negative infinity" item on
817+
* internal pages. They are always truncated to zero attributes.
823818
*/
824819
if (offset_is_negative_infinity(topaque, offset))
825820
continue;
@@ -1430,9 +1425,9 @@ offset_is_negative_infinity(BTPageOpaque opaque, OffsetNumber offset)
14301425
* infinity item is either first or second line item, or there is none
14311426
* within page.
14321427
*
1433-
* "Negative infinity" tuple is a special corner case of pivot tuples,
1434-
* it has zero attributes while rest of pivot tuples have nkeyatts number
1435-
* of attributes.
1428+
* Negative infinity items are a special case among pivot tuples. They
1429+
* always have zero attributes, while all other pivot tuples always have
1430+
* nkeyatts attributes.
14361431
*
14371432
* Right-most pages don't have a high key, but could be said to
14381433
* conceptually have a "positive infinity" high key. Thus, there is a

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

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include "access/heapam.h"
2020
#include "access/itup.h"
2121
#include "access/tuptoaster.h"
22-
#include "utils/rel.h"
2322

2423

2524
/* ----------------------------------------------------------------
@@ -32,6 +31,9 @@
3231
*
3332
* This shouldn't leak any memory; otherwise, callers such as
3433
* tuplesort_putindextuplevalues() will be very unhappy.
34+
*
35+
* This shouldn't perform external table access provided caller
36+
* does not pass values that are stored EXTERNAL.
3537
* ----------------
3638
*/
3739
IndexTuple
@@ -448,30 +450,49 @@ CopyIndexTuple(IndexTuple source)
448450
}
449451

450452
/*
451-
* Truncate tailing attributes from given index tuple leaving it with
452-
* new_indnatts number of attributes.
453+
* Create a palloc'd copy of an index tuple, leaving only the first
454+
* leavenatts attributes remaining.
455+
*
456+
* Truncation is guaranteed to result in an index tuple that is no
457+
* larger than the original. It is safe to use the IndexTuple with
458+
* the original tuple descriptor, but caller must avoid actually
459+
* accessing truncated attributes from returned tuple! In practice
460+
* this means that index_getattr() must be called with special care,
461+
* and that the truncated tuple should only ever be accessed by code
462+
* under caller's direct control.
463+
*
464+
* It's safe to call this function with a buffer lock held, since it
465+
* never performs external table access. If it ever became possible
466+
* for index tuples to contain EXTERNAL TOAST values, then this would
467+
* have to be revisited.
453468
*/
454469
IndexTuple
455-
index_truncate_tuple(TupleDesc tupleDescriptor, IndexTuple olditup,
456-
int new_indnatts)
470+
index_truncate_tuple(TupleDesc sourceDescriptor, IndexTuple source,
471+
int leavenatts)
457472
{
458-
TupleDesc itupdesc = CreateTupleDescCopyConstr(tupleDescriptor);
473+
TupleDesc truncdesc;
459474
Datum values[INDEX_MAX_KEYS];
460475
bool isnull[INDEX_MAX_KEYS];
461-
IndexTuple newitup;
476+
IndexTuple truncated;
462477

463-
Assert(tupleDescriptor->natts <= INDEX_MAX_KEYS);
464-
Assert(new_indnatts > 0);
465-
Assert(new_indnatts < tupleDescriptor->natts);
478+
Assert(leavenatts < sourceDescriptor->natts);
466479

467-
index_deform_tuple(olditup, tupleDescriptor, values, isnull);
480+
/* Create temporary descriptor to scribble on */
481+
truncdesc = palloc(TupleDescSize(sourceDescriptor));
482+
TupleDescCopy(truncdesc, sourceDescriptor);
483+
truncdesc->natts = leavenatts;
468484

469-
/* form new tuple that will contain only key attributes */
470-
itupdesc->natts = new_indnatts;
471-
newitup = index_form_tuple(itupdesc, values, isnull);
472-
newitup->t_tid = olditup->t_tid;
485+
/* Deform, form copy of tuple with fewer attributes */
486+
index_deform_tuple(source, truncdesc, values, isnull);
487+
truncated = index_form_tuple(truncdesc, values, isnull);
488+
truncated->t_tid = source->t_tid;
489+
Assert(IndexTupleSize(truncated) <= IndexTupleSize(source));
490+
491+
/*
492+
* Cannot leak memory here, TupleDescCopy() doesn't allocate any
493+
* inner structure, so, plain pfree() should clean all allocated memory
494+
*/
495+
pfree(truncdesc);
473496

474-
FreeTupleDesc(itupdesc);
475-
Assert(IndexTupleSize(newitup) <= IndexTupleSize(olditup));
476-
return newitup;
497+
return truncated;
477498
}

0 commit comments

Comments
 (0)