Fabrízio de Royes Mello escribió:
> On Fri, Mar 7, 2014 at 5:56 PM, Alvaro Herrera 
> <alvhe...@2ndquadrant.com>wrote:
> 
> > I am reworking this patch, both to accomodate earlier review comments
> > and to fix the multiple verify step of namespaces, as noted by Tom in
> > 4530.1390023...@sss.pgh.pa.us
> >
> Alvaro,
> 
> Do you need some help?

Would you give this version of the patch a look?  I reworked your patch
a bit, mainly to add a bit of checking to custom namespaces.  In this
code, before you can add a namespaced option to an object, you need to
register the namespace.  There are two interfaces for this: C code can
call registerOptionNamespace().  This patch adds a call to plpgsql that
does so (It's not my intention that plpgsql is modified in any way by
this patch; this part of the patch is here just for demonstration purposes.)

I expect most extension modules would do things that way.

The other interface for namespace registration is a SQL-callable
function.  I expect Jim Nasby to do something like
  SELECT pg_register_option_namespace('decibel');
  ALTER TABLE nasby SET (decibel.seed = true);
which seems to cover what he wanted.

If you register a namespace, you can later do "ALTER TABLE .. SET
(yournsp.foobar=blah)" and your value will be stored in catalogs.  Note
that if you have a C module, you can register the options themselves,
using add_bool_reloption() and so on; that way, your option will be
type-checked.  If you don't "add" your options, they will not be
checked.  This is in line with what we do for custom GUCs: if we know
about them, they are checked, otherwise we just pass them around
untouched.

Note one weird thing related to TOAST tables: we need to tell
transformRelOptions specifically whether we want custom namespaces to be
kept in its output or not.  This is because we want such options in the
main table, but not in the toast table; and we call transformRelOptions
on both tables with the whole array of values.  That's what the new
"include_custom" bit is for.  For most callers that bit is true, but
when a table is being processed and the options are for the toast table,
that option is false.

Another thing to note is that I've separated the checking of the
namespaces from the transformation.  There's actually very little
duplicated work that we're saving from doing things that way AFAICS, but
the new interface does make more sense than the old one.  This is per
the thread I linked to previously.  (Note there is surely a better way
to do the HEAP_RELOPT_NAMESPACES than a #define with the "static const
char * const valid[]" thingy sprinkled all over the place; I assume we
can just declare that once in the header.  I will fix that later.)


I haven't touched pg_dump yet, but if this proposed design sits well
with everyone, my intention is that the dump output will contain the
pg_register_option_namespace() calls necessary so that a table
definition will be able to do the SET calls to set the values the
original table has, and succeed.  In other words, restoring a dump will
preserve the values you had, without a need of having the module loaded
in the new server.  I think this is what was discussed.  Robert, do you
agree?

I think there is more work to do here, mainly to ensure that the
internal representation makes sense for C users (i.e. can extensions get
at the values they previously set).  At this point I'm interested in
getting no objections to the SQL interface and the pg_dump bits.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/doc/src/sgml/ref/alter_index.sgml
--- b/doc/src/sgml/ref/alter_index.sgml
***************
*** 82,87 **** ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> RESE
--- 82,93 ----
        <xref linkend="SQL-REINDEX">
        to get the desired effects.
       </para>
+      <note>
+        <para>
+          Custom storage parameters are of the form namespace.option. See
+          example below.
+        </para>
+      </note>
      </listitem>
     </varlistentry>
  
***************
*** 202,207 **** ALTER INDEX distributors SET (fillfactor = 75);
--- 208,219 ----
  REINDEX INDEX distributors;
  </programlisting></para>
  
+   <para>
+    To set a custom storage parameter:
+ <programlisting>
+ ALTER INDEX distributors
+   SET (somenamespace.optionname=true);
+ 
   </refsect1>
  
   <refsect1>
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
***************
*** 213,218 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
--- 213,226 ----
        of statistics by the <productname>PostgreSQL</productname> query
        planner, refer to <xref linkend="planner-stats">.
       </para>
+ 
+      <note>
+       <para>
+         Custom storage parameters are of the form namespace.option. See
+         example below.
+       </para>
+      </note>
+ 
      </listitem>
     </varlistentry>
  
***************
*** 476,481 **** ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
--- 484,493 ----
         <command>ALTER TABLE</> does not treat <literal>OIDS</> as a
         storage parameter.  Instead use the <literal>SET WITH OIDS</>
         and <literal>SET WITHOUT OIDS</> forms to change OID status.
+        A custom name can be used as namespace to define a storage parameter.
+        Storage option pattern: namespace.option=value
+        (namespace=custom name, option=option name and value=option value).
+        See example bellow.
        </para>
       </note>
      </listitem>
***************
*** 1112,1117 **** ALTER TABLE distributors DROP CONSTRAINT distributors_pkey,
--- 1124,1143 ----
      ADD CONSTRAINT distributors_pkey PRIMARY KEY USING INDEX dist_id_temp_idx;
  </programlisting></para>
  
