fillfactor for toast tables is useless?

Started by ITAGAKI Takahiroalmost 17 years ago4 messages
#1ITAGAKI Takahiro
itagaki.takahiro@oss.ntt.co.jp

With reloption patch, we can set WITH options to toast tables.
However, fillfactor for toast tables is useless, no?
(autovacuum options will work as expected, though.)

Tuples in toast tables are never updated. When the main tuple is updated,
old toast tuples are deleted and new ones are inserted. Even if there are
some freespaces in pages of toast table, they are never used by inserts.
I think we should not allow users to modify fillfactor for toast tables
and it should be always 100%.

We could optimize a delete+insert operation for toast tuples to one update
when the number of chunks are not changed by update.
Fillfactor for toast tables will be only useful after the optimization.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

#2Alvaro Herrera
alvherre@commandprompt.com
In reply to: ITAGAKI Takahiro (#1)
Re: fillfactor for toast tables is useless?

ITAGAKI Takahiro wrote:

With reloption patch, we can set WITH options to toast tables.
However, fillfactor for toast tables is useless, no?

Maybe what we should do is just reject fillfactor for toast tables for
now. I think this is easy to do.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#3ITAGAKI Takahiro
itagaki.takahiro@oss.ntt.co.jp
In reply to: Alvaro Herrera (#2)
1 attachment(s)
Re: fillfactor for toast tables is useless?

Alvaro Herrera <alvherre@commandprompt.com> wrote:

ITAGAKI Takahiro wrote:

With reloption patch, we can set WITH options to toast tables.
However, fillfactor for toast tables is useless, no?

Maybe what we should do is just reject fillfactor for toast tables for
now. I think this is easy to do.

Hmmm... this might have been discussed already, I think it would be better
to have relopt_kind not as a sequence number but as a bit flag.
If it was flags, we can reject fillfactor for toast tables in a natural way
without duplicated autovacuum_* definitions:

- fillfactor -> RELOPT_KIND_HEAP
- autovacuum_* -> RELOPT_KIND_HEAP | RELOPT_KIND_TOAST

26 entries are remained even after we use 6 kinds in the core.
(HEAP, TOAST, BTREE, HASH, GIN and GIST)

The attached is a patch to change 3 things:
- Reject toast.fillfactor.
- Modify relopt_kind to bit flags.
- Report relation type on "unrecognized parameter" errors.

Comments welcome.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center

Attachments:

reject_toast_fillfactor.patchapplication/octet-stream; name=reject_toast_fillfactor.patchDownload
diff -cpr head/src/backend/access/common/reloptions.c reject_toast_fillfactor/src/backend/access/common/reloptions.c
*** head/src/backend/access/common/reloptions.c	2009-02-10 11:21:34.582935000 +0900
--- reject_toast_fillfactor/src/backend/access/common/reloptions.c	2009-02-13 10:54:43.250000000 +0900
*************** static relopt_bool boolRelOpts[] =
*** 52,58 ****
  		{
  			"autovacuum_enabled",
  			"Enables autovacuum in this relation",
! 			RELOPT_KIND_HEAP
  		},
  		true
  	},
--- 52,58 ----
  		{
  			"autovacuum_enabled",
  			"Enables autovacuum in this relation",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		true
  	},
*************** static relopt_int intRelOpts[] =
*** 98,104 ****
  		{
  			"autovacuum_vacuum_threshold",
  			"Minimum number of tuple updates or deletes prior to vacuum",
! 			RELOPT_KIND_HEAP
  		},
  		50, 0, INT_MAX
  	},
--- 98,104 ----
  		{
  			"autovacuum_vacuum_threshold",
  			"Minimum number of tuple updates or deletes prior to vacuum",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		50, 0, INT_MAX
  	},
*************** static relopt_int intRelOpts[] =
*** 106,112 ****
  		{
  			"autovacuum_analyze_threshold",
  			"Minimum number of tuple inserts, updates or deletes prior to analyze",
! 			RELOPT_KIND_HEAP
  		},
  		50, 0, INT_MAX
  	},
--- 106,112 ----
  		{
  			"autovacuum_analyze_threshold",
  			"Minimum number of tuple inserts, updates or deletes prior to analyze",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		50, 0, INT_MAX
  	},
