From: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Euler Taveira de Oliveira <euler(at)timbira(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: portability of "designated initializers" |
Date: | 2008-12-10 20:17:22 |
Message-ID: | 20081210201722.GI5503@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> 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.
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Davis | 2008-12-10 20:26:52 | Re: Sync Rep: First Thoughts on Code |
Previous Message | Simon Riggs | 2008-12-10 20:11:59 | Re: Sync Rep: First Thoughts on Code |