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

Hello,

At Thu, 30 Mar 2017 18:24:16 +0900, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote in
<ee5f1cd4-4783-42e8-0263-648ae9c11264@lab.ntt.co.jp>
> On 2017/03/29 23:58, Robert Haas wrote:
> > On Wed, Mar 29, 2017 at 4:21 AM, Amit Langote
> > <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> >> Looks correct, so incorporated in the attached updated patch.  Thanks.
> > 
> > This seems like a hacky way to limit the reloptions to just OIDs.
> > Shouldn't we instead have a new RELOPT_KIND_PARTITIONED or something
> > like that?
> 
> OK, I tried that in the updated patch.

The name RELOPT_KIND_PARTITIONED looks somewhat odd. RELKIND for
partitioned tables is RELKIND_PARTITIONED_TABLE, so is this
better to be _TABLE, even if a bit longer?

parseRelOptions seems to return garbage pointer if no option of
the kind is available.

> DefineRelation() and ATExecSetRelOptions() still calls heap_reloptions(),
> but passes the new relopt_kind.  None of the options listed in
> reloptions.c are of this kind as of now, so parseRelOptions() simply
> outputs the "unrecognized parameter %s" in the case of partitioned tables
> (except in some cases described below, but not because of the limitations
> of this patch).  If and when partitioned tables start supporting the
> existing parameters, we'll update the relopt_gen.kinds bitmask of those
> parameters to allow them to be specified for partitioned tables.
> 
> With the patch:
> 
> create table parted (a int) partition by list (a) with (fillfactor = 10);
> ERROR:  unrecognized parameter "fillfactor"
> 
> create table parted (a int) partition by list (a) with (toast.fillfactor =
> 10);
> ERROR:  unrecognized parameter "fillfactor"
> 
> and:
> 
> create table parted (a int) partition by list (a) with (oids = true);
> alter table parted set (fillfactor = 90);
> ERROR:  unrecognized parameter "fillfactor"
> 
> but:
> 
> -- appears to succeed, whereas an error should have been reported (I think)
> alter table parted set (toast.fillfactor = 10);
> ALTER TABLE
> 
> -- even with views
> create table foo (a int);
> create view foov with (toast.fillfactor = 10) as select * from foo;
> CREATE VIEW
> alter view foov set (toast.fillfactor = 20);
> ALTER VIEW
> 
> Nothing is stored in pg_class.reloptions really, but the validation that
> should have occurred in parseRelOptions() doesn't.  This happens because
> of the way transformRelOptions() works.  It will ignore the DefElem's that
> don't apply to the specified relation based on the value of the namspace
> parameter ("toast" or NULL).  That means it won't be included in the array
> of options later received by parseRelOptions(), which is where the
> validation occurs.
> 
> Also, alter table reset (xxx) never validates anything:
> 
> alter table foo reset (foo);
> ALTER TABLE
> alter table foo reset (foo.bar);
> ALTER TABLE
> 
> Again, no pg_class.reloptions update occurs in this case. The reason this
> happens is because transformRelOptions() never includes the options to be
> reset in the array of options received by parseRelOptions(), so no
> validation occurs.
> 
> But since both are existing behaviors, perhaps we can worry about it some
> other time.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center