*************** static relopt_int intRelOpts[] =
*** 114,120 ****
  		{
  			"autovacuum_vacuum_cost_delay",
  			"Vacuum cost delay in milliseconds, for autovacuum",
! 			RELOPT_KIND_HEAP
  		},
  		20, 0, 1000
  	},
--- 114,120 ----
  		{
  			"autovacuum_vacuum_cost_delay",
  			"Vacuum cost delay in milliseconds, for autovacuum",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		20, 0, 1000
  	},
*************** static relopt_int intRelOpts[] =
*** 122,128 ****
  		{
  			"autovacuum_vacuum_cost_limit",
  			"Vacuum cost amount available before napping, for autovacuum",
! 			RELOPT_KIND_HEAP
  		},
  		200, 1, 10000
  	},
--- 122,128 ----
  		{
  			"autovacuum_vacuum_cost_limit",
  			"Vacuum cost amount available before napping, for autovacuum",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		200, 1, 10000
  	},
*************** static relopt_int intRelOpts[] =
*** 130,136 ****
  		{
  			"autovacuum_freeze_min_age",
  			"Minimum age at which VACUUM should freeze a table row, for autovacuum",
! 			RELOPT_KIND_HEAP
  		},
  		100000000, 0, 1000000000
  	},
--- 130,136 ----
  		{
  			"autovacuum_freeze_min_age",
  			"Minimum age at which VACUUM should freeze a table row, for autovacuum",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		100000000, 0, 1000000000
  	},
*************** static relopt_int intRelOpts[] =
*** 138,144 ****
  		{
  			"autovacuum_freeze_max_age",
  			"Age at which to autovacuum a table to prevent transaction ID wraparound",
! 			RELOPT_KIND_HEAP
  		},
  		200000000, 100000000, 2000000000
  	},
--- 138,144 ----
  		{
  			"autovacuum_freeze_max_age",
  			"Age at which to autovacuum a table to prevent transaction ID wraparound",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		200000000, 100000000, 2000000000
  	},
*************** static relopt_int intRelOpts[] =
*** 146,152 ****
  		{
  			"autovacuum_freeze_table_age",
  			"Age at which VACUUM should perform a full table sweep to replace old Xid values with FrozenXID",
! 			RELOPT_KIND_HEAP
  		}, 150000000, 0, 2000000000
  	},
  	/* list terminator */
--- 146,152 ----
  		{
  			"autovacuum_freeze_table_age",
  			"Age at which VACUUM should perform a full table sweep to replace old Xid values with FrozenXID",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		}, 150000000, 0, 2000000000
  	},
  	/* list terminator */
*************** static relopt_real realRelOpts[] =
*** 159,165 ****
  		{
  			"autovacuum_vacuum_scale_factor",
  			"Number of tuple updates or deletes prior to vacuum as a fraction of reltuples",
! 			RELOPT_KIND_HEAP
  		},
  		0.2, 0.0, 100.0
  	},
--- 159,165 ----
  		{
  			"autovacuum_vacuum_scale_factor",
  			"Number of tuple updates or deletes prior to vacuum as a fraction of reltuples",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		0.2, 0.0, 100.0
  	},
*************** static relopt_real realRelOpts[] =
*** 167,173 ****
  		{
  			"autovacuum_analyze_scale_factor",
  			"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
! 			RELOPT_KIND_HEAP
  		},
  		0.1, 0.0, 100.0
  	},
--- 167,173 ----
  		{
  			"autovacuum_analyze_scale_factor",
  			"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
! 			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
  		0.1, 0.0, 100.0
  	},
*************** static relopt_string stringRelOpts[] = 
*** 182,188 ****
  };
  
  static relopt_gen **relOpts = NULL;
! static int last_assigned_kind = RELOPT_KIND_LAST_DEFAULT + 1;
  
  static int		num_custom_options = 0;
  static relopt_gen **custom_options = NULL;
--- 182,188 ----
  };
  
  static relopt_gen **relOpts = NULL;
! static bits32 last_assigned_kind = RELOPT_KIND_LAST_DEFAULT << 1;
  
  static int		num_custom_options = 0;
  static relopt_gen **custom_options = NULL;
*************** initialize_reloptions(void)
*** 267,280 ****
   * 		Create a new relopt_kind value, to be used in custom reloptions by
   * 		user-defined AMs.
   */
