Hi,

We can specify the unit when setting autovacuum_vacuum_cost_delay
GUC as follows.

    ALTER SYSTEM SET autovacuum_vacuum_cost_delay TO '80ms';

OTOH we cannot specify the unit when setting autovacuum_vacuum_cost_delay
as storage parameter as follows.

    CREATE TABLE test (col1 int) WITH (autovacuum_vacuum_cost_delay = '80ms');
    ERROR:  invalid value for integer option
"autovacuum_vacuum_cost_delay": 80ms

This is not user-friendly. I'd like to propose the attached patch which
introduces the infrastructure which allows us to specify the unit when
setting INTEGER storage parameter like autovacuum_vacuum_cost_delay.
Comment? Review?

Regards,

-- 
Fujii Masao
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 105,111 **** static relopt_int intRelOpts[] =
  			"Packs btree index pages only to this percentage",
  			RELOPT_KIND_BTREE
  		},
! 		BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
  	},
  	{
  		{
--- 105,111 ----
  			"Packs btree index pages only to this percentage",
  			RELOPT_KIND_BTREE
  		},
! 		BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100, 0
  	},
  	{
  		{
***************
*** 113,119 **** static relopt_int intRelOpts[] =
  			"Packs hash index pages only to this percentage",
  			RELOPT_KIND_HASH
  		},
! 		HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
  	},
  	{
  		{
--- 113,119 ----
  			"Packs hash index pages only to this percentage",
  			RELOPT_KIND_HASH
  		},
! 		HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100, 0
  	},
  	{
  		{
***************
*** 121,127 **** static relopt_int intRelOpts[] =
  			"Packs gist index pages only to this percentage",
  			RELOPT_KIND_GIST
  		},
! 		GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
  	},
  	{
  		{
--- 121,127 ----
  			"Packs gist index pages only to this percentage",
  			RELOPT_KIND_GIST
  		},
! 		GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100, 0
  	},
  	{
  		{
***************
*** 129,135 **** static relopt_int intRelOpts[] =
  			"Packs spgist index pages only to this percentage",
  			RELOPT_KIND_SPGIST
  		},
! 		SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100
  	},
  	{
  		{
--- 129,135 ----
  			"Packs spgist index pages only to this percentage",
  			RELOPT_KIND_SPGIST
  		},
! 		SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100, 0
  	},
  	{
  		{
***************
*** 137,143 **** static relopt_int intRelOpts[] =
  			"Minimum number of tuple updates or deletes prior to vacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, INT_MAX
  	},
  	{
  		{
--- 137,143 ----
  			"Minimum number of tuple updates or deletes prior to vacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, INT_MAX, 0
  	},
  	{
  		{
***************
*** 145,151 **** static relopt_int intRelOpts[] =
  			"Minimum number of tuple inserts, updates or deletes prior to analyze",
  			RELOPT_KIND_HEAP
  		},
! 		-1, 0, INT_MAX
  	},
  	{
  		{
--- 145,151 ----
  			"Minimum number of tuple inserts, updates or deletes prior to analyze",
  			RELOPT_KIND_HEAP
  		},
! 		-1, 0, INT_MAX, 0
  	},
  	{
  		{
***************
*** 153,159 **** static relopt_int intRelOpts[] =
  			"Vacuum cost delay in milliseconds, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, 100
  	},
  	{
  		{
--- 153,159 ----
  			"Vacuum cost delay in milliseconds, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, 100, GUC_UNIT_MS
  	},
  	{
  		{
***************
*** 161,167 **** static relopt_int intRelOpts[] =
  			"Vacuum cost amount available before napping, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 1, 10000
  	},
  	{
  		{
--- 161,167 ----
  			"Vacuum cost amount available before napping, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 1, 10000, 0
  	},
  	{
  		{
***************
*** 169,175 **** static relopt_int intRelOpts[] =
  			"Minimum age at which VACUUM should freeze a table row, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, 1000000000
  	},
  	{
  		{
--- 169,175 ----
  			"Minimum age at which VACUUM should freeze a table row, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, 1000000000, 0
  	},
  	{
  		{
***************
*** 177,183 **** static relopt_int intRelOpts[] =
  			"Minimum multixact age at which VACUUM should freeze a row multixact's, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, 1000000000
  	},
  	{
  		{
--- 177,183 ----
  			"Minimum multixact age at which VACUUM should freeze a row multixact's, for autovacuum",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 0, 1000000000, 0
  	},
  	{
  		{
***************
*** 185,191 **** static relopt_int intRelOpts[] =
  			"Age at which to autovacuum a table to prevent transaction ID wraparound",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 100000000, 2000000000
  	},
  	{
  		{
--- 185,191 ----
  			"Age at which to autovacuum a table to prevent transaction ID wraparound",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 100000000, 2000000000, 0
  	},
  	{
  		{
***************
*** 193,213 **** static relopt_int intRelOpts[] =
  			"Multixact age at which to autovacuum a table to prevent multixact wraparound",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 100000000, 2000000000
  	},
  	{
  		{
  			"autovacuum_freeze_table_age",
  			"Age at which VACUUM should perform a full table sweep to freeze row versions",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! 		}, -1, 0, 2000000000
  	},
  	{
  		{
  			"autovacuum_multixact_freeze_table_age",
  			"Age of multixact at which VACUUM should perform a full table sweep to freeze row versions",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! 		}, -1, 0, 2000000000
  	},
  
  	/* list terminator */
--- 193,213 ----
  			"Multixact age at which to autovacuum a table to prevent multixact wraparound",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
  		},
! 		-1, 100000000, 2000000000, 0
  	},
  	{
  		{
  			"autovacuum_freeze_table_age",
  			"Age at which VACUUM should perform a full table sweep to freeze row versions",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! 		}, -1, 0, 2000000000, 0
  	},
  	{
  		{
  			"autovacuum_multixact_freeze_table_age",
  			"Age of multixact at which VACUUM should perform a full table sweep to freeze row versions",
  			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
! 		}, -1, 0, 2000000000, 0
  	},
  
  	/* list terminator */
***************
*** 503,509 **** add_bool_reloption(bits32 kinds, char *name, char *desc, bool default_val)
   */
  void
  add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
! 				  int min_val, int max_val)
  {
  	relopt_int *newoption;
  
--- 503,509 ----
   */
  void
  add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
! 				  int min_val, int max_val, int flags_val)
  {
  	relopt_int *newoption;
  
***************
*** 512,517 **** add_int_reloption(bits32 kinds, char *name, char *desc, int default_val,
--- 512,518 ----
  	newoption->default_val = default_val;
  	newoption->min = min_val;
  	newoption->max = max_val;
+ 	newoption->flags = flags_val;
  
  	add_reloption((relopt_gen *) newoption);
  }
***************
*** 1003,1014 **** parse_one_reloption(relopt_value *option, char *text_str, int text_len,
  		case RELOPT_TYPE_INT:
  			{
  				relopt_int *optint = (relopt_int *) option->gen;
  
! 				parsed = parse_int(value, &option->values.int_val, 0, NULL);
  				if (validate && !parsed)
  					ereport(ERROR,
  					   (errmsg("invalid value for integer option \"%s\": %s",
! 							   option->gen->name, value)));
  				if (validate && (option->values.int_val < optint->min ||
  								 option->values.int_val > optint->max))
  					ereport(ERROR,
--- 1004,1018 ----
  		case RELOPT_TYPE_INT:
  			{
  				relopt_int *optint = (relopt_int *) option->gen;
+ 				const char *hintmsg;
  
! 				parsed = parse_int(value, &option->values.int_val,
! 								   optint->flags, &hintmsg);
  				if (validate && !parsed)
  					ereport(ERROR,
  					   (errmsg("invalid value for integer option \"%s\": %s",
! 							   option->gen->name, value),
! 						hintmsg ? errhint("%s", _(hintmsg)) : 0));
  				if (validate && (option->values.int_val < optint->min ||
  								 option->values.int_val > optint->max))
  					ereport(ERROR,
*** a/src/include/access/reloptions.h
--- b/src/include/access/reloptions.h
***************
*** 92,97 **** typedef struct relopt_int
--- 92,98 ----
  	int			default_val;
  	int			min;
  	int			max;
+ 	int			flags;
  } relopt_int;
  
  typedef struct relopt_real
***************
*** 244,250 **** 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,
--- 245,251 ----
  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, int flags_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,
-- 
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