Re: Documentation improvements for partitioning

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