Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > I've already modified your patch a bit ... please send your updated
> > patch so I can merge it into mine.  However, my changes were also
> > relatively minor.  Since Tom wants it to be entirely rewritten then
> > maybe merging minor fixes to it is a waste of time ...
> 
> The thing I'm complaining about is having dropped the intermediate
> struct that represents the fully decoded set of reloptions.  If you
> put that back I won't object to a table-driven approach to doing the
> decoding.

Do we need a struct, or can we get away with storing the values directly
in RelationData?  Something like this:

typedef struct RelationData
{
        ...

        /*
         * Parsed values from reloptions.  These are set whenever rd_rel is 
loaded
         * into the relcache entry.
         */
        bool    autovacuum_enabled;
        int             fillfactor;
        int             autovacuum_vacuum_threshold;
        int             autovacuum_analyze_threshold;
        int             autovacuum_vacuum_cost_delay;
        int             autovacuum_vacuum_cost_limit;
        int             autovacuum_freeze_min_age;
        int             autovacuum_freeze_max_age;
        float8  autovacuum_vacuum_scale_factor;
        float8  autovacuum_analyze_scale_factor;

One problem is that this enlarges Relation by a fair bit for every
relation, even those that are never concerned with autovacuum, e.g.
indexes.  I'm not sure how much is this a problem in practice.  (It
could become a problem for people who has thousands of partitions, but
we don't actually support that.)

I have also modified the patch to include the default value for each
option in the table; to solve the problem of differing fillfactors for
the different index AMs, I've created separated "kinds" this way:

/* kind supported by reloptions */
#define RELOPT_KIND_NULL        (1 << 0)
#define RELOPT_KIND_HEAP        (1 << 1)
#define RELOPT_KIND_BTREE       (1 << 2)
#define RELOPT_KIND_HASH        (1 << 3)
#define RELOPT_KIND_GIN         (1 << 4)
#define RELOPT_KIND_GIST        (1 << 5)

so we can do something like this:

        {
                {
                        "fillfactor",
                        "Packs table pages only to this percentage",
                        RELOPT_TYPE_INT, RELOPT_KIND_HEAP
                },
                HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
        },
        {
                {
                        "fillfactor",
                        "Packs btree index pages only to this percentage",
                        RELOPT_TYPE_INT, RELOPT_KIND_BTREE
                },
                BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
        },
         ... more for the other AMs ...


Is that approach OK?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to