! int
  add_reloption_kind(void)
  {
! 	if (last_assigned_kind >= RELOPT_KIND_MAX)
  		ereport(ERROR,
  				(errmsg("user-defined relation parameter types limit exceeded")));
  
! 	return last_assigned_kind++;
  }
  
  /*
--- 267,283 ----
   * 		Create a new relopt_kind value, to be used in custom reloptions by
   * 		user-defined AMs.
   */
! relopt_kind
  add_reloption_kind(void)
  {
! 	relopt_kind	kind;
! 	if (last_assigned_kind >= (bits32) RELOPT_KIND_MAX)
  		ereport(ERROR,
  				(errmsg("user-defined relation parameter types limit exceeded")));
  
! 	kind = (relopt_kind) last_assigned_kind;
! 	last_assigned_kind <<= 1;
! 	return kind;
  }
  
  /*
*************** add_reloption(relopt_gen *newoption)
*** 317,323 ****
   * 		(for types other than string)
   */
  static relopt_gen *
! allocate_reloption(int kind, int type, char *name, char *desc)
  {
  	MemoryContext	oldcxt;
  	size_t			size;
--- 320,326 ----
   * 		(for types other than string)
   */
  static relopt_gen *
! allocate_reloption(bits32 kinds, int type, char *name, char *desc)
  {
  	MemoryContext	oldcxt;
  	size_t			size;
*************** allocate_reloption(int kind, int type, c
*** 350,356 ****
  		newoption->desc = pstrdup(desc);
  	else
  		newoption->desc = NULL;
! 	newoption->kind = kind;
  	newoption->namelen = strlen(name);
  	newoption->type = type;
  
--- 353,359 ----
  		newoption->desc = pstrdup(desc);
  	else
  		newoption->desc = NULL;
! 	newoption->kinds = kinds;
  	newoption->namelen = strlen(name);
  	newoption->type = type;
  
*************** allocate_reloption(int kind, int type, c
*** 364,374 ****
   * 		Add a new boolean reloption
   */
  void
! add_bool_reloption(int kind, char *name, char *desc, bool default_val)
  {
  	relopt_bool	   *newoption;
  
! 	newoption = (relopt_bool *) allocate_reloption(kind, RELOPT_TYPE_BOOL,
  												   name, desc);
  	newoption->default_val = default_val;
  
--- 367,377 ----
   * 		Add a new boolean reloption
   */
  void
! add_bool_reloption(bits32 kinds, char *name, char *desc, bool default_val)
  {
  	relopt_bool	   *newoption;
  
! 	newoption = (relopt_bool *) allocate_reloption(kinds, RELOPT_TYPE_BOOL,
  												   name, desc);
  	newoption->default_val = default_val;
  
*************** add_bool_reloption(int kind, char *name,
*** 380,391 ****
   * 		Add a new integer reloption
   */
  void
! add_int_reloption(int kind, char *name, char *desc, int default_val,
  				  int min_val, int max_val)
  {
  	relopt_int	   *newoption;
  
! 	newoption = (relopt_int *) allocate_reloption(kind, RELOPT_TYPE_INT,
  												  name, desc);
  	newoption->default_val = default_val;
  	newoption->min = min_val;
--- 383,394 ----
   * 		Add a new integer reloption
   */
  void
! add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
  				  int min_val, int max_val)
  {
  	relopt_int	   *newoption;
  
! 	newoption = (relopt_int *) allocate_reloption(kinds, RELOPT_TYPE_INT,
  												  name, desc);
  	newoption->default_val = default_val;
  	newoption->min = min_val;
*************** add_int_reloption(int kind, char *name, 
*** 399,410 ****
   * 		Add a new float reloption
   */
  void
! add_real_reloption(int kind, char *name, char *desc, double default_val,
  				  double min_val, double max_val)
  {
  	relopt_real	   *newoption;
  
! 	newoption = (relopt_real *) allocate_reloption(kind, RELOPT_TYPE_REAL,
  												   name, desc);
  	newoption->default_val = default_val;
  	newoption->min = min_val;
--- 402,413 ----
   * 		Add a new float reloption
   */
  void
! add_real_reloption(bits32 kinds, char *name, char *desc, double default_val,
  				  double min_val, double max_val)
  {
  	relopt_real	   *newoption;
  
! 	newoption = (relopt_real *) allocate_reloption(kinds, RELOPT_TYPE_REAL,
  												   name, desc);
  	newoption->default_val = default_val;
  	newoption->min = min_val;
*************** add_real_reloption(int kind, char *name,
*** 423,429 ****
   * the validation.
   */
  void
! add_string_reloption(int kind, char *name, char *desc, char *default_val,
  					 validate_string_relopt validator)
  {
  	MemoryContext	oldcxt;
--- 426,432 ----
   * the validation.
   */
  void
! add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
  					 validate_string_relopt validator)
  {
  	MemoryContext	oldcxt;
*************** add_string_reloption(int kind, char *nam
*** 442,448 ****
  		newoption->gen.desc = pstrdup(desc);
  	else
  		newoption->gen.desc = NULL;
! 	newoption->gen.kind = kind;
  	newoption->gen.namelen = strlen(name);
  	newoption->gen.type = RELOPT_TYPE_STRING;
  	newoption->validate_cb = validator;
--- 445,451 ----
  		newoption->gen.desc = pstrdup(desc);
  	else
  		newoption->gen.desc = NULL;
! 	newoption->gen.kinds = kinds;
  	newoption->gen.namelen = strlen(name);
  	newoption->gen.type = RELOPT_TYPE_STRING;
  	newoption->validate_cb = validator;
*************** parseRelOptions(Datum options, bool vali
*** 776,782 ****
  	/* Build a list of expected options, based on kind */
  
  	for (i = 0; relOpts[i]; i++)
! 		if (relOpts[i]->kind == kind)
  			numoptions++;
  
  	if (numoptions == 0)
--- 779,785 ----
  	/* Build a list of expected options, based on kind */
  
  	for (i = 0; relOpts[i]; i++)
! 		if (relOpts[i]->kinds & kind)
  			numoptions++;
  
  	if (numoptions == 0)
*************** parseRelOptions(Datum options, bool vali
*** 789,795 ****
  
  	for (i = 0, j = 0; relOpts[i]; i++)
  	{
! 		if (relOpts[i]->kind == kind)
  		{
  			reloptions[j].gen = relOpts[i];
  			reloptions[j].isset = false;
--- 792,798 ----
  
  	for (i = 0, j = 0; relOpts[i]; i++)
  	{
! 		if (relOpts[i]->kinds & kind)
  		{
  			reloptions[j].gen = relOpts[i];
  			reloptions[j].isset = false;
*************** parseRelOptions(Datum options, bool vali
*** 837,850 ****
  			{
  				char	   *s;
  				char	   *p;
  
  				s = TextDatumGetCString(optiondatums[i]);
  				p = strchr(s, '=');
  				if (p)
  					*p = '\0';
  				ereport(ERROR,
  						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! 						 errmsg("unrecognized parameter \"%s\"", s)));
  			}
  		}
  	}
--- 840,878 ----
  			{
  				char	   *s;
  				char	   *p;
+ 				const char *kindname;
  
  				s = TextDatumGetCString(optiondatums[i]);
  				p = strchr(s, '=');
  				if (p)
  					*p = '\0';
+ 				switch (kind)
+ 				{
+ 					case RELOPT_KIND_HEAP:
+ 						kindname = "heap";
+ 						break;
+ 					case RELOPT_KIND_TOAST:
+ 						kindname = "toast";
+ 						break;
+ 					case RELOPT_KIND_BTREE:
+ 						kindname = "btree";
+ 						break;
+ 					case RELOPT_KIND_HASH:
+ 						kindname = "hash";
+ 						break;
+ 					case RELOPT_KIND_GIN:
+ 						kindname = "gin";
+ 						break;
+ 					case RELOPT_KIND_GIST:
+ 						kindname = "gist";
+ 						break;
+ 					default:
+ 						kindname = "user defined";
+ 						break;
+ 				}
  				ereport(ERROR,
  						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! 						 errmsg("unrecognized parameter \"%s\" for %s relation", s, kindname)));
  			}
  		}
  	}
*************** default_reloptions(Datum reloptions, boo
*** 1107,1113 ****
  bytea *
  heap_reloptions(char relkind, Datum reloptions, bool validate)
  {
! 	return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
  }
  
  
--- 1135,1147 ----
  bytea *
  heap_reloptions(char relkind, Datum reloptions, bool validate)
  {
! 	switch (relkind)
! 	{
! 	case RELKIND_TOASTVALUE:
! 		return default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
! 	default:
! 		return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
! 	}
  }
  
  
diff -cpr head/src/include/access/reloptions.h reject_toast_fillfactor/src/include/access/reloptions.h
*** head/src/include/access/reloptions.h	2009-02-04 10:05:45.855884000 +0900
--- reject_toast_fillfactor/src/include/access/reloptions.h	2009-02-13 10:54:43.250000000 +0900
*************** typedef enum relopt_type
*** 33,47 ****
  /* kinds supported by reloptions */
  typedef enum relopt_kind
  {
! 	RELOPT_KIND_HEAP,
! 	/* XXX do we need a separate kind for TOAST tables? */
! 	RELOPT_KIND_BTREE,
! 	RELOPT_KIND_HASH,
! 	RELOPT_KIND_GIN,
! 	RELOPT_KIND_GIST,
  	/* if you add a new kind, make sure you update "last_default" too */
  	RELOPT_KIND_LAST_DEFAULT = RELOPT_KIND_GIST,
! 	RELOPT_KIND_MAX = 255
  } relopt_kind;
  
  /* reloption namespaces allowed for heaps -- currently only TOAST */
--- 33,47 ----
  /* kinds supported by reloptions */
  typedef enum relopt_kind
  {
! 	RELOPT_KIND_HEAP	= (1 << 0),
! 	RELOPT_KIND_TOAST	= (1 << 1),
! 	RELOPT_KIND_BTREE	= (1 << 2),
! 	RELOPT_KIND_HASH	= (1 << 3),
! 	RELOPT_KIND_GIN		= (1 << 4),
! 	RELOPT_KIND_GIST	= (1 << 5),
  	/* if you add a new kind, make sure you update "last_default" too */
  	RELOPT_KIND_LAST_DEFAULT = RELOPT_KIND_GIST,
! 	RELOPT_KIND_MAX = (1 << 31)
  } relopt_kind;
  
  /* reloption namespaces allowed for heaps -- currently only TOAST */
*************** typedef struct relopt_gen
*** 52,58 ****
  {
  	const char *name;	/* must be first (used as list termination marker) */
  	const char *desc;
! 	relopt_kind	kind;
  	int			namelen;
  	relopt_type	type;
  } relopt_gen;
--- 52,58 ----
  {
  	const char *name;	/* must be first (used as list termination marker) */
  	const char *desc;
! 	bits32		kinds;
  	int			namelen;
  	relopt_type	type;
  } relopt_gen;
*************** typedef struct
*** 232,245 ****
  	 (char *)(optstruct) + (optstruct)->member)
  
  
! extern int add_reloption_kind(void);
! extern void add_bool_reloption(int kind, char *name, char *desc,
  				   bool default_val);
! extern void add_int_reloption(int kind, char *name, char *desc,
  				  int default_val, int min_val, int max_val);
! extern void add_real_reloption(int kind, char *name, char *desc,
  				   double default_val, double min_val, double max_val);
! extern void add_string_reloption(int kind, char *name, char *desc,
  					 char *default_val, validate_string_relopt validator);
  
  extern Datum transformRelOptions(Datum oldOptions, List *defList,
--- 232,245 ----
  	 (char *)(optstruct) + (optstruct)->member)
  
  
! extern relopt_kind add_reloption_kind(void);
! extern void add_bool_reloption(bits32 kinds, char *name, char *desc,
  				   bool default_val);
! extern void add_int_reloption(bits32 kinds, char *name, char *desc,
  				  int default_val, int min_val, int max_val);
! extern void add_real_reloption(bits32 kinds, char *name, char *desc,
  				   double default_val, double min_val, double max_val);
! extern void add_string_reloption(bits32 kinds, char *name, char *desc,
  					 char *default_val, validate_string_relopt validator);
  
  extern Datum transformRelOptions(Datum oldOptions, List *defList,
#4ITAGAKI Takahiro
itagaki.takahiro@oss.ntt.co.jp
In reply to: ITAGAKI Takahiro (#3)
Re: fillfactor for toast tables is useless?

ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> wrote:

The attached is a patch to change 3 things:
- Reject toast.fillfactor.
- Modify relopt_kind to bit flags.
- Report relation type on "unrecognized parameter" errors.

I registered the patch as an open item:
http://wiki.postgresql.org/wiki/PostgreSQL_8.4_Open_Items

Comments and discussions welcome.

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center