On Fri, Feb 10, 2017 at 3:00 AM, Simon Riggs <si...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