+   <para>
+    To set a custom per-attribute option:
+ <programlisting>
+ ALTER TABLE distributors
+   ALTER COLUMN dist_id SET (somenamespace.optionname=true);
+ </programlisting>
+ </para>
+ 
+   <para>
+    To set a custom storage parameter:
+ <programlisting>
+ ALTER TABLE distributors
+   SET (somenamespace.optionname=true);
+ 
   </refsect1>
  
   <refsect1>
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 25,30 ****
--- 25,31 ----
  #include "commands/defrem.h"
  #include "commands/tablespace.h"
  #include "commands/view.h"
+ #include "fmgr.h"
  #include "nodes/makefuncs.h"
  #include "utils/array.h"
  #include "utils/attoptcache.h"
***************
*** 303,312 **** static int	num_custom_options = 0;
--- 304,318 ----
  static relopt_gen **custom_options = NULL;
  static bool need_initialization = true;
  
+ static List *customNamespaces = NIL;
+ 
+ 
  static void initialize_reloptions(void);
  static void parse_one_reloption(relopt_value *option, char *text_str,
  					int text_len, bool validate);
  
+ static bool is_custom_namespace(char *namspace);
+ 
  /*
   * initialize_reloptions
   *		initialization routine, must be called before parsing
***************
*** 575,584 **** add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
  }
  
  /*
   * Transform a relation options list (list of DefElem) into the text array
   * format that is kept in pg_class.reloptions, including only those options
!  * that are in the passed namespace.  The output values do not include the
!  * namespace.
   *
   * This is used for three cases: CREATE TABLE/INDEX, ALTER TABLE SET, and
   * ALTER TABLE RESET.  In the ALTER cases, oldOptions is the existing
--- 581,695 ----
  }
  
  /*
+  * Enable reloptions to be created in a new custom namespace.
+  */
+ void
+ registerOptionNamespace(const char *namspace)
+ {
+ 	MemoryContext	oldcxt;
+ 
+ 	oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+ 
+ 	customNamespaces = lcons(pstrdup(namspace), customNamespaces);
+ 
+ 	MemoryContextSwitchTo(oldcxt);
+ }
+ 
+ Datum
+ pg_register_option_namespace(PG_FUNCTION_ARGS)
+ {
+ 	text *nspname = PG_GETARG_TEXT_P(0);
+ 
+ 	registerOptionNamespace(TextDatumGetCString(nspname));
+ 
+ 	PG_RETURN_VOID();
+ }
+ 
+ static bool
+ is_custom_namespace(char *namspace)
+ {
+ 	ListCell   *cell;
+ 
+ 	foreach(cell, customNamespaces)
+ 	{
+ 		char *custom = lfirst(cell);
+ 
+ 		if (pg_strcasecmp(custom, namspace) == 0)
+ 			return true;
+ 	}
+ 
+ 	return false;
+ }
+ 
+ /*
+  * Verify that all the options in the given defList belong into one of the
+  * given valid namespaces (which might be NULL).  The NULL namespace is always
+  * valid, even if not listed.
+  *
+  * If there's a value with an invalid namespace, this function does not return.
+  */
+ void
+ checkOptionNamespaces(List *defList, const char * const validnsps[])
+ {
+ 	ListCell   *cell;
+ 
+ 	foreach(cell, defList)
+ 	{
+ 		DefElem	   *def = (DefElem *) lfirst(cell);
+ 		bool		valid = false;
+ 		ListCell   *cell2;
+ 		int			i;
+ 
+ 		/* An option in the NULL namespace is always valid */
+ 		if (def->defnamespace == NULL)
+ 			continue;
+ 
+ 		/* Options in the given hardcoded namespaces are valid too */
+ 		if (validnsps)
+ 			for (i = 0; validnsps[i] != NULL; i++)
+ 			{
+ 				if (strcmp(def->defnamespace, validnsps[i]) == 0)
+ 				{
+ 					valid = true;
+ 					break;
+ 				}
+ 			}
+ 		if (valid)
+ 			continue;
+ 
+ 		/*
+ 		 * Options in custom namespaces are valid.  Note that these namespaces
+ 		 * are valid for any type of option, regardless of object class or
+ 		 * kind that they are attached to (as opposed to the hardcoded
+ 		 * namespaces, which are specific to certain classes or kinds.)
+ 		 */
+ 		foreach(cell2, customNamespaces)
+ 		{
+ 			char *custom = (char *) lfirst(cell2);
+ 
+ 			if (strcmp(def->defnamespace, custom) == 0)
+ 			{
+ 				valid = true;
+ 				break;
+ 			}
+ 		}
+ 		if (valid)
+ 			continue;
+ 
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ 				 errmsg("unrecognized parameter namespace \"%s\"",
+ 						def->defnamespace)));
+ 	}
+ }
+ 
+ /*
   * Transform a relation options list (list of DefElem) into the text array
   * format that is kept in pg_class.reloptions, including only those options
!  * that are either in the passed namespace, or in any custom namespace if
!  * caller requested that.  For options that are in the namespace given as
!  * parameter, the names do not include the namespace; options in custom
!  * namespaces do include it.
   *
   * This is used for three cases: CREATE TABLE/INDEX, ALTER TABLE SET, and
   * ALTER TABLE RESET.  In the ALTER cases, oldOptions is the existing
***************
*** 589,605 **** add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
   * in the list (it will be or has been handled by interpretOidsOption()).
   *
   * Note that this is not responsible for determining whether the options
!  * are valid, but it does check that namespaces for all the options given are
!  * listed in validnsps.  The NULL namespace is always valid and need not be
!  * explicitly listed.  Passing a NULL pointer means that only the NULL
!  * namespace is valid.
   *
   * Both oldOptions and the result are text arrays (or NULL for "default"),
   * but we declare them as Datums to avoid including array.h in reloptions.h.
   */
  Datum
  transformRelOptions(Datum oldOptions, List *defList, char *namspace,
! 					char *validnsps[], bool ignoreOids, bool isReset)
  {
  	Datum		result;
  	ArrayBuildState *astate;
--- 700,713 ----
   * in the list (it will be or has been handled by interpretOidsOption()).
   *
   * Note that this is not responsible for determining whether the options
!  * are valid, or their namespaces, are valid.
   *
   * Both oldOptions and the result are text arrays (or NULL for "default"),
   * but we declare them as Datums to avoid including array.h in reloptions.h.
   */
  Datum
  transformRelOptions(Datum oldOptions, List *defList, char *namspace,
! 					bool include_custom, bool ignoreOids, bool isReset)
  {
  	Datum		result;
  	ArrayBuildState *astate;
***************
*** 634,656 **** transformRelOptions(Datum oldOptions, List *defList, char *namspace,
  			/* Search for a match in defList */
  			foreach(cell, defList)
  			{
! 				DefElem    *def = (DefElem *) lfirst(cell);
  				int			kw_len;
  
  				/* ignore if not in the same namespace */
! 				if (namspace == NULL)
  				{
! 					if (def->defnamespace != NULL)
  						continue;
  				}
- 				else if (def->defnamespace == NULL)
- 					continue;
- 				else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
- 					continue;
  
! 				kw_len = strlen(def->defname);
  				if (text_len > kw_len && text_str[kw_len] == '=' &&
! 					pg_strncasecmp(text_str, def->defname, kw_len) == 0)
  					break;
  			}
  			if (!cell)
--- 742,781 ----
  			/* Search for a match in defList */
  			foreach(cell, defList)
  			{
! 				DefElem	   *def = (DefElem *) lfirst(cell);
  				int			kw_len;
+ 				char	   *text_compare;
+ 				bool		customnsp;
+ 
+ 				/* ignore if custom and caller requested it */
+ 				customnsp = def->defnamespace ?
+ 					is_custom_namespace(def->defnamespace) : false;
+ 				if (customnsp && !include_custom)
+ 					continue;
  
  				/* ignore if not in the same namespace */
! 				if (!customnsp)
  				{
! 					if (namspace == NULL)
! 					{
! 						if (def->defnamespace != NULL)
! 							continue;
! 					}
! 					else if (def->defnamespace == NULL)
! 						continue;
! 					else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
  						continue;
  				}
  
! 				if (customnsp)
! 					text_compare = psprintf("%s.%s",
! 											def->defnamespace, def->defname);
! 				else
! 					text_compare = psprintf("%s", def->defname);
! 				kw_len = strlen(text_compare);
! 
  				if (text_len > kw_len && text_str[kw_len] == '=' &&
! 					pg_strncasecmp(text_str, text_compare, kw_len) == 0)
  					break;
  			}
  			if (!cell)
***************
*** 684,747 **** transformRelOptions(Datum oldOptions, List *defList, char *namspace,
  			text	   *t;
  			const char *value;
  			Size		len;
  
  			/*
! 			 * Error out if the namespace is not valid.  A NULL namespace is
! 			 * always valid.
  			 */
! 			if (def->defnamespace != NULL)
  			{
! 				bool		valid = false;
! 				int			i;
! 
! 				if (validnsps)
  				{
! 					for (i = 0; validnsps[i]; i++)
! 					{
! 						if (pg_strcasecmp(def->defnamespace,
! 										  validnsps[i]) == 0)
! 						{
! 							valid = true;
! 							break;
! 						}
! 					}
  				}
! 
! 				if (!valid)
! 					ereport(ERROR,
! 							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! 							 errmsg("unrecognized parameter namespace \"%s\"",
! 									def->defnamespace)));
! 			}
! 
! 			if (ignoreOids && pg_strcasecmp(def->defname, "oids") == 0)
! 				continue;
! 
! 			/* ignore if not in the same namespace */
! 			if (namspace == NULL)
! 			{
! 				if (def->defnamespace != NULL)
  					continue;
  			}
- 			else if (def->defnamespace == NULL)
- 				continue;
- 			else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
- 				continue;
  
  			/*
  			 * Flatten the DefElem into a text string like "name=arg". If we
  			 * have just "name", assume "name=true" is meant.  Note: the
! 			 * namespace is not output.
  			 */
  			if (def->arg != NULL)
  				value = defGetString(def);
  			else
  				value = "true";
  			len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
  			/* +1 leaves room for sprintf's trailing null */
  			t = (text *) palloc(len + 1);
  			SET_VARSIZE(t, len);
! 			sprintf(VARDATA(t), "%s=%s", def->defname, value);
  
  			astate = accumArrayResult(astate, PointerGetDatum(t),
  									  false, TEXTOID,
--- 809,869 ----
  			text	   *t;
  			const char *value;
  			Size		len;
+ 			bool		customnsp;
+ 
+ 			/* ignore "oids" option in the null namespace, if requested */
+ 			if (ignoreOids &&
+ 				def->defnamespace == NULL &&
+ 				pg_strcasecmp(def->defname, "oids") == 0)
+ 				continue;
+ 
+ 			/* ignore if custom and caller requested it */
+ 			customnsp = def->defnamespace ?
+ 				is_custom_namespace(def->defnamespace) : false;
+ 			if (customnsp && !include_custom)
+ 				continue;
  
  			/*
! 			 * Check namespace: either both are NULL, or they are identical,
! 			 * or defnamespace is registered as a custom namespace.
  			 */
! 			if (!customnsp)
  			{
! 				if (namspace == NULL)
  				{
! 					if (def->defnamespace != NULL)
! 						continue;
  				}
! 				else if (def->defnamespace == NULL)
! 					continue;
! 				else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
  					continue;
  			}
  
  			/*
  			 * Flatten the DefElem into a text string like "name=arg". If we
  			 * have just "name", assume "name=true" is meant.  Note: the
! 			 * namespace is not output, unless it's a custom one.
  			 */
+ 
+ 			/* XXX can we just psprintf all this? */
  			if (def->arg != NULL)
  				value = defGetString(def);
  			else
  				value = "true";
  			len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
+ 			if (customnsp)
+ 				len += strlen(def->defnamespace) + 1;
+ 
  			/* +1 leaves room for sprintf's trailing null */
  			t = (text *) palloc(len + 1);
  			SET_VARSIZE(t, len);
! 
! 			if (customnsp)
! 				sprintf(VARDATA(t), "%s.%s=%s", def->defnamespace,
! 						def->defname, value);
! 			else
! 				sprintf(VARDATA(t), "%s=%s", def->defname, value);
  
  			astate = accumArrayResult(astate, PointerGetDatum(t),
  									  false, TEXTOID,
***************
*** 795,800 **** untransformRelOptions(Datum options)
--- 917,923 ----
  			*p++ = '\0';
  			val = (Node *) makeString(pstrdup(p));
  		}
+ 		/* XXX need to handle namespaced names here? */
  		result = lappend(result, makeDefElem(pstrdup(s), val));
  	}
  
***************
*** 943,957 **** parseRelOptions(Datum options, bool validate, relopt_kind kind,
--- 1066,1102 ----
  				}
  			}
  
+ 			/*
+ 			 * If we couldn't find the option and we were asked to validate,
+ 			 * it's time to complain about it.
+ 			 */
  			if (j >= numoptions && validate)
  			{
  				char	   *s;
+ 				char	   *n;
  				char	   *p;
  
  				s = TextDatumGetCString(optiondatums[i]);
  				p = strchr(s, '=');
  				if (p)
+ 				{
  					*p = '\0';
+ 					n = strchr(s, '.');
+ 					if (n)
+ 					{
+ 						char	   *nsp;
+ 
+ 						nsp = palloc(n - s + 1);
+ 						memcpy(nsp, s, n - s);
+ 						nsp[n - s] = '\0';
+ 						if (is_custom_namespace(nsp))
+ 						{
+ 							pfree(nsp);
+ 							continue;
+ 						}
+ 					}
+ 				}
+ 
  				ereport(ERROR,
  						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
  						 errmsg("unrecognized parameter \"%s\"", s)));
***************
*** 1086,1092 **** allocateReloptStruct(Size base, relopt_value *options, int numoptions)
   * basesize is the sizeof(struct) that was passed to allocateReloptStruct.
   * options, of length numoptions, is parseRelOptions' output.
   * elems, of length numelems, is the table describing the allowed options.
!  * When validate is true, it is expected that all options appear in elems.
   */
  void
  fillRelOptions(void *rdopts, Size basesize,
--- 1231,1239 ----
   * basesize is the sizeof(struct) that was passed to allocateReloptStruct.
   * options, of length numoptions, is parseRelOptions' output.
   * elems, of length numelems, is the table describing the allowed options.
!  *
!  * When validate is true, it is expected that all options appear in elems,
!  * except those that are in a custom namespace.
   */
  void
  fillRelOptions(void *rdopts, Size basesize,
***************
*** 1102,1107 **** fillRelOptions(void *rdopts, Size basesize,
--- 1249,1255 ----
  		int			j;
  		bool		found = false;
  
+ 
  		for (j = 0; j < numelems; j++)
  		{
  			if (pg_strcasecmp(options[i].gen->name, elems[j].optname) == 0)
***************
*** 1154,1162 **** fillRelOptions(void *rdopts, Size basesize,
  				break;
  			}
  		}
  		if (validate && !found)
! 			elog(ERROR, "reloption \"%s\" not found in parse table",
! 				 options[i].gen->name);
  	}
  	SET_VARSIZE(rdopts, offset);
  }
--- 1302,1347 ----
  				break;
  			}
  		}
+ 
  		if (validate && !found)
! 		{
! 			/*
! 			 * Before complaining, see if the option is in a custom
! 			 * namespace.
! 			 */
! 			if (list_length(customNamespaces) > 0)
! 			{
! 				char *dot = strchr(options[i].gen->name, '.');
! 
! 				if (dot != NULL)
! 				{
! 					char *optnamespace;
! 					int		nsplen;
! 					ListCell *cell;
! 
! 					nsplen = dot - options[i].gen->name;
! 					optnamespace = palloc(nsplen + 1);
! 					memcpy(optnamespace, options[i].gen->name, nsplen);
! 					optnamespace[nsplen] = '\0';
! 
! 					foreach(cell, customNamespaces)
! 					{
! 						char *customnsp = lfirst(cell);
! 
! 						if (pg_strcasecmp(customnsp, optnamespace) == 0)
! 						{
! 							found = true;
! 							break;
! 						}
! 					}
! 					pfree(optnamespace);
! 				}
! 			}
! 
! 			if (!found)
! 				elog(ERROR, "reloption \"%s\" not found in parse table",
! 					 options[i].gen->name);
! 		}
  	}
  	SET_VARSIZE(rdopts, offset);
  }
