Re: Inheriting table AMs for partitioned tables

On 2019-03-04 22:08:04 -0800, Andres Freund wrote:
> Hi,
> 
> On 2019-03-05 16:01:50 +1300, David Rowley wrote:
> > On Tue, 5 Mar 2019 at 12:47, Andres Freund <andres@anarazel.de> wrote:
> > > CREATE TABLE tableam_parted_heap2 (a text, b int) PARTITION BY list (a) USING heap2;
> > >
> > > SET default_table_access_method = 'heap';
> > > CREATE TABLE tableam_parted_a_heap2 PARTITION OF tableam_parted_heap2 FOR VALUES IN ('a');
> > 
> > 
> > > But for tableam_parted_a_heap2 tableam_parted_b_heap2 the answer isn't
> > > quite as clear.  I think it'd both be sensible for new partitions to
> > > inherit the AM from the root, but it'd also be sensible to use the
> > > current default.
> > 
> > I'd suggest it's made to work the same way as ca4103025dfe26 made
> > tablespaces work.
> 
> Hm, is that actually correct?  Because as far as I can tell that doesn't
> have the necessary pg_dump code to make this behaviour persistent:
> 
> CREATE TABLESPACE frak LOCATION '/tmp/frak';
> CREATE TABLE test_tablespace (a text, b int) PARTITION BY list (a) TABLESPACE frak ;
> CREATE TABLE test_tablespace_1 PARTITION OF test_tablespace FOR VALUES in ('a');
> CREATE TABLE test_tablespace_2 PARTITION OF test_tablespace FOR VALUES in ('b') TABLESPACE pg_default;
> CREATE TABLE test_tablespace_3 PARTITION OF test_tablespace FOR VALUES in ('c') TABLESPACE frak;
> 
> SELECT relname, relkind, reltablespace FROM pg_class WHERE relname LIKE 'test_tablespace%' ORDER BY 1;
> ┌───────────────────┬─────────┬───────────────┐
> │      relname      │ relkind │ reltablespace │
> ├───────────────────┼─────────┼───────────────┤
> │ test_tablespace   │ p       │         16384 │
> │ test_tablespace_1 │ r       │         16384 │
> │ test_tablespace_2 │ r       │             0 │
> │ test_tablespace_3 │ r       │         16384 │
> └───────────────────┴─────────┴───────────────┘
> (4 rows)
> 
> but a dump outputs (abbreviated)
> 
> SET default_tablespace = frak;
> CREATE TABLE public.test_tablespace (
>     a text,
>     b integer
> )
> PARTITION BY LIST (a);
> CREATE TABLE public.test_tablespace_1 PARTITION OF public.test_tablespace
> FOR VALUES IN ('a');
> SET default_tablespace = '';
> CREATE TABLE public.test_tablespace_2 PARTITION OF public.test_tablespace
> FOR VALUES IN ('b');
> SET default_tablespace = frak;
> CREATE TABLE public.test_tablespace_3 PARTITION OF public.test_tablespace
> FOR VALUES IN ('c');
> 
> which restores to:
> 
> postgres[32125][1]=# SELECT relname, relkind, reltablespace FROM pg_class WHERE relname LIKE 'test_tablespace%' ORDER
BY1;
 
> ┌───────────────────┬─────────┬───────────────┐
> │      relname      │ relkind │ reltablespace │
> ├───────────────────┼─────────┼───────────────┤
> │ test_tablespace   │ p       │         16384 │
> │ test_tablespace_1 │ r       │         16384 │
> │ test_tablespace_2 │ r       │         16384 │
> │ test_tablespace_3 │ r       │         16384 │
> └───────────────────┴─────────┴───────────────┘
> (4 rows)
> 
> because public.test_tablespace_2 assumes it's ought to inherit the
> tablespace from the partitioned table.
> 
> 
> I also find it far from clear that:
>     <listitem>
>      <para>
>       The <replaceable class="parameter">tablespace_name</replaceable> is the name
>       of the tablespace in which the new table is to be created.
>       If not specified,
>       <xref linkend="guc-default-tablespace"/> is consulted, or
>       <xref linkend="guc-temp-tablespaces"/> if the table is temporary.  For
>       partitioned tables, since no storage is required for the table itself,
>       the tablespace specified here only serves to mark the default tablespace
>       for any newly created partitions when no other tablespace is explicitly
>       specified.
>      </para>
>     </listitem>
> is handled correctly. The above says that the *specified* tablespaces -
> which seems to exclude the default tablespace - is what's going to
> determine what partitions use as their default tablespace. But in fact
> that's not true, the partitioned table's pg_class.retablespace is set to
> what default_tablespaces was at the time of the creation.
> 
> 
> > i.e. if they specify the storage type when creating
> > the partition, then always use that, unless they mention otherwise. If
> > nothing was mentioned when they created the partition, then use
> > default_table_access_method.
> 
> Hm. That'd be doable, but given the above ambiguities I'm not convinced
> that's the best approach.  As far as I can see that'd require:
> 
> 1) At relation creation, for partitioned tables only, do not take
>    default_table_access_method into account.
> 
> 2) At partition creation, if the AM is not specified and if the
>    partitioned table's relam is 0, use the default_table_access_method.
> 
> 3) At pg_dump, for partitioned tables only, explicitly emit a USING
>    ... rather than use the method of manipulating default_table_access_method.
> 
> As far as I can tell, the necessary steps are also what'd need to be
> done to actually implement the described behaviour for TABLESPACE (with
> s/default_table_access_method/default_tablespace/ and s/USING/TABLESPACE
> of course).

Based on this mail I'm currently planning to simply forbid specifying
USING for partitioned tables. Then we can argue about this later.

Greetings,

Andres Freund