Add comment on why pulling data from a "name" index column can't crash.

It's been bothering me for several days that pretending that the cstring
data stored in a btree name_ops column is really a "name" Datum could lead
to reading past the end of memory.  However, given the current memory
layout used for index-only scans in the btree code, a crash is in fact not
possible.  Document that so we don't break it.  I have not thought of any
other solutions that aren't fairly ugly too, and most of them lose the
functionality of index-only scans on name columns altogether, so this seems
like the way to go.

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 80e703b1906a0cfc843e1d8c51eabf660d30c361..3401bc5bdb26b2be2314acaa28969c9510930980 100644 (file)
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -433,6 +433,16 @@ btrescan(PG_FUNCTION_ARGS)
     * not already done in a previous rescan call.  To save on palloc
     * overhead, both workspaces are allocated as one palloc block; only this
     * function and btendscan know that.
+    *
+    * NOTE: this data structure also makes it safe to return data from a
+    * "name" column, even though btree name_ops uses an underlying storage
+    * datatype of cstring.  The risk there is that "name" is supposed to be
+    * padded to NAMEDATALEN, but the actual index tuple is probably shorter.
+    * However, since we only return data out of tuples sitting in the
+    * currTuples array, a fetch of NAMEDATALEN bytes can at worst pull some
+    * data out of the markTuples array --- running off the end of memory for
+    * a SIGSEGV is not possible.  Yeah, this is ugly as sin, but it beats
+    * adding special-case treatment for name_ops elsewhere.
     */
    if (scan->xs_want_itup && so->currTuples == NULL)
    {