*** a/src/backend/commands/createas.c
--- b/src/backend/commands/createas.c
***************
*** 264,270 **** intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
  	Datum		toast_options;
  	ListCell   *lc;
  	int			attnum;
! 	static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
  
  	Assert(into != NULL);		/* else somebody forgot to set it */
  
--- 264,270 ----
  	Datum		toast_options;
  	ListCell   *lc;
  	int			attnum;
! 	static const char * const validnsps[] = HEAP_RELOPT_NAMESPACES;
  
  	Assert(into != NULL);		/* else somebody forgot to set it */
  
***************
*** 365,375 **** intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
  	CommandCounterIncrement();
  
  	/* parse and validate reloptions for the toast table */
  	toast_options = transformRelOptions((Datum) 0,
  										create->options,
  										"toast",
! 										validnsps,
! 										true, false);
  
  	(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
  
--- 365,375 ----
  	CommandCounterIncrement();
  
  	/* parse and validate reloptions for the toast table */
+ 	checkOptionNamespaces(create->options, validnsps);
  	toast_options = transformRelOptions((Datum) 0,
  										create->options,
  										"toast",
! 										false, true, false);
  
  	(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
  
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
***************
*** 519,526 **** DefineIndex(Oid relationId,
  	/*
  	 * Parse AM-specific options, convert to text array form, validate.
  	 */
  	reloptions = transformRelOptions((Datum) 0, stmt->options,
! 									 NULL, NULL, false, false);
  
  	(void) index_reloptions(amoptions, reloptions, true);
  
--- 519,527 ----
  	/*
  	 * Parse AM-specific options, convert to text array form, validate.
  	 */
+ 	checkOptionNamespaces(stmt->options, NULL);
  	reloptions = transformRelOptions((Datum) 0, stmt->options,
! 									 NULL, true, false, false);
  
  	(void) index_reloptions(amoptions, reloptions, true);
  
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 450,456 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
  	Datum		reloptions;
  	ListCell   *listptr;
  	AttrNumber	attnum;
! 	static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
  	Oid			ofTypeId;
  
  	/*
--- 450,456 ----
  	Datum		reloptions;
  	ListCell   *listptr;
  	AttrNumber	attnum;
! 	static const char * const validnsps[] = HEAP_RELOPT_NAMESPACES;
  	Oid			ofTypeId;
  
  	/*
***************
*** 531,538 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
  	/*
  	 * Parse and validate reloptions, if any.
  	 */
! 	reloptions = transformRelOptions((Datum) 0, stmt->options, NULL, validnsps,
! 									 true, false);
  
  	(void) heap_reloptions(relkind, reloptions, true);
  
--- 531,539 ----
  	/*
  	 * Parse and validate reloptions, if any.
  	 */
! 	checkOptionNamespaces(stmt->options, validnsps);
! 	reloptions = transformRelOptions((Datum) 0, stmt->options, NULL,
! 									 true, true, false);
  
  	(void) heap_reloptions(relkind, reloptions, true);
  
***************
*** 5194,5201 **** ATExecSetOptions(Relation rel, const char *colName, Node *options,
  	Assert(IsA(options, List));
  	datum = SysCacheGetAttr(ATTNAME, tuple, Anum_pg_attribute_attoptions,
  							&isnull);
  	newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 									 (List *) options, NULL, NULL, false,
  									 isReset);
  	/* Validate new options */
  	(void) attribute_reloptions(newOptions, true);
--- 5195,5203 ----
  	Assert(IsA(options, List));
  	datum = SysCacheGetAttr(ATTNAME, tuple, Anum_pg_attribute_attoptions,
  							&isnull);
+ 	checkOptionNamespaces((List *) options, NULL);
  	newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 									 (List *) options, NULL, true, false,
  									 isReset);
  	/* Validate new options */
  	(void) attribute_reloptions(newOptions, true);
***************
*** 8802,8808 **** ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
  	Datum		repl_val[Natts_pg_class];
  	bool		repl_null[Natts_pg_class];
  	bool		repl_repl[Natts_pg_class];
! 	static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
  
  	if (defList == NIL && operation != AT_ReplaceRelOptions)
  		return;					/* nothing to do */
--- 8804,8810 ----
  	Datum		repl_val[Natts_pg_class];
  	bool		repl_null[Natts_pg_class];
  	bool		repl_repl[Natts_pg_class];
! 	static const char *const validnsps[] = HEAP_RELOPT_NAMESPACES;
  
  	if (defList == NIL && operation != AT_ReplaceRelOptions)
  		return;					/* nothing to do */
***************
*** 8832,8839 **** ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
  	}
  
  	/* Generate new proposed reloptions (text array) */
  	newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 									 defList, NULL, validnsps, false,
  									 operation == AT_ResetRelOptions);
  
  	/* Validate */
