Centralize slot deforming logic a bit.
authorAndres Freund <andres@anarazel.de>
Fri, 4 Aug 2017 22:06:29 +0000 (15:06 -0700)
committerAndres Freund <andres@anarazel.de>
Fri, 1 Sep 2017 06:22:35 +0000 (23:22 -0700)
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:

src/backend/access/common/heaptuple.c

index 13ee528e2616a07de177253cc9fcd13301f0dbfe..f77ea477fbc8ca7ac89fc719254faf365ece5f82 100644 (file)
@@ -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
-    */
-   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)
+    * 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 (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
-    */
-   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.
+    * otherwise we had better have a physical tuple (tts_nvalid should equal
+    * natts in all virtual-tuple cases)
     */
-   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");
-
-   /*
-    * load up any slots available from physical tuple
-    */
-   attno = HeapTupleHeaderGetNatts(tuple->t_data);
-   attno = Min(attno, attnum);
+   Assert(slot->tts_tuple != NULL); /* internal error */
 
-   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);