Re: Documentation improvements for partitioning

On Fri, Feb 10, 2017 at 3:00 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> Given that we already have partitioning feature committed, we really
> need to have the docs committed as well.

Just for the record, it's not like there were no documentation changes
in the originally committed patch. In fact there were over 400 lines
of documentation:

doc/src/sgml/catalogs.sgml | 129 +++++++++++++++++++++++-
doc/src/sgml/ref/alter_table.sgml | 117 +++++++++++++++++++++-
doc/src/sgml/ref/create_foreign_table.sgml | 26 +++++
doc/src/sgml/ref/create_table.sgml | 154 +++++++++++++++++++++++++++++

The patch you committed approximately doubles the amount of
documentation for this feature, which is fine as far as it goes, but
the key points were all explained in the original commit. I have been
known to leave out documentation from commits from time to time and
fill it in after-the-fact, but that's not really what happened here.

> Without claiming I'm happy about this, I think the best way to improve
> the number of eyeballs on this is to commit these docs as is.
>
> For me, the most important thing is understanding the feature, not
> (yet) discussing what the docs should look like. This is especially
> true if other patches reference the way partitioning works and nobody
> can comment on those patches because they don't understand
>
> Any issues with that?

There are a number of things that I think are awkward about the patch
as committed:

+ <listitem>
+ <para>
+ See last section for some general information:
+ <xref linkend="ddl-partitioned-tables">
+ </para>
+ </listitem>

I think we generally try to write the documentation in such a way as
to minimize backward references, and a backward reference to the
previous section seems particularly odd. We've now got section "5.10
Partitioned Tables" followed immediately by section "5.11
Partitioning", where the latter seems to think that you haven't read
the former.

I think that section 5.11 needs a much heavier rewrite than what it
got as part of this patch. It's a hodgepodge of the old content
(which explained how to fake partitioning when we didn't have an
explicit concept of partitioning) and new content that talks about how
the new way is different from the old way. But now that we have the
new way, I'm guessing that most people are going to use that and not
care about the old way any more. I'm not that it's even appropriate
to keep the lengthy explanation of how to fake table partitioning
using table inheritance and non-overlapping CHECK constraints, but if
we still want that stuff it should be de-emphasized more than it is
here. Probably the section should be retitled: in previous releases
we called this "Partitioning" because we had no explicit notion of
partitioning, but now that we do, it's confusing to have a section
called "Partitioning" that explains how to avoid using the
partitioning feature, which is more or less what this does. Or maybe
the section title should stay the same (or this should be merged into
the previous section?) but rewritten to change the emphasis.

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