On Thu, Feb 18, 2021 at 6:06 PM Hou, Zhijie <houzj.f...@cn.fujitsu.com> wrote:
> > Here we go, my first patch... solves
> > https://www.postgresql.org/message-id/7d6fdc20-857c-4cbe-ae2e-c0ff9520
> > e...@www.fastmail.com
> >
>
> Hi,
>
> partitioned_table_reloptions(Datum reloptions, bool validate)
>  {
> +       static const relopt_parse_elt tab[] = {
> +               {"parallel_workers", RELOPT_TYPE_INT,
> +               offsetof(StdRdOptions, parallel_workers)},
> +       };
> +
>         return (bytea *) build_reloptions(reloptions, validate,
>                                                                           
> RELOPT_KIND_PARTITIONED,
> -                                                                         0, 
> NULL, 0);
> +                                                                         
> sizeof(StdRdOptions),
> +                                                                         
> tab, lengthof(tab));
>  }
>
> I noticed that you plan to store the parallel_workers in the same 
> struct(StdRdOptions) as normal heap relation.
> It seems better to store it in a separate struct.
>
> And as commit 1bbd608 said:
> ----
> > Splitting things has the advantage to
> > make the information stored in rd_options include only the necessary
> > information, reducing the amount of memory used for a relcache entry
> > with partitioned tables if new reloptions are introduced at this level.
> ----
>
> What do you think?

That may be a good idea.  So instead of referring to the
parallel_workers in StdRdOptions, define a new one, say,
PartitionedTableRdOptions as follows and refer to it in
partitioned_table_reloptions():

typedef struct PartitionedTableRdOptions
{
    int32       vl_len_;        /* varlena header (do not touch directly!) */
    int         parallel_workers;   /* max number of parallel workers */
} PartitionedTableRdOptions;

-- 
Amit Langote
EDB: http://www.enterprisedb.com


Reply via email to