Re: Partitioning with temp tables is broken

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Michael Paquier <michael(at)paquier(dot)xyz>, amul sul <sulamul(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Partitioning with temp tables is broken
Date: 2018-06-14 04:12:31
Message-ID: a6bab73c-c5a8-2c25-f858-5d6d800a852d@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2018/06/14 11:09, Michael Paquier wrote:
> On Wed, Jun 13, 2018 at 10:25:23PM +0530, amul sul wrote:
>> On Wed, Jun 13, 2018, 8:34 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Even if you want to argue that there's a use case for these situations,
>>> it seems far too late in the release cycle to be trying to fix all these
>>> issues. I think we need to forbid the problematic cases for now, and
>>> leave relaxing the prohibition to be treated as a future new feature.
>>
>> +1, forbid the problematic case.
>
> +1 on that. And I can see that nobody on this thread has tested with
> REL_10_STABLE, right? In that case you don't get a crash, but other
> sessions can see the temporary partition of another's session, say with
> \d+. So it seems to me that we should forbid the use of temporary
> relations in a partition tree and back-patch it as well to v10 for
> consistency. And if trying to insert into the temporary relation you
> can a nice error:
>
> =# insert into listp values (2);
> ERROR: 0A000: cannot access temporary tables of other sessions
> LOCATION: ReadBufferExtended, bufmgr.c:657
>
> This is also a bit uninstinctive as I would think of it as an authorized
> operation, still my guts tell me that the code complications are not
> really worth the use-cases:
>
> =# create temp table listp2 partition of listp for values in (2);
> ERROR: 42P17: partition "listp2" would overlap partition "listp2"
> LOCATION: check_new_partition_bound, partition.c:81

I have to agree to reverting this "feature". As Michael points out, even
PG 10 fails to give a satisfactory error message when using a declarative
feature like tuple routing. It should've been "partition not found for
tuple" rather than "cannot access temporary tables of other sessions".
Further as David and Ashutosh point out, PG 11 added even more code around
declarative partitioning that failed to consider the possibility that some
partitions may not be accessible in a given session even though visible
through relcache.

I'm attaching a patch here to forbid adding a temporary table as partition
of permanent table. I didn't however touch the feature that allows *all*
members in a partition tree to be temporary tables.

Thanks,
Amit

Attachment Content-Type Size
forbid-temp-parts-1.patch text/plain 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marko Tiikkaja 2018-06-14 04:54:22 Re: Two round for Client Authentication
Previous Message Yinjie Lin 2018-06-14 04:12:16 Two round for Client Authentication