From 4db1d9b56b28ea81c542d2276c3b494a86eb75dc Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 4 Aug 2017 15:06:29 -0700 Subject: [PATCH 12/16] Centralize slot deforming logic a bit. Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/access/common/heaptuple.c | 148 +++++++++++----------------------- 1 file changed, 47 insertions(+), 101 deletions(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index 13ee528e26..f77ea477fb 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -1046,6 +1046,7 @@ slot_deform_tuple(TupleTableSlot *slot, int natts) long off; /* offset in tuple data */ bits8 *bp = tup->t_bits; /* ptr to null bitmap in tuple */ bool slow; /* can we use/set attcacheoff? */ + int valnatts = natts; /* * Check whether the first call for this tuple, and initialize or restore @@ -1065,6 +1066,9 @@ slot_deform_tuple(TupleTableSlot *slot, int natts) slow = slot->tts_slow; } + + natts = Min(natts, Min(HeapTupleHeaderGetNatts(tuple->t_data), slot->tts_tupleDescriptor->natts)); + tp = (char *) tup + tup->t_hoff; for (; attnum < natts; attnum++) @@ -1118,10 +1122,16 @@ slot_deform_tuple(TupleTableSlot *slot, int natts) slow = true; /* can't use attcacheoff anymore */ } + for (; attnum < valnatts; attnum++) + { + values[attnum] = 0; + isnull[attnum] = 1; + } + /* * Save state for next execution */ - slot->tts_nvalid = attnum; + slot->tts_nvalid = valnatts; slot->tts_off = off; slot->tts_slow = slow; } @@ -1142,46 +1152,38 @@ Datum slot_getattr(TupleTableSlot *slot, int attnum, bool *isnull) { HeapTuple tuple = slot->tts_tuple; - TupleDesc tupleDesc = slot->tts_tupleDescriptor; - HeapTupleHeader tup; + TupleDesc tupleDesc PG_USED_FOR_ASSERTS_ONLY = slot->tts_tupleDescriptor; /* * system attributes are handled by heap_getsysattr */ - if (attnum <= 0) + if (unlikely(attnum <= 0)) { - if (tuple == NULL) /* internal error */ - elog(ERROR, "cannot extract system attribute from virtual tuple"); - if (tuple == &(slot->tts_minhdr)) /* internal error */ - elog(ERROR, "cannot extract system attribute from minimal tuple"); + + /* cannot extract system attribute from virtual tuple */ + Assert(tuple); + /* "cannot extract system attribute from minimal tuple */ + Assert(tuple != &(slot->tts_minhdr)); return heap_getsysattr(tuple, attnum, tupleDesc, isnull); } /* * fast path if desired attribute already cached */ - if (attnum <= slot->tts_nvalid) + if (likely(attnum <= slot->tts_nvalid)) { *isnull = slot->tts_isnull[attnum - 1]; return slot->tts_values[attnum - 1]; } /* - * return NULL if attnum is out of range according to the tupdesc + * While tuples might possibly be wider than the slot, they should never + * be accessed. We used to return NULL if so, but that a) isn't free b) + * seems more likely to hide bugs than anything. */ - if (attnum > tupleDesc->natts) - { - *isnull = true; - return (Datum) 0; - } - - /* - * otherwise we had better have a physical tuple (tts_nvalid should equal - * natts in all virtual-tuple cases) - */ - if (tuple == NULL) /* internal error */ - elog(ERROR, "cannot extract attribute from empty tuple slot"); + Assert(attnum <= tupleDesc->natts); +#ifdef NOT_ANYMORE /* * return NULL if attnum is out of range according to the tuple * @@ -1195,26 +1197,13 @@ slot_getattr(TupleTableSlot *slot, int attnum, bool *isnull) *isnull = true; return (Datum) 0; } +#endif /* - * check if target attribute is null: no point in groveling through tuple + * otherwise we had better have a physical tuple (tts_nvalid should equal + * natts in all virtual-tuple cases) */ - if (HeapTupleHasNulls(tuple) && att_isnull(attnum - 1, tup->t_bits)) - { - *isnull = true; - return (Datum) 0; - } - - /* - * If the attribute's column has been dropped, we force a NULL result. - * This case should not happen in normal use, but it could happen if we - * are executing a plan cached before the column was dropped. - */ - if (TupleDescAttr(tupleDesc, attnum - 1)->attisdropped) - { - *isnull = true; - return (Datum) 0; - } + Assert(tuple != NULL); /* * Extract the attribute, along with any preceding attributes. @@ -1238,8 +1227,7 @@ void slot_getallattrs(TupleTableSlot *slot) { int tdesc_natts = slot->tts_tupleDescriptor->natts; - int attnum; - HeapTuple tuple; + HeapTuple tuple PG_USED_FOR_ASSERTS_ONLY; /* Quick out if we have 'em all already */ if (slot->tts_nvalid == tdesc_natts) @@ -1250,27 +1238,10 @@ slot_getallattrs(TupleTableSlot *slot) * natts in all virtual-tuple cases) */ tuple = slot->tts_tuple; - if (tuple == NULL) /* internal error */ - elog(ERROR, "cannot extract attribute from empty tuple slot"); + Assert(tuple != NULL); - /* - * load up any slots available from physical tuple - */ - attnum = HeapTupleHeaderGetNatts(tuple->t_data); - attnum = Min(attnum, tdesc_natts); - - slot_deform_tuple(slot, attnum); - - /* - * If tuple doesn't have all the atts indicated by tupleDesc, read the - * rest as null - */ - for (; attnum < tdesc_natts; attnum++) - { - slot->tts_values[attnum] = (Datum) 0; - slot->tts_isnull[attnum] = true; - } - slot->tts_nvalid = tdesc_natts; + slot_deform_tuple(slot, tdesc_natts); + Assert(tdesc_natts <= slot->tts_nvalid); } /* @@ -1281,43 +1252,22 @@ slot_getallattrs(TupleTableSlot *slot) void slot_getsomeattrs(TupleTableSlot *slot, int attnum) { - HeapTuple tuple; - int attno; - /* Quick out if we have 'em all already */ if (slot->tts_nvalid >= attnum) return; /* Check for caller error */ - if (attnum <= 0 || attnum > slot->tts_tupleDescriptor->natts) - elog(ERROR, "invalid attribute number %d", attnum); + Assert(attnum > 0); + Assert(attnum <= slot->tts_tupleDescriptor->natts); /* * otherwise we had better have a physical tuple (tts_nvalid should equal * natts in all virtual-tuple cases) */ - tuple = slot->tts_tuple; - if (tuple == NULL) /* internal error */ - elog(ERROR, "cannot extract attribute from empty tuple slot"); + Assert(slot->tts_tuple != NULL); /* internal error */ - /* - * load up any slots available from physical tuple - */ - attno = HeapTupleHeaderGetNatts(tuple->t_data); - attno = Min(attno, attnum); - - slot_deform_tuple(slot, attno); - - /* - * If tuple doesn't have all the atts indicated by tupleDesc, read the - * rest as null - */ - for (; attno < attnum; attno++) - { - slot->tts_values[attno] = (Datum) 0; - slot->tts_isnull[attno] = true; - } - slot->tts_nvalid = attnum; + slot_deform_tuple(slot, attnum); + Assert(attnum <= slot->tts_nvalid); } /* @@ -1329,38 +1279,34 @@ bool slot_attisnull(TupleTableSlot *slot, int attnum) { HeapTuple tuple = slot->tts_tuple; - TupleDesc tupleDesc = slot->tts_tupleDescriptor; + TupleDesc tupleDesc PG_USED_FOR_ASSERTS_ONLY = slot->tts_tupleDescriptor; /* * system attributes are handled by heap_attisnull */ - if (attnum <= 0) + if (unlikely(attnum <= 0)) { - if (tuple == NULL) /* internal error */ - elog(ERROR, "cannot extract system attribute from virtual tuple"); - if (tuple == &(slot->tts_minhdr)) /* internal error */ - elog(ERROR, "cannot extract system attribute from minimal tuple"); + /* cannot extract system attribute from virtual tuple */ + Assert(tuple); + /* "cannot extract system attribute from minimal tuple */ + Assert(tuple != &(slot->tts_minhdr)); return heap_attisnull(tuple, attnum); } /* * fast path if desired attribute already cached */ - if (attnum <= slot->tts_nvalid) + if (likely(attnum <= slot->tts_nvalid)) return slot->tts_isnull[attnum - 1]; - /* - * return NULL if attnum is out of range according to the tupdesc - */ - if (attnum > tupleDesc->natts) - return true; + /* Check for caller error */ + Assert(attnum <= tupleDesc->natts); /* * otherwise we had better have a physical tuple (tts_nvalid should equal * natts in all virtual-tuple cases) */ - if (tuple == NULL) /* internal error */ - elog(ERROR, "cannot extract attribute from empty tuple slot"); + Assert(tuple != NULL); /* and let the tuple tell it */ return heap_attisnull(tuple, attnum); -- 2.14.1.2.g4274c698f4.dirty