Re: Partitioned tables and relfilenode - Mailing list pgsql-hackers

From Amit Langote
Subject Re: Partitioned tables and relfilenode
Date
Msg-id c4d71df2-9e0e-3912-dc81-9a72e080c238@lab.ntt.co.jp
Whole thread Raw
In response to Re: Partitioned tables and relfilenode  (Robert Haas <robertmhaas@gmail.com>)
List 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@lab.ntt.co.jp> wrote:
>> On 2017/03/23 23:47, Amit Langote wrote:
>>> On Thu, Mar 23, 2017 at 11:27 PM, Maksim Milyutin
>>> <m.milyutin@postgrespro.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

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Patch: Write Amplification Reduction Method (WARM)
Next
From: Amit Langote
Date:
Subject: Re: Prologue of set_append_rel_size() and partitionedtables