On Fri, Feb 3, 2017 at 4:15 AM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote:
> Here are some patches to improve the documentation about partitioned > tables: > > 0001: Adds some details about partition_bound_spec to the CREATE TABLE > page, especially: > > - a note about inclusivity of range partition bounds, > - a note about the UNBOUNDED literal in case of range partitioning, > - a note about the NULL literal in case of list partitioning, > > I wonder if the above "note" should be added under the Notes section or > are they fine to be added as part of the description of PARTITION OF > clause. Also: > > - in syntax synopsis, it appears now that any expression is OK to be used > for individual bound datum, but it's not true. Only literals are > allowed. So fixed that. > - added an example showing how to create partitions of a range > partitioned table with multiple columns in the partition key > - added PARTITION BY and PARTITION OF (FOR VALUES) as PostgreSQL > extensions in the compatibility section > > > 0002: Adds details about partitioned tables to the DDL chapter (ddl.sgml) > > - a new section named "Partitioned Tables" right next to the > Inheritance and Partitioning sections is created. > - examples are added to the existing Partitioning section using the new > partitioned tables. Old text about implementing table partitioning > using inheritance is kept, sort of as a still supported older > alternative. > > 0003: Add partitioning keywords to keywords.sgml > > This is all I have for now. Any feedback is greatly appreciated. Adding > this to the next CF. > > Thanks, > Amit > Patch applies. Overall this looks really good. It goes a long way towards stating some of the things I had to learn through experimentation. I had to read a really long way into the patch before finding a blurb that I felt wasn't completely clear: + + <para> + <command>INSERT</command> statements with <literal>ON CONFLICT</> + clause are currently not allowed on partitioned tables, that is, + cause error when specified. + </para> Here's some other tries at saying the same thing, none of which are completely satisfying: ...ON CONFLICT clause are currently not allowed on partitioned tables and will cause an error? ...ON CONFLICT clause are currently not allowed on partitioned tables and will instead cause an error? ...ON CONFLICT clause will currently cause an error if used on a partitioned table? As far as additional issues to cover, this bit: + <listitem> + <para> + One cannot drop a <literal>NOT NULL</literal> constraint on a + partition's column, if the constraint is present in the parent table. + </para> + </listitem> Maybe we should add something about how one would go about dropping a NOT NULL constraint (parent first then partitions?) In reviewing this patch, do all our target formats make word spacing irrelevant? i.e. is there any point in looking at the number of spaces after a period, etc? A final note, because I'm really familiar with partitioning on Postgres and other databases, documentation which is clear to me might not be to someone less familiar with partitioning. Maybe we want another reviewer for that?