--- 8834,8842 ----
  	}
  
  	/* Generate new proposed reloptions (text array) */
+ 	checkOptionNamespaces(defList, validnsps);
  	newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 									 defList, NULL, true, false,
  									 operation == AT_ResetRelOptions);
  
  	/* Validate */
***************
*** 8950,8957 **** ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
  									&isnull);
  		}
  
  		newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 										 defList, "toast", validnsps, false,
  										 operation == AT_ResetRelOptions);
  
  		(void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
--- 8953,8961 ----
  									&isnull);
  		}
  
+ 		/* option namespaces were already checked above */
  		newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 										 defList, "toast", false, false,
  										 operation == AT_ResetRelOptions);
  
  		(void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 326,334 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
  
  	/* Generate new proposed spcoptions (text array) */
  	newOptions = transformRelOptions((Datum) 0,
  									 stmt->options,
! 									 NULL, NULL, false, false);
  	(void) tablespace_reloptions(newOptions, true);
  	if (newOptions != (Datum) 0)
  		values[Anum_pg_tablespace_spcoptions - 1] = newOptions;
--- 326,335 ----
  	nulls[Anum_pg_tablespace_spcacl - 1] = true;
  
  	/* Generate new proposed spcoptions (text array) */
+ 	checkOptionNamespaces(stmt->options, NULL);
  	newOptions = transformRelOptions((Datum) 0,
  									 stmt->options,
! 									 NULL, true, false, false);
  	(void) tablespace_reloptions(newOptions, true);
  	if (newOptions != (Datum) 0)
  		values[Anum_pg_tablespace_spcoptions - 1] = newOptions;
***************
*** 944,951 **** AlterTableSpaceOptions(AlterTableSpaceOptionsStmt *stmt)
  	/* Generate new proposed spcoptions (text array) */
  	datum = heap_getattr(tup, Anum_pg_tablespace_spcoptions,
  						 RelationGetDescr(rel), &isnull);
  	newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 									 stmt->options, NULL, NULL, false,
  									 stmt->isReset);
  	(void) tablespace_reloptions(newOptions, true);
  
