Re: Documentation improvements for partitioning

On 2017/02/15 13:31, Robert Haas wrote:
> On Mon, Feb 13, 2017 at 5:57 AM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> On 2017/02/13 14:21, Amit Langote wrote:
>>> On 2017/02/10 19:19, Simon Riggs wrote:
>>>> * The OID inheritance needs work - you shouldn't need to specify a
>>>> partition needs OIDS if the parent has it already.
>>>
>>> That sounds right. It's better to keep the behavior same as for regular
>>> inheritance. Will post a patch to get rid of the unnecessary check.
>>>
>>> FWIW, the check was added to prevent the command from succeeding in the
>>> case where WITHOUT OIDS has been specified for a partition and the parent
>>> partitioned table has the OID column. Regular inheritance simply
>>> *overrides* the WITHOUT OIDS specification, which might be seen as surprising.
>>
>> 0001 of the attached patches takes care of this.
>
> I think 0001 needs to remove this hunk of documentation:
>
> <listitem>
> <para>
> If the partitioned table specified <literal>WITH OIDS</literal> then
> each partition must also specify <literal>WITH OIDS</literal>. Oids
> are not automatically inherited by partitions.
> </para>
> </listitem>

Attached updated 0001 which does that.

> I think 0001 is better than the status quo, but I'm wondering whether
> we should try to do something slightly different. Maybe it should
> always work for the child table to specify neither WITH OIDS nor
> WITHOUT OIDS, but if you do specify one of them then it has to be the
> one that matches the parent partitioned table? With this patch, IIUC,
> WITH OIDS is allowed only if the parent has the same, but WITHOUT OIDS
> is allowed (but ignored) regardless of the parent setting.

With the patch, one can always specify (or not) WITH/WITHOUT OIDS when
creating partitions. If WITH OIDS is specified and the parent doesn't
have OIDs, then an error is raised. Then just like with normal
inheritance, WITHOUT OIDS specification for a partition will be
*overridden* if the parent has OIDs. By the way, CREATE TABLE page says
this about inheritance and OIDS:

(If the new table inherits from any tables that have OIDs, then
<literal>OIDS=TRUE</> is forced even if the command says
<literal>OIDS=FALSE</>.

Hopefully it's clear to someone reading "If the table inherits from any
tables ..." that it also refers to creating partition of a partitioned table.

Also attaching 0002 (unchanged) for tab-completion support for the new
partitioning syntax.

0003 changes how ExecFindPartition() shows the row for which
get_partition_for_tuple() failed to find a partition. As Simon commented
upthread, we should show just the partition key, not the whole row in the
error DETAIL. So the DETAIL now looks like what's shown by
_bt_check_unique() upon uniqueness violation:

DETAIL: Partition key of the failing row contains (key1, key2, ...)=(val1,
val2, ...)

The rules about which columns to show or whether to show the DETAIL at all
are similar to those in BuildIndexValueDescription():

- if user has SELECT privilege on the whole table, simply go ahead

- if user doesn't have SELECT privilege on the table, check that they
can see all the columns in the key (no point in showing partial key);
however abort on finding an expression for which we don't try finding
out privilege situation of whatever columns may be in the expression

Thanks,
Amit