Re: SPITupleTable members missing in docs

Daniel Gustafsson <[email protected]>
Newsgroups gmane.comp.db.postgresql.devel.documentation
Message-ID <[email protected]>
> On 12 Jul 2019, at 17:04, Tom Lane <[email protected]> wrote:
> 
> Fabien COELHO <[email protected]> writes:
>> To take into account Tom's comment, I'd suggest a middle ground by 
>> commenting a public and private part explicitely in the struct, something 
>> like:

Thanks for the review!

>>   typedef struct {
>>     /* PUBLIC members to be used by callers ... */
>>     ...
>>     ...
>>     /* PRIVATE members, not intended for external usage ... */
>>     ...
>>   } ... ;
> 
> One problem is that the members we've retroactively decided are "public"
> are in the middle of the struct :-(.
> 
> But it occurs to me that there's no good reason we couldn't re-order the
> members, as long as we only do so on HEAD and not in released versions.
> That would make it a bit less inconsistent and easier to add labels
> such as you suggest.

I quite like this suggestion, so I’ve changed the patch to do this.  Removed
the doc: in the commit message to indicate that this is no longer just touching
documentation.

cheers ./daniel
spitupletable-v2.patch (application/octet-stream, 3.3 KB)
From 9c0985edece22bd3e1c9c4f0c921dde99f14574e Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Fri, 14 Jun 2019 12:38:11 +0200
Subject: [PATCH] Add missing members to SPITupleTable documentation

Commit 3d13623d75d3206c8f009353415043a191ebab39 added the next and
subid fields to the SPITupleTable struct, but they never made it
into the documentation. While these are internal members, we already
document several other internal ones so add these too to make the
documentation match reality.  Also reorder the members to separate
the public from the private fields.

Since this makes the number of internal members far outnumber the
public ones, also reword the statement about which fields can be
used to improve clarity.

fixup
---
 doc/src/sgml/spi.sgml      | 21 +++++++++++++--------
 src/include/executor/spi.h |  7 +++++--
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index 66eced6c94..37fe11b4f5 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -320,19 +320,24 @@ SPI_execute("INSERT INTO foo SELECT * FROM bar RETURNING *", false, 5);
 <programlisting>
 typedef struct
 {
-    MemoryContext tuptabcxt;    /* memory context of result table */
-    uint64      alloced;        /* number of alloced vals */
-    uint64      free;           /* number of free vals */
-    TupleDesc   tupdesc;        /* row descriptor */
-    HeapTuple  *vals;           /* rows */
+    /* Public members */
+    TupleDesc	tupdesc;		/* row descriptor */
+    HeapTuple  *vals;			/* rows */
+
+    /* Private members, not intended for external callers */
+    MemoryContext tuptabcxt;	/* memory context of result table */
+    uint64		alloced;		/* # of alloced vals */
+    uint64		free;			/* # of free vals */
+    slist_node	next;			/* link for internal bookkeeping */
+    SubTransactionId subid;		/* subxact in which tuptable was created */
 } SPITupleTable;
 </programlisting>
+   <structfield>vals</structfield> and <structfield>tupdesc</structfield> can
+   be used by SPI callers, the remaining fields are internal.
    <structfield>vals</structfield> is an array of pointers to rows.  (The number
    of valid entries is given by <varname>SPI_processed</varname>.)
    <structfield>tupdesc</structfield> is a row descriptor which you can pass to
-   SPI functions dealing with rows.  <structfield>tuptabcxt</structfield>,
-   <structfield>alloced</structfield>, and <structfield>free</structfield> are internal
-   fields not intended for use by SPI callers.
+   SPI functions dealing with rows.
   </para>
 
   <para>
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index 7bf361874d..b3dfe30f4e 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -21,11 +21,14 @@
 
 typedef struct SPITupleTable
 {
+	/* Public members */
+	TupleDesc	tupdesc;		/* tuple descriptor */
+	HeapTuple  *vals;			/* tuples */
+
+	/* Private members, not intended for external callers */
 	MemoryContext tuptabcxt;	/* memory context of result table */
 	uint64		alloced;		/* # of alloced vals */
 	uint64		free;			/* # of free vals */
-	TupleDesc	tupdesc;		/* tuple descriptor */
-	HeapTuple  *vals;			/* tuples */
 	slist_node	next;			/* link for internal bookkeeping */
 	SubTransactionId subid;		/* subxact in which tuptable was created */
 } SPITupleTable;
-- 
2.14.1.145.gb3622a4ee
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.