Remove partial, broken support for NULL pointers when fetching attributes.

Previously, fastgetattr() and heap_getattr() tested their fourth argument
against a null pointer, but any attempt to use them with a literal-NULL
fourth argument evaluated to *(void *)0, resulting in a compiler error.
Remove these NULL tests to avoid leading future readers of this code to
believe that this has a chance of working.  Also clean up related legacy
code in nocachegetattr(), heap_getsysattr(), and nocache_index_getattr().

The new coding standard is that any code which calls a getattr-type
function or macro which takes an isnull argument MUST pass a valid
boolean pointer.  Per discussion with Bruce Momjian, Tom Lane, Alvaro
Herrera.

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 4fe103fb4b95717dbedf76b04f52d325c660e9e0..6ec73f0815b56bef3d41daa53609f7dd624622cb 100644 (file)
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -323,8 +323,7 @@ heap_attisnull(HeapTuple tup, int attnum)
 Datum
 nocachegetattr(HeapTuple tuple,
               int attnum,
-              TupleDesc tupleDesc,
-              bool *isnull)
+              TupleDesc tupleDesc)
 {
    HeapTupleHeader tup = tuple->t_data;
    Form_pg_attribute *att = tupleDesc->attrs;
@@ -333,8 +332,6 @@ nocachegetattr(HeapTuple tuple,
    bool        slow = false;   /* do we have to walk attrs? */
    int         off;            /* current offset within data */
 
-   (void) isnull;              /* not used */
-
    /* ----------------
     *   Three cases:
     *
@@ -344,68 +341,32 @@ nocachegetattr(HeapTuple tuple,
     * ----------------
     */
 
-#ifdef IN_MACRO
-/* This is handled in the macro */
-   Assert(attnum > 0);
-
-   if (isnull)
-       *isnull = false;
-#endif
-
    attnum--;
 
-   if (HeapTupleNoNulls(tuple))
-   {
-#ifdef IN_MACRO
-/* This is handled in the macro */
-       if (att[attnum]->attcacheoff >= 0)
-       {
-           return fetchatt(att[attnum],
-                           (char *) tup + tup->t_hoff +
-                           att[attnum]->attcacheoff);
-       }
-#endif
-   }
-   else
+   if (!HeapTupleNoNulls(tuple))
    {
        /*
         * there's a null somewhere in the tuple
         *
-        * check to see if desired att is null
+        * check to see if any preceding bits are null...
         */
+       int byte = attnum >> 3;
+       int         finalbit = attnum & 0x07;
 
-#ifdef IN_MACRO
-/* This is handled in the macro */
-       if (att_isnull(attnum, bp))
-       {
-           if (isnull)
-               *isnull = true;
-           return (Datum) NULL;
-       }
-#endif
-
-       /*
-        * Now check to see if any preceding bits are null...
-        */
+       /* check for nulls "before" final bit of last byte */
+       if ((~bp[byte]) & ((1 << finalbit) - 1))
+           slow = true;
+       else
        {
-           int byte = attnum >> 3;
-           int         finalbit = attnum & 0x07;
+           /* check for nulls in any "earlier" bytes */
+           int         i;
 
-           /* check for nulls "before" final bit of last byte */
-           if ((~bp[byte]) & ((1 << finalbit) - 1))
-               slow = true;
-           else
+           for (i = 0; i < byte; i++)
            {
-               /* check for nulls in any "earlier" bytes */
-               int         i;
-
-               for (i = 0; i < byte; i++)
+               if (bp[i] != 0xFF)
                {
-                   if (bp[i] != 0xFF)
-                   {
-                       slow = true;
-                       break;
-                   }
+                   slow = true;
+                   break;
                }
            }
        }
@@ -567,8 +528,7 @@ heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull)
    Assert(tup);
 
    /* Currently, no sys attribute ever reads as NULL. */
-   if (isnull)
-       *isnull = false;
+   *isnull = false;
 
    switch (attnum)
    {

diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c
index 76b4299751d701a414e687048833a56f44d55c8c..fd4530b0746b57c40eb53b07060e2dff99630371 100644 (file)
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -200,8 +200,7 @@ index_form_tuple(TupleDesc tupleDescriptor,
 Datum
 nocache_index_getattr(IndexTuple tup,
                      int attnum,
-                     TupleDesc tupleDesc,
-                     bool *isnull)
+                     TupleDesc tupleDesc)
 {
    Form_pg_attribute *att = tupleDesc->attrs;
    char       *tp;             /* ptr to data part of tuple */
@@ -210,8 +209,6 @@ nocache_index_getattr(IndexTuple tup,
    int         data_off;       /* tuple data offset */
    int         off;            /* current offset within data */
 
-   (void) isnull;              /* not used */
-
    /* ----------------
     *   Three cases:
     *
@@ -221,31 +218,11 @@ nocache_index_getattr(IndexTuple tup,
     * ----------------
     */
 
-#ifdef IN_MACRO
-/* This is handled in the macro */
-   Assert(PointerIsValid(isnull));
-   Assert(attnum > 0);
-
-   *isnull = false;
-#endif
-
    data_off = IndexInfoFindDataOffset(tup->t_info);
 
    attnum--;
 
-   if (!IndexTupleHasNulls(tup))
-   {
-#ifdef IN_MACRO
-/* This is handled in the macro */
-       if (att[attnum]->attcacheoff >= 0)
-       {
-           return fetchatt(att[attnum],
-                           (char *) tup + data_off +
-                           att[attnum]->attcacheoff);
-       }
-#endif
-   }
-   else
+   if (IndexTupleHasNulls(tup))
    {
        /*
         * there's a null somewhere in the tuple
@@ -256,16 +233,6 @@ nocache_index_getattr(IndexTuple tup,
        /* XXX "knows" t_bits are just after fixed tuple header! */
        bp = (bits8 *) ((char *) tup + sizeof(IndexTupleData));
 
-#ifdef IN_MACRO
-/* This is handled in the macro */
-
-       if (att_isnull(attnum, bp))
-       {
-           *isnull = true;
-           return (Datum) NULL;
-       }
-#endif
-
        /*
         * Now check to see if any preceding bits are null...
         */

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 509e31f07aed5c6ed8367b3ce6b1310fd9475085..037c8f0209ed7381cc559ff6f0f9835034309f53 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -834,7 +834,7 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
    return (
            (attnum) > 0 ?
            (
-            ((isnull) ? (*(isnull) = false) : (dummyret) NULL),
+            (*(isnull) = false),
             HeapTupleNoNulls(tup) ?
             (
              (tupleDesc)->attrs[(attnum) - 1]->attcacheoff >= 0 ?
@@ -844,18 +844,18 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
                        (tupleDesc)->attrs[(attnum) - 1]->attcacheoff)
               )
              :
-             nocachegetattr((tup), (attnum), (tupleDesc), (isnull))
+             nocachegetattr((tup), (attnum), (tupleDesc))
              )
             :
             (
              att_isnull((attnum) - 1, (tup)->t_data->t_bits) ?
              (
-              ((isnull) ? (*(isnull) = true) : (dummyret) NULL),
+              (*(isnull) = true),
               (Datum) NULL
               )
              :
              (
-              nocachegetattr((tup), (attnum), (tupleDesc), (isnull))
+              nocachegetattr((tup), (attnum), (tupleDesc))
               )
              )
             )

diff --git a/src/include/access/htup.h b/src/include/access/htup.h
index a42782ea36d8b570e68a81b2470443f3e3b98834..81dd40efc22c3fa3287e60bea68509b6c3a0eb48 100644 (file)
--- a/src/include/access/htup.h
+++ b/src/include/access/htup.h
@@ -763,7 +763,7 @@ extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
 #define fastgetattr(tup, attnum, tupleDesc, isnull)                    \
 (                                                                  \
    AssertMacro((attnum) > 0),                                      \
-   (((isnull) != NULL) ? (*(isnull) = false) : (dummyret)NULL),    \
+   (*(isnull) = false),                                            \
    HeapTupleNoNulls(tup) ?                                         \
    (                                                               \
        (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ?          \
@@ -773,18 +773,18 @@ extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
                    (tupleDesc)->attrs[(attnum)-1]->attcacheoff)    \
        )                                                           \
        :                                                           \
-           nocachegetattr((tup), (attnum), (tupleDesc), (isnull))  \
+           nocachegetattr((tup), (attnum), (tupleDesc))            \
    )                                                               \
    :                                                               \
    (                                                               \
        att_isnull((attnum)-1, (tup)->t_data->t_bits) ?             \
        (                                                           \
-           (((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL),     \
+           (*(isnull) = true),                                     \
            (Datum)NULL                                             \
        )                                                           \
        :                                                           \
        (                                                           \
-           nocachegetattr((tup), (attnum), (tupleDesc), (isnull))  \
+           nocachegetattr((tup), (attnum), (tupleDesc))            \
        )                                                           \
    )                                                               \
 )
@@ -818,7 +818,7 @@ extern Datum fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
        ( \
            ((attnum) > (int) HeapTupleHeaderGetNatts((tup)->t_data)) ? \
            ( \
-               (((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \
+               (*(isnull) = true), \
                (Datum)NULL \
            ) \
            : \
@@ -838,7 +838,7 @@ extern void heap_fill_tuple(TupleDesc tupleDesc,
                uint16 *infomask, bits8 *bit);
 extern bool heap_attisnull(HeapTuple tup, int attnum);
 extern Datum nocachegetattr(HeapTuple tup, int attnum,
-              TupleDesc att, bool *isnull);
+              TupleDesc att);
 extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
                bool *isnull);
 extern HeapTuple heap_copytuple(HeapTuple tuple);

diff --git a/src/include/access/itup.h b/src/include/access/itup.h
index 7286b601805f6282a811c525b9bd20f9f6d49db4..afaffdd669b6e294bec1f463772c066bb8380e2f 100644 (file)
--- a/src/include/access/itup.h
+++ b/src/include/access/itup.h
@@ -110,7 +110,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap;
            + (tupleDesc)->attrs[(attnum)-1]->attcacheoff) \
        ) \
        : \
-           nocache_index_getattr((tup), (attnum), (tupleDesc), (isnull)) \
+           nocache_index_getattr((tup), (attnum), (tupleDesc)) \
    ) \
    : \
    ( \
@@ -121,7 +121,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap;
        ) \
        : \
        ( \
-           nocache_index_getattr((tup), (attnum), (tupleDesc), (isnull)) \
+           nocache_index_getattr((tup), (attnum), (tupleDesc)) \
        ) \
    ) \
 )
@@ -142,7 +142,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap;
 extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor,
                 Datum *values, bool *isnull);
 extern Datum nocache_index_getattr(IndexTuple tup, int attnum,
-                     TupleDesc tupleDesc, bool *isnull);
+                     TupleDesc tupleDesc);
 extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor,
                 Datum *values, bool *isnull);
 extern IndexTuple CopyIndexTuple(IndexTuple source);