Re: Partitioned tables and relfilenode

Hello,

At Thu, 30 Mar 2017 18:24:16 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote in <ee5f1cd4-4783-42e8-0263-648ae9c11264(at)lab(dot)ntt(dot)co(dot)jp>
> On 2017/03/29 23:58, Robert Haas wrote:
> > On Wed, Mar 29, 2017 at 4:21 AM, Amit Langote
> > <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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