Re: Partitioned tables and relfilenode

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, Maksim Milyutin <m(dot)milyutin(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Partitioned tables and relfilenode
Date: 2017-03-29 06:40:20
Message-ID: c4d71df2-9e0e-3912-dc81-9a72e080c238@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2017/03/27 23:27, Robert Haas wrote:
> On Thu, Mar 23, 2017 at 8:54 PM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> On 2017/03/23 23:47, Amit Langote wrote:
>>> On Thu, Mar 23, 2017 at 11:27 PM, Maksim Milyutin
>>> <m(dot)milyutin(at)postgrespro(dot)ru> wrote:
>>>> Hi!
>>>>
>>>> I have noticed that there is scheduled unlinking of nonexistent physical
>>>> storage under partitioned table when we execute DROP TABLE statement on this
>>>> partitioned table. Though this action doesn't generate any error under
>>>> typical behavior of postgres because the error of storage's lack is caught
>>>> through if-statement [1] I think it is not safe.
>>>>
>>>> My patch fixes this issue.
>>>>
>>>> 1. src/backend/storage/smgr/md.c:1385
>>>
>>> Good catch, will incorporate that in the main patch.
>>
>> And here is the updated patch.
>
> I think you should go back to the earlier strategy of disallowing heap
> reloptions to be set on the partitioned table. The thing is, we don't
> really know which way we're going to want to go in the future. Maybe
> we'll come up with a system where you can set options on the
> partitioned table, and those options will cascade to the children. Or
> maybe we'll come up with a system where partitioned tables have a
> completely different set of options to control behaviors specific to
> partitioned tables. If we do the latter, then we don't want to also
> have to support useless heap reloptions for forward compatibility, nor
> do we want to break backward-compatibility to remove support. If we
> do the former, then it's better if we allow it in the same release
> where it starts working.

You're right, modified the patch accordingly.

By the way, the previous version of the patch didn't really "disallow"
specifying heap reloptions though. What I'd imagine that should entail is
CREATE TABLE or ALTER TABLE raising error if one of those reloptions is
specified, which didn't really occur with the patch. The options were
silently accepted and stored into pg_class, but their values were never
used. I modified the patch so that an error occurs instead of silently
accepting the user input.

create table p (a int) partition by list (a) with (fillfactor = 10);
ERROR: unrecognized parameter "fillfactor" for a partitioned table

Thanks,
Amit

Attachment Content-Type Size
0001-Do-not-allocate-storage-for-partitioned-tables.patch text/x-diff 9.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2017-03-29 06:53:53 Re: Prologue of set_append_rel_size() and partitioned tables
Previous Message Amit Kapila 2017-03-29 06:39:28 Re: Patch: Write Amplification Reduction Method (WARM)