fillfactor gets set to zero for toast tables
in [PgSql]
|
Prev: Generalized Inverted Generalized Search Tree
Next: [HACKERS] Generating Lots of PKs with nextval(): A Feature Proposal
From: Alvaro Herrera on 14 May 2010 14:56 Excerpts from Tom Lane's message of vie may 14 14:19:30 -0400 2010: > The problem is that if any reloption is set for the toast table, > we build a StdRdOptions struct in which fillfactor is zero, and > then all the code that actually uses fillfactor honors that. > And the reason fillfactor gets left as zero is that there is no > entry for it in the tables for toast-table reloptions. Clearly, > this wasn't thought through carefully enough. Ouch :-( We messed with this stuff after the initial commit because I didn't get it right the first time either. I'm surprised that we didn't find this problem right away. > I'm inclined to think that we should remove the notion of there > being separate sets of rdoptions for heaps and toast tables --- > any case in which someone tries to omit a StdRdOption for toast > tables is going to fail the same way, unless we are fortunate > enough that zero is a desirable default. It doesn't seem like having the distinction has bought us anything. However, if we do that, we can't add a separate entry to intRelOpts to fix min/max/default to 100, so I think we should document that options for toast must match those for heaps. > What seems more rational is to provide toast.fillfactor but give it > min/max/default = 100. Adding an entry to intRelOpts should be enough to fix this bug, correct? -- -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 14 May 2010 15:03 Alvaro Herrera <alvherre(a)commandprompt.com> writes: > Excerpts from Tom Lane's message of vie may 14 14:19:30 -0400 2010: >> What seems more rational is to provide toast.fillfactor but give it >> min/max/default = 100. > Adding an entry to intRelOpts should be enough to fix this bug, correct? I think so, but haven't tested. The docs would need some correction perhaps. BTW, I notice that the code allows people to fool with autovacuum_analyze_threshold and related parameters for toast tables, which seems rather pointless since we never analyze toast tables. So the fillfactor isn't really the only instance of the issue. Maybe a better solution is to have some kind of notion of a default-only entry, which is sufficient to insert the default into the struct but isn't accepted as a user-settable item. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Alvaro Herrera on 18 May 2010 16:44 Excerpts from Tom Lane's message of vie may 14 15:03:57 -0400 2010: > Maybe a better solution is to have some kind of notion of a default-only > entry, which is sufficient to insert the default into the struct but > isn't accepted as a user-settable item. This patch (for 8.4, but applies fuzzily to 9.0) implements this idea. Note that there's no explicit check that every heap option has a corresponding toast option; that's left to the developer's judgement to add. I added the new member to relopt_gen struct so that existing entries did not require changes in initializers. (I'll leave the error as ERRCODE_CANT_CHANGE_RUNTIME_PARAM unless someone thinks ERRCODE_INVALID_PARAMETER_VALUE is better) I checked and this is the only heap setting that did not have a toast setting. I'll leave this open to comments for a bit more time than usual, to give room for pgcon beers and such ... --
From: Takahiro Itagaki on 26 May 2010 03:32 Alvaro Herrera <alvherre(a)commandprompt.com> wrote: > Excerpts from Tom Lane's message of vie may 14 15:03:57 -0400 2010: > > > Maybe a better solution is to have some kind of notion of a default-only > > entry, which is sufficient to insert the default into the struct but > > isn't accepted as a user-settable item. > > This patch (for 8.4, but applies fuzzily to 9.0) implements this idea. > Note that there's no explicit check that every heap option has a > corresponding toast option; that's left to the developer's judgement to > add. I added the new member to relopt_gen struct so that existing > entries did not require changes in initializers. The new "default_only" field can be initialized only from the internal codes and is not exported to user definded reloptions. We could add an additional argument to add_xxx_reloption() functions, but it breaks ABI. How about the attached patch? It just fills fillfactor (and analyze_threshold) to default values for TOAST relations. I think responsibility for filling reloptions with proper values is not in the generic option routines but in AM-specific reloption handlers. Regards, --- Takahiro Itagaki NTT Open Source Software Center
From: Takahiro Itagaki on 30 May 2010 20:59
This is still an open item for 9.0, and we should also backport it to 8.4. Which do we take on? Or is there better idea? Alvaro's idea: > > > Maybe a better solution is to have some kind of notion of a default-only > > > entry, which is sufficient to insert the default into the struct but > > > isn't accepted as a user-settable item. Itagaki's idea: > It just fills fillfactor (and analyze_threshold) > to default values for TOAST relations. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |