Re: Partitioning with temp tables is broken

On Tue, Jun 19, 2018 at 1:24 PM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Tue, Jun 19, 2018 at 04:27:08PM +0900, Amit Langote wrote:
>> Looking at what changed from my patch:
>>
>> - One cannot have both temporary and permanent relations in a given
>> - partition tree. That is, if the root partitioned table is permanent,
>> - so must be its partitions at all levels and vice versa.
>> + Mixing temporary and permanent relations in the same partition tree
>> + is not allowed. Hence, if the root partitioned table is permanent,
>> + so must be its partitions at all levels and vice versa for temporary
>> + relations.
>>
>> The "vice versa" usage in my patch wasn't perhaps right to begin with, but
>> the way your patch extends it make it a bit more confusing. Maybe we
>> should write it as: "... and likewise if the root partitioned table is
>> temporary."
>
> I like you wording better here.

+
+ <listitem>
+ <para>
+ Mixing temporary and permanent relations in the same partition tree
+ is not allowed. Hence, if the root partitioned table is permanent,

Do we want to mention "root" explicitly here?

+ so must be its partitions at all levels and vice versa for temporary
+ relations.
+ </para>
+ </listitem>

Also, we need to mention that all temporary relations should be from the same
session. We can specify tables from other session in "partition of" clause by
using temporary schema of that session.

In general it looks like we could write the above paragraph as
"A permanant partitioned table should have all its partitions permanant. A
temporary partitioned table should have all its partitions temporary and should
belong to the same session which temporary partitioned table belongs to."

Applying this recursively implies that the whole partition tree either consists
of permanant tables or temporary tables but not both.

Looks like MergeAttributes() is doing more than just merging attributes. But
that's not fault of this patch. It's been so for quite some time.

Rest of the patch looks good to me.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company