Re: Documentation improvements for partitioning - Mailing list pgsql-hackers

On 2017/03/28 0:23, Robert Haas wrote:
> On Thu, Mar 9, 2017 at 8:23 PM, Amit Langote wrote:
>> Attached updated patches.
> 
> Committed 0002, 0003.

Thanks a lot for committing these and reviewing 0001.

> I think the section on BRIN in 0001 is just silly.  BRIN is a very
> useful index type, possibly more useful than anything except btree,
> but I think documenting it as an alternative method of partitioning is
> over the top.

Okay, removing the BRIN part from this patch for now.

> +     The following forms of partitioning can be implemented in
> +     <productname>PostgreSQL</productname>:
> 
> Any form of partitioning can be implemented, at least to some degree,
> using inheritance or UNION ALL views.  I think what this should say is
> that PostgreSQL has native support for list and range partitioning,
> and then it can go on top say that if this built-in support is not
> suitable for a particular use case (either because you need some other
> partitioning scheme or due to some other limitation), inheritance or
> UNION ALL views can be used instead, adding flexibility but losing
> some of the performance benefits of built-in declarative partitioning.

You're right.  I've updated the text to sound like what you said here.

>     <para>
>      Partitions may have their own indexes, constraints and default values,
> -    distinct from other partitions. Partitions do not inherit indexes from
> -    the partitioned table.
> +    distinct from other partitions. Partitions do not currently inherit
> +    indexes from the partitioned table.
> +   </para>
> +
> +   <para>
> +    See <xref linkend="sql-createtable"> for more details creating partitioned
> +    tables and partitions.
>     </para>
> 
> I don't think we should add "currently"; that amounts to speculation
> about what will happen in future versions.  Also, I favor collapsing
> these into one paragraph.  A single-sentence paragraph tends to look
> OK when you're reading the SGML directly, but it looks funny in the
> rendered version.

Done.

> 
> +    <firstterm>sub-partitioning</firstterm>.  It is not currently possible to
> +    alter a regular table into a partitioned table or vice versa.  However,
> +    it is possible to add a regular or partitioned table containing data into
> +    a partition of a partitioned table, or remove a partition; see
> 
> I think we should say "as a partition" rather than "into a partition",
> assuming you're talking about ATTACH PARTITION here.

Right, fixed.

> 
> -       partitioned tables.  For example, specifying <literal>ONLY</literal>
> -       when querying data from a partitioned table would not make much sense,
> -       because all the data is contained in partitions, so this raises an
> -       error.  Specifying <literal>ONLY</literal> when modifying schema is
> -       not desirable in certain cases with partitioned tables where it may be
> -       fine for regular inheritance parents (for example, dropping a column
> -       from only the parent); an error will be thrown in that case.
> +       partitioned tables.  Specifying <literal>ONLY</literal> when modifying
> +       schema is not desirable in certain cases with partitioned tables
> +       whereas it may be fine for regular inheritance parents (for example,
> +       dropping a column from only the parent); an error will be thrown in
> +       that case.
> 
> I don't see why this is an improvement.

Because we neither raise an error nor ignore it if ONLY is specified when
querying data from a partitioned table.

create table p (a int, b char) partition by list (a);
create table p1 partition of p for values in (1);
insert into p values (1);

select * from only p;
 a | b
---+---
(0 rows)

explain select * from only p;
                QUERY PLAN
-------------------------------------------
 Result  (cost=0.00..0.00 rows=0 width=12)
   One-Time Filter: false
(2 rows)

IOW, querying behavior is same as regular inheritance.  I rewrote the
paragraph as follows:

    <para>
     The <literal>ONLY</literal> notation used to exclude child tables
     will cause an error for partitioned tables in the case of
     schema-modifying commands such as most <literal>ALTER TABLE</literal>
     commands.  For example, dropping a column from only the parent does
     not make sense for partitioned tables.
    </para>

> -    data inserted into the partitioned table cannot be routed to foreign table
> -    partitions.
> +    data inserted into the partitioned table is currently not routed to foreign
> +    table partitions.
> 
> Again, let's not speculate about the future.
> 
> +       Note that it is currently not supported to propagate index definition
> +       from the master partitioned table to its partitions; in fact, it is
> +       not possible to define indexes on partitioned tables in the first
> +       place.  This might change in future releases.
> 
> Same here.
> 
> +    There are currently the following limitations of using partitioned tables:
> 
> And here.  Better to write "The following limitations apply to
> partitioned tables:"

Fixed all of these.

> +       It is currently not possible to define indexes on partitioned tables
> +       that include all rows from all partitions in one global index.
> +       Consequently, it is not possible to create constraints that are realized
> +       using an index such as <literal>UNIQUE</>.
> 
> This doesn't seem very grammatical, and it kind of overlaps with the
> previous point, and the following point.  How about just adding a
> sentence to the previous paragraph: This also means that there is no
> way to create a primary key, unique constraint, or exclusion
> constraint spanning all partitions; it is only possible to constrain
> each leaf partition individually.

OK, done.

> +       <command>INSERT</command> statements with <literal>ON CONFLICT</>
> +       clause are currently not allowed on partitioned tables.
> 
> Obsolete.

Text from the patch you just committed now replaces this item.

> +       implicit partition constraint of the original partition.  This might
> +       change in future releases.
> 
> Remove speculation.

Done.  Also, a few other "currently"s I had added.

> +     In some cases, one may want to add columns to partitions that are not
> +     present in the parent table which is not possible to do with the above
> +     method.  For such cases, partitioning can be implemented using
> +     inheritance (see <xref linkend="ddl-inherit">).
> 
> Hmm, I bet that's not the only advantage.  And it doesn't seem like
> the way to lead.
> 
> e.g.
> 
> While the built-in declarative partitioning is suitable for most
> common use cases, there are some circumstances where a more flexible
> approach may be useful.  Partitioning can be implemented using table
> inheritance, which allows for several features which are not supported
> by declarative partitioning, such as:
> 
> - Partitioning enforces a rule that all partitions must have exactly
> the same set of columns as the parent, but table inheritance allows
> children to have extra columns not present in the parent.
> 
> - Table inheritance allows for multiple inheritance.
> 
> - Declarative partitioning only supports list and range partitioning,
> whereas table inheritance allows data to be divided in a manner of the
> user's choosing.  (Note, however, that if constraint exclusion is
> unable to prune partitions effectively, query performance will be very
> poor.)
> 
> - Some operations require a stronger lock when using declarative
> partitioning than when using table inheritance.  (list these)

Thanks, that's a lot better.

Attached updated patch.

Regards,
Amit