On Mon, Mar 21, 2016 at 2:18 PM, David Rowley
<david.row...@2ndquadrant.com> wrote:
> I've attached 2 of the patches which are affected by the changes.

I think the documentation for 0001 needs some work yet.  The
additional paragraph that you've added...

(1) doesn't seem to appear at a very logical place in the
documentation - I think it should be much further down, as it's a
minor detail.  Maybe document this the same way as the documentation
patch you just sent for the combine-function stuff does it; and

(2) isn't indented consistently with the surrounding paragraphs; and

(3) is missing a closing </para> tag

Also, I'd just cut this:

+  This is required due to
+  the process model being unable to pass references to <literal>INTERNAL
+  </literal> types between different <productname>PostgreSQL</productname>
+  processes.

Instead, I'd change the earlier sentence in the paragraph, which
currently reads:

+  These
+  functions are required in order to allow parallel aggregation for aggregates
+  with an <replaceable class="PARAMETER">stype</replaceable> of <literal>
+  INTERNAL</>.

I'd replace the period at end with a comma and add "since
<literal>INTERNAL</> values represent arbitrary in-memory data
structures which can't be passed between processes".  I think that's a
bit smoother.

I'm going to read through the code again now.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to