Re: Documentation improvements for partitioning

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Documentation improvements for partitioning
Date: 2017-02-10 07:35:19
Message-ID: 89ebfd64-e32e-5a23-139e-0db816390091@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Corey,

On 2017/02/09 6:14, Corey Huinker wrote:
> On Fri, Feb 3, 2017 at 4:15 AM, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
> wrote:
>
>> Here are some patches to improve the documentation about partitioned
>> tables:
>
> Patch applies.
>
> Overall this looks really good. It goes a long way towards stating some of
> the things I had to learn through experimentation.

Thanks a lot for taking a look at it.

> 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?

The last one sounds better.

> 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?)

Dropping it on the parent will cause it to be dropped on the partitions as
well. About your point whether we should add a note about how to go about
dropping it in the partition, it seems to me it would be out of place
here; it's just saying that dropping NOT NULL constraint has a different
behavior with partitioned tables than regular inheritance. That note most
likely belongs in the ALTER TABLE reference page in the DROP NOT NULL
description, so created a patch for that (patch 0004 of the attached patches).

> 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?

It seems to be a convention in the sources to include 2 spaces after a
period, which I just try to follow (both in the code comments and SGML).
I don't see that spaces are relevant as far as how the targets such as
HTML are rendered.

> 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?

More eyeballs will only help make this better.

Thanks,
Amit

Attachment Content-Type Size
0001-Improve-CREATE-TABLE-documentation-of-partitioning.patch text/x-diff 6.8 KB
0002-Update-ddl.sgml-for-declarative-partitioning.patch text/x-diff 23.5 KB
0003-Add-partitioning-keywords-to-keywords.sgml.patch text/x-diff 1.4 KB
0004-Add-a-note-about-DROP-NOT-NULL-and-partitions.patch text/x-diff 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2017-02-10 07:48:15 Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)
Previous Message Amit Langote 2017-02-10 07:29:29 Re: contrib modules and relkind check