--- 945,953 ----
  	/* Generate new proposed spcoptions (text array) */
  	datum = heap_getattr(tup, Anum_pg_tablespace_spcoptions,
  						 RelationGetDescr(rel), &isnull);
+ 	checkOptionNamespaces(stmt->options, NULL);
  	newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
! 									 stmt->options, NULL, true, false,
  									 stmt->isReset);
  	(void) tablespace_reloptions(newOptions, true);
  
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
***************
*** 900,906 **** ProcessUtilitySlow(Node *parsetree,
  						if (IsA(stmt, CreateStmt))
  						{
  							Datum		toast_options;
! 							static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
  
  							/* Create the table itself */
  							relOid = DefineRelation((CreateStmt *) stmt,
--- 900,908 ----
  						if (IsA(stmt, CreateStmt))
  						{
  							Datum		toast_options;
! 							List	   *options;
! 							static const char * const validnsps[] =
! 								HEAP_RELOPT_NAMESPACES;
  
  							/* Create the table itself */
  							relOid = DefineRelation((CreateStmt *) stmt,
***************
*** 917,926 **** ProcessUtilitySlow(Node *parsetree,
  							 * parse and validate reloptions for the toast
  							 * table
  							 */
  							toast_options = transformRelOptions((Datum) 0,
! 											  ((CreateStmt *) stmt)->options,
  																"toast",
! 																validnsps,
  																true,
  																false);
  							(void) heap_reloptions(RELKIND_TOASTVALUE,
--- 919,930 ----
  							 * parse and validate reloptions for the toast
  							 * table
  							 */
+ 							options = (List *) ((CreateStmt *) stmt)->options;
+ 							checkOptionNamespaces(options, validnsps);
  							toast_options = transformRelOptions((Datum) 0,
! 																options,
  																"toast",
! 																false,
  																true,
  																false);
  							(void) heap_reloptions(RELKIND_TOASTVALUE,
*** a/src/include/access/reloptions.h
--- b/src/include/access/reloptions.h
***************
*** 250,258 **** extern void add_real_reloption(bits32 kinds, char *name, char *desc,
  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,
! 					char *namspace, char *validnsps[],
! 					bool ignoreOids, bool isReset);
  extern List *untransformRelOptions(Datum options);
  extern bytea *extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
  				  Oid amoptions);
--- 250,261 ----
  extern void add_string_reloption(bits32 kinds, char *name, char *desc,
  					 char *default_val, validate_string_relopt validator);
  
+ extern void registerOptionNamespace(const char *namspace);
+ extern void checkOptionNamespaces(List *defList,
+ 					  const char * const validnsps[]);
  extern Datum transformRelOptions(Datum oldOptions, List *defList,
! 					char *namspace, bool include_custom, bool ignoreOids,
! 					bool isReset);
  extern List *untransformRelOptions(Datum options);
  extern bytea *extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
  				  Oid amoptions);
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***************
*** 4829,4834 **** DESCR("peek at binary changes from replication slot");
--- 4829,4837 ----
  DATA(insert OID = 3566 (  pg_event_trigger_dropped_objects		PGNSP PGUID 12 10 100 0 0 f f f f t t s 0 0 2249 "" "{26,26,23,25,25,25,25}" "{o,o,o,o,o,o,o}" "{classid, objid, objsubid, object_type, schema_name, object_name, object_identity}" _null_ pg_event_trigger_dropped_objects _null_ _null_ _null_ ));
  DESCR("list objects dropped by the current command");
  
+ DATA(insert OID = 3567 (  pg_register_option_namespace          PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 2278 "25" _null_ _null_ _null_ _null_ pg_register_option_namespace _null_ _null_ _null_ ));
+ DESCR("register a new namespace for custom options");
+ 
  /* generic transition functions for ordered-set aggregates */
  DATA(insert OID = 3970 ( ordered_set_transition			PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2281 "2281 2276" _null_ _null_ _null_ _null_ ordered_set_transition _null_ _null_ _null_ ));
  DESCR("aggregate transition function");
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
***************
*** 1136,1141 **** extern Datum window_first_value(PG_FUNCTION_ARGS);
--- 1136,1144 ----
  extern Datum window_last_value(PG_FUNCTION_ARGS);
  extern Datum window_nth_value(PG_FUNCTION_ARGS);
  
+ /* access/common/reloptions.c */
+ extern Datum pg_register_option_namespace(PG_FUNCTION_ARGS);
+ 
  /* access/spgist/spgquadtreeproc.c */
  extern Datum spg_quad_config(PG_FUNCTION_ARGS);
  extern Datum spg_quad_choose(PG_FUNCTION_ARGS);
*** a/src/pl/plpgsql/src/pl_handler.c
--- b/src/pl/plpgsql/src/pl_handler.c
***************
*** 16,21 ****
--- 16,22 ----
  #include "plpgsql.h"
  
  #include "access/htup_details.h"
+ #include "access/reloptions.h"
  #include "catalog/pg_proc.h"
  #include "catalog/pg_type.h"
  #include "funcapi.h"
***************
*** 57,62 **** _PG_init(void)
--- 58,67 ----
  	if (inited)
  		return;
  
+ 	registerOptionNamespace("plpgsql");
+ 	add_bool_reloption(RELOPT_KIND_HEAP,
+ 					   "plpgsql.foo", "description of foo", true);
+ 
  	pg_bindtextdomain(TEXTDOMAIN);
  
  	DefineCustomEnumVariable("plpgsql.variable_conflict",
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 2370,2372 **** TRUNCATE old_system_table;
--- 2370,2477 ----
  ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey;
  ALTER TABLE old_system_table DROP COLUMN othercol;
  DROP TABLE old_system_table;
+ --
+ -- Custom Option Test
+ --
+ CREATE TABLE custom_reloption_test(id SERIAL PRIMARY KEY);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+           relname           | reloptions 
+ ----------------------------+------------
+  custom_reloption_test      | 
+  custom_reloption_test.id   | 
+  custom_reloption_test_pkey | 
+ (3 rows)
+ 
+ ALTER TABLE custom_reloption_test SET (fillfactor=70);
+ ALTER INDEX custom_reloption_test_pkey SET (fillfactor=80);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (n_distinct=100);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+           relname           |    reloptions    
+ ----------------------------+------------------
+  custom_reloption_test      | {fillfactor=70}
+  custom_reloption_test.id   | {n_distinct=100}
+  custom_reloption_test_pkey | {fillfactor=80}
+ (3 rows)
+ 
+ SELECT pg_register_option_namespace('xx');
+  pg_register_option_namespace 
+ ------------------------------
+  
+ (1 row)
+ 
+ ALTER TABLE custom_reloption_test SET (xx.bdr.do_replicate=true);
+ ERROR:  syntax error at or near "."
+ LINE 1: ALTER TABLE custom_reloption_test SET (xx.bdr.do_replicate=t...
+                                                      ^
+ ALTER INDEX custom_reloption_test_pkey SET (xx.bdr.do_replicate=true);
+ ERROR:  syntax error at or near "."
+ LINE 1: ALTER INDEX custom_reloption_test_pkey SET (xx.bdr.do_replic...
+                                                           ^
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr.do_replicate=true);
+ ERROR:  syntax error at or near "."
+ LINE 1: ... custom_reloption_test ALTER COLUMN id SET (xx.bdr.do_replic...
+                                                              ^
+ ALTER TABLE custom_reloption_test SET (xx.bdr=true);
+ ALTER INDEX custom_reloption_test_pkey SET (xx.bdr=true);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr=true);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+           relname           |          reloptions          
+ ----------------------------+------------------------------
+  custom_reloption_test      | {fillfactor=70,xx.bdr=true}
+  custom_reloption_test.id   | {n_distinct=100,xx.bdr=true}
+  custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true}
+ (3 rows)
+ 
+ -- test extension reloptions
+ ALTER TABLE custom_reloption_test SET (plpgsql.option=true);
+ ALTER INDEX custom_reloption_test_pkey SET (plpgsql.option=true);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.option=true);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+           relname           |                    reloptions                    
+ ----------------------------+--------------------------------------------------
+  custom_reloption_test      | {fillfactor=70,xx.bdr=true,plpgsql.option=true}
+  custom_reloption_test.id   | {n_distinct=100,xx.bdr=true,plpgsql.option=true}
+  custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true,plpgsql.option=true}
+ (3 rows)
+ 
+ ALTER TABLE custom_reloption_test SET (plpgsql.foo=1234);
+ ERROR:  invalid value for boolean option "plpgsql.foo": 1234
+ ALTER INDEX custom_reloption_test_pkey SET (plpgsql.foo=1234);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.foo=1234);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+           relname           |                            reloptions                             
+ ----------------------------+-------------------------------------------------------------------
+  custom_reloption_test      | {fillfactor=70,xx.bdr=true,plpgsql.option=true}
+  custom_reloption_test.id   | {n_distinct=100,xx.bdr=true,plpgsql.option=true,plpgsql.foo=1234}
+  custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true,plpgsql.option=true,plpgsql.foo=1234}
+ (3 rows)
+ 
+ ALTER TABLE custom_reloption_test RESET (plpgsql.foo);
+ ALTER INDEX custom_reloption_test_pkey RESET (plpgsql.foo);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id RESET (plpgsql.foo);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+           relname           |                    reloptions                    
+ ----------------------------+--------------------------------------------------
+  custom_reloption_test      | {fillfactor=70,xx.bdr=true,plpgsql.option=true}
+  custom_reloption_test.id   | {n_distinct=100,xx.bdr=true,plpgsql.option=true}
+  custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true,plpgsql.option=true}
+ (3 rows)
+ 
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1594,1596 **** TRUNCATE old_system_table;
--- 1594,1645 ----
  ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey;
  ALTER TABLE old_system_table DROP COLUMN othercol;
  DROP TABLE old_system_table;
+ 
+ --
+ -- Custom Option Test
+ --
+ CREATE TABLE custom_reloption_test(id SERIAL PRIMARY KEY);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+ ALTER TABLE custom_reloption_test SET (fillfactor=70);
+ ALTER INDEX custom_reloption_test_pkey SET (fillfactor=80);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (n_distinct=100);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+ SELECT pg_register_option_namespace('xx');
+ ALTER TABLE custom_reloption_test SET (xx.bdr.do_replicate=true);
+ ALTER INDEX custom_reloption_test_pkey SET (xx.bdr.do_replicate=true);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr.do_replicate=true);
+ ALTER TABLE custom_reloption_test SET (xx.bdr=true);
+ ALTER INDEX custom_reloption_test_pkey SET (xx.bdr=true);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr=true);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+ -- test extension reloptions
+ ALTER TABLE custom_reloption_test SET (plpgsql.option=true);
+ ALTER INDEX custom_reloption_test_pkey SET (plpgsql.option=true);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.option=true);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+ ALTER TABLE custom_reloption_test SET (plpgsql.foo=1234);
+ ALTER INDEX custom_reloption_test_pkey SET (plpgsql.foo=1234);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.foo=1234);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
+ ALTER TABLE custom_reloption_test RESET (plpgsql.foo);
+ ALTER INDEX custom_reloption_test_pkey RESET (plpgsql.foo);
+ ALTER TABLE custom_reloption_test ALTER COLUMN id RESET (plpgsql.foo);
+ SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+ UNION ALL
+ SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ ORDER BY 1;
-- 
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