Remove the recently added node types ReloptElem and OptionDefElem in favor

of adding optional namespace and action fields to DefElem.  Having three
node types that do essentially the same thing bloats the code and leads
to errors of confusion, such as in yesterday's bug report from Khee Chin.

src/backend/access/common/reloptions.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/common/reloptions.c,v 1.25 2009/04/04 00:45:02 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/common/reloptions.c,v 1.26 2009/04/04 21:12:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -483,7 +483,7 @@ add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
 }
 
 /*
- * Transform a relation options list (list of ReloptElem) into the text array
+ * 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.
@@ -542,23 +542,23 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 			/* Search for a match in defList */
 			foreach(cell, defList)
 			{
-				ReloptElem *def = lfirst(cell);
+				DefElem	   *def = (DefElem *) lfirst(cell);
 				int			kw_len;
 
 				/* ignore if not in the same namespace */
 				if (namspace == NULL)
 				{
-					if (def->nmspc != NULL)
+					if (def->defnamespace != NULL)
 						continue;
 				}
-				else if (def->nmspc == NULL)
+				else if (def->defnamespace == NULL)
 					continue;
-				else if (pg_strcasecmp(def->nmspc, namspace) != 0)
+				else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
 					continue;
 
-				kw_len = strlen(def->optname);
+				kw_len = strlen(def->defname);
 				if (text_len > kw_len && text_str[kw_len] == '=' &&
-					pg_strncasecmp(text_str, def->optname, kw_len) == 0)
+					pg_strncasecmp(text_str, def->defname, kw_len) == 0)
 					break;
 			}
 			if (!cell)
@@ -578,8 +578,7 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 	 */
 	foreach(cell, defList)
 	{
-		ReloptElem    *def = lfirst(cell);
-
+		DefElem	   *def = (DefElem *) lfirst(cell);
 
 		if (isReset)
 		{
@@ -598,7 +597,7 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 			 * Error out if the namespace is not valid.  A NULL namespace
 			 * is always valid.
 			 */
-			if (def->nmspc != NULL)
+			if (def->defnamespace != NULL)
 			{
 				bool	valid = false;
 				int		i;
@@ -607,7 +606,8 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 				{
 					for (i = 0; validnsps[i]; i++)
 					{
-						if (pg_strcasecmp(def->nmspc, validnsps[i]) == 0)
+						if (pg_strcasecmp(def->defnamespace,
+										  validnsps[i]) == 0)
 						{
 							valid = true;
 							break;
@@ -619,37 +619,37 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 					ereport(ERROR,
 							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 							 errmsg("unrecognized parameter namespace \"%s\"",
-									def->nmspc)));
+									def->defnamespace)));
 			}
 
-			if (ignoreOids && pg_strcasecmp(def->optname, "oids") == 0)
+			if (ignoreOids && pg_strcasecmp(def->defname, "oids") == 0)
 				continue;
 
 			/* ignore if not in the same namespace */
 			if (namspace == NULL)
 			{
-				if (def->nmspc != NULL)
+				if (def->defnamespace != NULL)
 					continue;
 			}
-			else if (def->nmspc == NULL)
+			else if (def->defnamespace == NULL)
 				continue;
-			else if (pg_strcasecmp(def->nmspc, namspace) != 0)
+			else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
 				continue;
 
 			/*
-			 * Flatten the ReloptElem into a text string like "name=arg". If we
+			 * 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 = reloptGetString(def);
+				value = defGetString(def);
 			else
 				value = "true";
-			len = VARHDRSZ + strlen(def->optname) + 1 + strlen(value);
+			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->optname, value);
+			sprintf(VARDATA(t), "%s=%s", def->defname, value);
 
 			astate = accumArrayResult(astate, PointerGetDatum(t),
 									  false, TEXTOID,

src/backend/commands/define.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/define.c,v 1.103 2009/02/02 19:31:38 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/define.c,v 1.104 2009/04/04 21:12:31 tgl Exp $
  *
  * DESCRIPTION
  *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -55,20 +55,24 @@ case_translate_language_name(const char *input)
 }
 
 
-static char *
-nodeGetString(Node *value, char *name)
+/*
+ * Extract a string value (otherwise uninterpreted) from a DefElem.
+ */
+char *
+defGetString(DefElem *def)
 {
-	if (value == NULL)
+	if (def->arg == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("%s requires a parameter", name)));
-	switch (nodeTag(value))
+				 errmsg("%s requires a parameter",
+						def->defname)));
+	switch (nodeTag(def->arg))
 	{
 		case T_Integer:
 			{
 				char	   *str = palloc(32);
 
-				snprintf(str, 32, "%ld", (long) intVal(value));
+				snprintf(str, 32, "%ld", (long) intVal(def->arg));
 				return str;
 			}
 		case T_Float:
@@ -77,28 +81,19 @@ nodeGetString(Node *value, char *name)
 			 * T_Float values are kept in string form, so this type cheat
 			 * works (and doesn't risk losing precision)
 			 */
-			return strVal(value);
+			return strVal(def->arg);
 		case T_String:
-			return strVal(value);
+			return strVal(def->arg);
 		case T_TypeName:
-			return TypeNameToString((TypeName *) value);
+			return TypeNameToString((TypeName *) def->arg);
 		case T_List:
-			return NameListToString((List *) value);
+			return NameListToString((List *) def->arg);
 		default:
-			elog(ERROR, "unrecognized node type: %d", (int) nodeTag(value));
+			elog(ERROR, "unrecognized node type: %d", (int) nodeTag(def->arg));
 	}
 	return NULL;				/* keep compiler quiet */
 }
 
-/*
- * Extract a string value (otherwise uninterpreted) from a DefElem.
- */
-char *
-defGetString(DefElem *def)
-{
-	return nodeGetString(def->arg, def->defname);
-}
-
 /*
  * Extract a numeric value (actually double) from a DefElem.
  */
@@ -125,22 +120,25 @@ defGetNumeric(DefElem *def)
 	return 0;					/* keep compiler quiet */
 }
 
-static bool
-nodeGetBoolean(Node *value, char *name)
+/*
+ * Extract a boolean value from a DefElem.
+ */
+bool
+defGetBoolean(DefElem *def)
 {
 	/*
 	 * If no parameter given, assume "true" is meant.
 	 */
-	if (value == NULL)
+	if (def->arg == NULL)
 		return true;
 
 	/*
 	 * Allow 0, 1, "true", "false"
 	 */
-	switch (nodeTag(value))
+	switch (nodeTag(def->arg))
 	{
 		case T_Integer:
-			switch (intVal(value))
+			switch (intVal(def->arg))
 			{
 				case 0:
 					return false;
@@ -153,7 +151,7 @@ nodeGetBoolean(Node *value, char *name)
 			break;
 		default:
 			{
-				char	   *sval = nodeGetString(value, name);
+				char	   *sval = defGetString(def);
 
 				if (pg_strcasecmp(sval, "true") == 0)
 					return true;
@@ -165,19 +163,11 @@ nodeGetBoolean(Node *value, char *name)
 	}
 	ereport(ERROR,
 			(errcode(ERRCODE_SYNTAX_ERROR),
-			 errmsg("%s requires a Boolean value", name)));
+			 errmsg("%s requires a Boolean value",
+					def->defname)));
 	return false;				/* keep compiler quiet */
 }
 
-/*
- * Extract a boolean value from a DefElem.
- */
-bool
-defGetBoolean(DefElem *def)
-{
-	return nodeGetBoolean(def->arg, def->defname);
-}
-
 /*
  * Extract an int64 value from a DefElem.
  */
@@ -315,35 +305,11 @@ defGetTypeLength(DefElem *def)
 	return 0;					/* keep compiler quiet */
 }
 
-
 /*
- * Extract a string value (otherwise uninterpreted) from a ReloptElem.
+ * Create a DefElem setting "oids" to the specified value.
  */
-char *
-reloptGetString(ReloptElem *relopt)
+DefElem *
+defWithOids(bool value)
 {
-	return nodeGetString(relopt->arg, relopt->optname);
-}
-
-/*
- * Extract a boolean value from a ReloptElem.
- */
-bool
-reloptGetBoolean(ReloptElem *relopt)
-{
-	return nodeGetBoolean(relopt->arg, relopt->optname);
-}
-
-/*
- * Create a ReloptElem setting "oids" to the specified value.
- */
-ReloptElem *
-reloptWithOids(bool value)
-{
-	ReloptElem    *f = makeNode(ReloptElem);
-
-	f->optname = "oids";
-	f->nmspc = NULL;
-	f->arg = (Node *) makeInteger(value);
-	return f;
+	return makeDefElem("oids", (Node *) makeInteger(value));
 }

src/backend/commands/foreigncmds.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/foreigncmds.c,v 1.6 2009/02/24 10:06:32 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/foreigncmds.c,v 1.7 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -76,33 +76,31 @@ optionListToArray(List *options)
 
 
 /*
- * Transform the list of OptionDefElem into list of generic options.
- * The result is converted to array of text suitable for storing in
- * options.
+ * Transform a list of DefElem into text array format.  This is substantially
+ * the same thing as optionListToArray(), except we recognize SET/ADD/DROP
+ * actions for modifying an existing list of options, which is passed in
+ * Datum form as oldOptions.  Also, if fdwvalidator isn't InvalidOid
+ * it specifies a validator function to call on the result.
  *
  * Returns the array in the form of a Datum, or PointerGetDatum(NULL)
  * if the list is empty.
  *
- * This is used by CREATE/ALTER of FOREIGN DATA WRAPPER/SERVER/USER
- * MAPPING.  In the ALTER cases, oldOptions is the current text array
- * of options.
+ * This is used by CREATE/ALTER of FOREIGN DATA WRAPPER/SERVER/USER MAPPING.
  */
 static Datum
 transformGenericOptions(Datum oldOptions,
-						List *optionDefList,
-						GenericOptionFlags flags,
-						ForeignDataWrapper *fdw,
+						List *options,
 						Oid fdwvalidator)
 {
 	List	 *resultOptions = untransformRelOptions(oldOptions);
 	ListCell *optcell;
 	Datum	  result;
 
-	foreach(optcell, optionDefList)
+	foreach(optcell, options)
 	{
-		OptionDefElem	*od = lfirst(optcell);
-		ListCell	    *cell;
-		ListCell		*prev = NULL;
+		DefElem	   *od = lfirst(optcell);
+		ListCell   *cell;
+		ListCell   *prev = NULL;
 
 		/*
 		 * Find the element in resultOptions.  We need this for
@@ -112,7 +110,7 @@ transformGenericOptions(Datum oldOptions,
 		{
 			DefElem	*def = lfirst(cell);
 
-			if (strcmp(def->defname, od->def->defname) == 0)
+			if (strcmp(def->defname, od->defname) == 0)
 				break;
 			else
 				prev = cell;
@@ -121,41 +119,42 @@ transformGenericOptions(Datum oldOptions,
 		/*
 		 * It is possible to perform multiple SET/DROP actions on the
 		 * same option.  The standard permits this, as long as the
-		 * options to be added are unique.
+		 * options to be added are unique.  Note that an unspecified
+		 * action is taken to be ADD.
 		 */
-
-		switch (od->alter_op)
+		switch (od->defaction)
 		{
-			case ALTER_OPT_DROP:
+			case DEFELEM_DROP:
 				if (!cell)
 					ereport(ERROR,
 							(errcode(ERRCODE_UNDEFINED_OBJECT),
 							 errmsg("option \"%s\" not found",
-									od->def->defname)));
+									od->defname)));
 				resultOptions = list_delete_cell(resultOptions, cell, prev);
 				break;
 
-			case ALTER_OPT_SET:
+			case DEFELEM_SET:
 				if (!cell)
 					ereport(ERROR,
 							(errcode(ERRCODE_UNDEFINED_OBJECT),
 							 errmsg("option \"%s\" not found",
-									od->def->defname)));
-				lfirst(cell) = od->def;
+									od->defname)));
+				lfirst(cell) = od;
 				break;
 
-			case ALTER_OPT_ADD:
+			case DEFELEM_ADD:
+			case DEFELEM_UNSPEC:
 				if (cell)
 					ereport(ERROR,
 							(errcode(ERRCODE_DUPLICATE_OBJECT),
 							 errmsg("option \"%s\" provided more than once",
-									od->def->defname)));
-				resultOptions = lappend(resultOptions, od->def);
+									od->defname)));
+				resultOptions = lappend(resultOptions, od);
 				break;
 
 			default:
 				elog(ERROR, "unrecognized action %d on option \"%s\"",
-					 od->alter_op, od->def->defname);
+					 (int) od->defaction, od->defname);
 				break;
 		}
 	}
@@ -163,7 +162,7 @@ transformGenericOptions(Datum oldOptions,
 	result = optionListToArray(resultOptions);
 
 	if (fdwvalidator)
-		OidFunctionCall2(fdwvalidator, result, 0);
+		OidFunctionCall2(fdwvalidator, result, (Datum) 0);
 
 	return result;
 }
@@ -386,7 +385,6 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
 	nulls[Anum_pg_foreign_data_wrapper_fdwacl - 1] = true;
 
 	fdwoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
-										 FdwOpt, NULL,
 										 fdwvalidator);
 
 	if (PointerIsValid(DatumGetPointer(fdwoptions)))
@@ -504,8 +502,7 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
 			datum = PointerGetDatum(NULL);
 
 		/* Transform the options */
-		datum = transformGenericOptions(datum, stmt->options, FdwOpt,
-										NULL, fdwvalidator);
+		datum = transformGenericOptions(datum, stmt->options, fdwvalidator);
 
 		if (PointerIsValid(DatumGetPointer(datum)))
 			repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;
@@ -672,7 +669,6 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
 
 	/* Add server options */
 	srvoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
-										 ServerOpt, fdw,
 										 fdw->fdwvalidator);
 
 	if (PointerIsValid(DatumGetPointer(srvoptions)))
@@ -770,8 +766,8 @@ AlterForeignServer(AlterForeignServerStmt *stmt)
 			datum = PointerGetDatum(NULL);
 
 		/* Prepare the options array */
-		datum = transformGenericOptions(datum, stmt->options, ServerOpt,
-										fdw, fdw->fdwvalidator);
+		datum = transformGenericOptions(datum, stmt->options,
+										fdw->fdwvalidator);
 
 		if (PointerIsValid(DatumGetPointer(datum)))
 			repl_val[Anum_pg_foreign_server_srvoptions - 1] = datum;
@@ -942,8 +938,7 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
 
 	/* Add user options */
 	useoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
-										 UserMappingOpt,
-										 fdw, fdw->fdwvalidator);
+										 fdw->fdwvalidator);
 
 	if (PointerIsValid(DatumGetPointer(useoptions)))
 		values[Anum_pg_user_mapping_umoptions - 1] = useoptions;
@@ -1037,8 +1032,8 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
 			datum = PointerGetDatum(NULL);
 
 		/* Prepare the options array */
-		datum = transformGenericOptions(datum, stmt->options, UserMappingOpt,
-										fdw, fdw->fdwvalidator);
+		datum = transformGenericOptions(datum, stmt->options,
+										fdw->fdwvalidator);
 
 		if (PointerIsValid(DatumGetPointer(datum)))
 			repl_val[Anum_pg_user_mapping_umoptions - 1] = datum;

src/backend/commands/sequence.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.158 2009/02/02 19:31:38 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.159 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -198,7 +198,7 @@ DefineSequence(CreateSeqStmt *seq)
 	stmt->relation = seq->sequence;
 	stmt->inhRelations = NIL;
 	stmt->constraints = NIL;
-	stmt->options = list_make1(reloptWithOids(false));
+	stmt->options = list_make1(defWithOids(false));
 	stmt->oncommit = ONCOMMIT_NOOP;
 	stmt->tablespacename = NULL;
 

src/backend/commands/typecmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.132 2009/02/24 01:38:09 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.133 2009/04/04 21:12:31 tgl Exp $
  *
  * DESCRIPTION
  *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -1496,7 +1496,7 @@ DefineCompositeType(const RangeVar *typevar, List *coldeflist)
 	createStmt->tableElts = coldeflist;
 	createStmt->inhRelations = NIL;
 	createStmt->constraints = NIL;
-	createStmt->options = list_make1(reloptWithOids(false));
+	createStmt->options = list_make1(defWithOids(false));
 	createStmt->oncommit = ONCOMMIT_NOOP;
 	createStmt->tablespacename = NULL;
 

src/backend/commands/view.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.114 2009/02/02 19:31:39 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.115 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -229,7 +229,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
 		createStmt->tableElts = attrList;
 		createStmt->inhRelations = NIL;
 		createStmt->constraints = NIL;
-		createStmt->options = list_make1(reloptWithOids(false));
+		createStmt->options = list_make1(defWithOids(false));
 		createStmt->oncommit = ONCOMMIT_NOOP;
 		createStmt->tablespacename = NULL;
 

src/backend/nodes/copyfuncs.c

@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.427 2009/03/21 00:04:39 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.428 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2098,31 +2098,10 @@ _copyDefElem(DefElem *from)
 {
 	DefElem    *newnode = makeNode(DefElem);
 
+	COPY_STRING_FIELD(defnamespace);
 	COPY_STRING_FIELD(defname);
 	COPY_NODE_FIELD(arg);
-
-	return newnode;
-}
-
-static OptionDefElem *
-_copyOptionDefElem(OptionDefElem *from)
-{
-	OptionDefElem    *newnode = makeNode(OptionDefElem);
-
-	COPY_SCALAR_FIELD(alter_op);
-	COPY_NODE_FIELD(def);
-
-	return newnode;
-}
-
-static ReloptElem *
-_copyReloptElem(ReloptElem *from)
-{
-	ReloptElem	   *newnode = makeNode(ReloptElem);
-
-	COPY_STRING_FIELD(optname);
-	COPY_STRING_FIELD(nmspc);
-	COPY_NODE_FIELD(arg);
+	COPY_SCALAR_FIELD(defaction);
 
 	return newnode;
 }
@@ -4076,12 +4055,6 @@ copyObject(void *from)
 		case T_DefElem:
 			retval = _copyDefElem(from);
 			break;
-		case T_OptionDefElem:
-			retval = _copyOptionDefElem(from);
-			break;
-		case T_ReloptElem:
-			retval = _copyReloptElem(from);
-			break;
 		case T_LockingClause:
 			retval = _copyLockingClause(from);
 			break;

src/backend/nodes/equalfuncs.c

@@ -22,7 +22,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.350 2009/03/10 22:09:25 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.351 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2074,27 +2074,10 @@ _equalConstraint(Constraint *a, Constraint *b)
 static bool
 _equalDefElem(DefElem *a, DefElem *b)
 {
+	COMPARE_STRING_FIELD(defnamespace);
 	COMPARE_STRING_FIELD(defname);
 	COMPARE_NODE_FIELD(arg);
-
-	return true;
-}
-
-static bool
-_equalOptionDefElem(OptionDefElem *a, OptionDefElem *b)
-{
-	COMPARE_SCALAR_FIELD(alter_op);
-	COMPARE_NODE_FIELD(def);
-
-	return true;
-}
-
-static bool
-_equalReloptElem(ReloptElem *a, ReloptElem *b)
-{
-	COMPARE_STRING_FIELD(nmspc);
-	COMPARE_STRING_FIELD(optname);
-	COMPARE_NODE_FIELD(arg);
+	COMPARE_SCALAR_FIELD(defaction);
 
 	return true;
 }
@@ -2850,12 +2833,6 @@ equal(void *a, void *b)
 		case T_DefElem:
 			retval = _equalDefElem(a, b);
 			break;
-		case T_OptionDefElem:
-			retval = _equalOptionDefElem(a, b);
-			break;
-		case T_ReloptElem:
-			retval = _equalReloptElem(a, b);
-			break;
 		case T_LockingClause:
 			retval = _equalLockingClause(a, b);
 			break;

src/backend/nodes/makefuncs.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/makefuncs.c,v 1.63 2009/02/02 19:31:39 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/makefuncs.c,v 1.64 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -351,37 +351,37 @@ makeFuncExpr(Oid funcid, Oid rettype, List *args, CoercionForm fformat)
 /*
  * makeDefElem -
  *	build a DefElem node
+ *
+ * This is sufficient for the "typical" case with an unqualified option name
+ * and no special action.
  */
 DefElem *
 makeDefElem(char *name, Node *arg)
 {
 	DefElem    *res = makeNode(DefElem);
 
+	res->defnamespace = NULL;
 	res->defname = name;
 	res->arg = arg;
+	res->defaction = DEFELEM_UNSPEC;
+
 	return res;
 }
 
 /*
- * makeOptionDefElem -
- *	build an OptionDefElem node
+ * makeDefElemExtended -
+ *	build a DefElem node with all fields available to be specified
  */
-OptionDefElem *
-makeOptionDefElem(int op, DefElem *def)
-{
-	OptionDefElem *res = makeNode(OptionDefElem);
-	res->alter_op = op;
-	res->def = def;
-	return res;
-}
-
-ReloptElem *
-makeReloptElem(char *name, char *nmspc, Node *arg)
+DefElem *
+makeDefElemExtended(char *namespace, char *name, Node *arg,
+					DefElemAction defaction)
 {
-	ReloptElem *res = makeNode(ReloptElem);
+	DefElem    *res = makeNode(DefElem);
 
-	res->optname = name;
-	res->nmspc = nmspc;
+	res->defnamespace = namespace;
+	res->defname = name;
 	res->arg = arg;
+	res->defaction = defaction;
+
 	return res;
 }

src/backend/nodes/outfuncs.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.356 2009/03/26 17:15:34 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.357 2009/04/04 21:12:31 tgl Exp $
  *
  * NOTES
  *	  Every node type that can appear in stored rules' parsetrees *must*
@@ -1797,18 +1797,10 @@ _outDefElem(StringInfo str, DefElem *node)
 {
 	WRITE_NODE_TYPE("DEFELEM");
 
+	WRITE_STRING_FIELD(defnamespace);
 	WRITE_STRING_FIELD(defname);
 	WRITE_NODE_FIELD(arg);
-}
-
-static void
-_outReloptElem(StringInfo str, ReloptElem *node)
-{
-	WRITE_NODE_TYPE("RELOPTELEM");
-
-	WRITE_STRING_FIELD(nmspc);
-	WRITE_STRING_FIELD(optname);
-	WRITE_NODE_FIELD(arg);
+	WRITE_ENUM_FIELD(defaction, DefElemAction);
 }
 
 static void
@@ -2774,9 +2766,6 @@ _outNode(StringInfo str, void *obj)
 			case T_DefElem:
 				_outDefElem(str, obj);
 				break;
-			case T_ReloptElem:
-				_outReloptElem(str, obj);
-				break;
 			case T_LockingClause:
 				_outLockingClause(str, obj);
 				break;

src/backend/parser/gram.y

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.660 2009/03/07 00:13:57 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.661 2009/04/04 21:12:31 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -163,8 +163,6 @@ static TypeName *TableFuncTypeName(List *columns);
 	FunctionParameterMode fun_param_mode;
 	FuncWithArgs		*funwithargs;
 	DefElem				*defelt;
-	OptionDefElem		*optdef;
-	ReloptElem			*reloptel;
 	SortBy				*sortby;
 	WindowDef			*windef;
 	JoinExpr			*jexpr;
@@ -343,8 +341,7 @@ static TypeName *TableFuncTypeName(List *columns);
 
 %type <node>	TableElement ConstraintElem TableFuncElement
 %type <node>	columnDef
-%type <defelt>	def_elem old_aggr_elem
-%type <reloptel> reloption_elem
+%type <defelt>	def_elem reloption_elem old_aggr_elem
 %type <node>	def_arg columnElem where_clause where_or_current_clause
 				a_expr b_expr c_expr func_expr AexprConst indirection_el
 				columnref in_expr having_clause func_table array_expr
@@ -366,8 +363,7 @@ static TypeName *TableFuncTypeName(List *columns);
 
 %type <str>		generic_option_name
 %type <node>	generic_option_arg
-%type <defelt>	generic_option_elem
-%type <optdef>	alter_generic_option_elem
+%type <defelt>	generic_option_elem alter_generic_option_elem
 %type <list>	generic_option_list alter_generic_option_list
 
 %type <typnam>	Typename SimpleTypename ConstTypename
@@ -1837,22 +1833,24 @@ reloption_list:
 			| reloption_list ',' reloption_elem		{ $$ = lappend($1, $3); }
 		;
 
+/* This should match def_elem and also allow qualified names */
 reloption_elem:	
 			ColLabel '=' def_arg
 				{
-					$$ = makeReloptElem($1, NULL, (Node *) $3);
+					$$ = makeDefElem($1, (Node *) $3);
 				}
 			| ColLabel
 				{
-					$$ = makeReloptElem($1, NULL, NULL);
+					$$ = makeDefElem($1, NULL);
 				}
 			| ColLabel '.' ColLabel '=' def_arg
 				{
-					$$ = makeReloptElem($3, $1, (Node *) $5);
+					$$ = makeDefElemExtended($1, $3, (Node *) $5,
+											 DEFELEM_UNSPEC);
 				}
 			| ColLabel '.' ColLabel
 				{
-					$$ = makeReloptElem($3, $1, NULL);
+					$$ = makeDefElemExtended($1, $3, NULL, DEFELEM_UNSPEC);
 				}
 		;
 
@@ -2482,8 +2480,8 @@ OptInherit: INHERITS '(' qualified_name_list ')'	{ $$ = $3; }
 /* WITH (options) is preferred, WITH OIDS and WITHOUT OIDS are legacy forms */
 OptWith:
 			WITH reloptions				{ $$ = $2; }
-			| WITH OIDS					{ $$ = list_make1(reloptWithOids(true)); }
-			| WITHOUT OIDS				{ $$ = list_make1(reloptWithOids(false)); }
+			| WITH OIDS					{ $$ = list_make1(defWithOids(true)); }
+			| WITHOUT OIDS				{ $$ = list_make1(defWithOids(false)); }
 			| /*EMPTY*/					{ $$ = NIL; }
 		;
 
@@ -2887,70 +2885,72 @@ AlterFdwStmt: ALTER FOREIGN DATA_P WRAPPER name validator_clause alter_generic_o
 
 /* Options definition for CREATE FDW, SERVER and USER MAPPING */
 create_generic_options:
-				OPTIONS '(' generic_option_list ')'			{ $$ = $3; }
-				| /*EMPTY*/									{ $$ = NIL; }
+			OPTIONS '(' generic_option_list ')'			{ $$ = $3; }
+			| /*EMPTY*/									{ $$ = NIL; }
 		;
 
-generic_option_list:		generic_option_elem
-					{
-						$$ = list_make1(makeOptionDefElem(ALTER_OPT_ADD, $1));
-					}
-				| generic_option_list ',' generic_option_elem
-					{
-						$$ = lappend($1, makeOptionDefElem(ALTER_OPT_ADD, $3));
-					}
+generic_option_list:
+			generic_option_elem
+				{
+					$$ = list_make1($1);
+				}
+			| generic_option_list ',' generic_option_elem
+				{
+					$$ = lappend($1, $3);
+				}
 		;
 
 /* Options definition for ALTER FDW, SERVER and USER MAPPING */
 alter_generic_options:
-				OPTIONS	'(' alter_generic_option_list ')'	{ $$ = $3; }
+			OPTIONS	'(' alter_generic_option_list ')'		{ $$ = $3; }
 		;
 
 alter_generic_option_list:
-				alter_generic_option_elem
-					{
-						$$ = list_make1($1);
-					}
-				| generic_option_elem
-					{
-						$$ = list_make1(makeOptionDefElem(ALTER_OPT_ADD, $1));
-					}
-				| alter_generic_option_list ',' alter_generic_option_elem
-					{
-						$$ = lappend($1, $3);
-					}
-				| alter_generic_option_list ',' generic_option_elem
-					{
-						$$ = lappend($1, makeOptionDefElem(ALTER_OPT_ADD, $3));
-					}
+			alter_generic_option_elem
+				{
+					$$ = list_make1($1);
+				}
+			| alter_generic_option_list ',' alter_generic_option_elem
+				{
+					$$ = lappend($1, $3);
+				}
 		;
 
 alter_generic_option_elem:
-				ADD_P generic_option_elem
-					{
-						$$ = makeOptionDefElem(ALTER_OPT_ADD, $2);
-					}
-				| SET generic_option_elem
-					{
-						$$ = makeOptionDefElem(ALTER_OPT_SET, $2);
-					}
-				| DROP generic_option_name
-					{
-						$$ = makeOptionDefElem(ALTER_OPT_DROP,
-											   makeDefElem($2, NULL));
-					}
+			generic_option_elem
+				{
+					$$ = $1;
+				}
+			| SET generic_option_elem
+				{
+					$$ = $2;
+					$$->defaction = DEFELEM_SET;
+				}
+			| ADD_P generic_option_elem
+				{
+					$$ = $2;
+					$$->defaction = DEFELEM_ADD;
+				}
+			| DROP generic_option_name
+				{
+					$$ = makeDefElemExtended(NULL, $2, NULL, DEFELEM_DROP);
+				}
 		;
 
 generic_option_elem:
-				generic_option_name generic_option_arg			{ $$ = makeDefElem($1, $2); }
+			generic_option_name generic_option_arg
+				{
+					$$ = makeDefElem($1, $2);
+				}
 		;
 
 generic_option_name:
-				attr_name			{ $$ = $1; }
+				ColLabel			{ $$ = $1; }
 		;
 
+/* We could use def_arg here, but the spec only requires string literals */
 generic_option_arg:
-				Sconst				{ $$ = (Node *)makeString($1); }
+				Sconst				{ $$ = (Node *) makeString($1); }
 		;
 
 /*****************************************************************************
@@ -3504,9 +3504,9 @@ def_list:  	def_elem								{ $$ = list_make1($1); }
 			| def_list ',' def_elem					{ $$ = lappend($1, $3); }
 		;
 
-def_elem:  ColLabel '=' def_arg
+def_elem:	ColLabel '=' def_arg
 				{
-					$$ = makeDefElem($1, (Node *)$3);
+					$$ = makeDefElem($1, (Node *) $3);
 				}
 			| ColLabel
 				{

src/backend/parser/parse_clause.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.187 2009/02/02 19:31:39 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.188 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -233,7 +233,7 @@ interpretInhOption(InhOption inhOpt)
 }
 
 /*
- * Given a relation-options list (of ReloptElems), return true iff the specified
+ * Given a relation-options list (of DefElems), return true iff the specified
  * table/result set should be created with OIDs. This needs to be done after
  * parsing the query string because the return value can depend upon the
  * default_with_oids GUC var.
@@ -246,10 +246,11 @@ interpretOidsOption(List *defList)
 	/* Scan list to see if OIDS was included */
 	foreach(cell, defList)
 	{
-		ReloptElem    *def = (ReloptElem *) lfirst(cell);
+		DefElem    *def = (DefElem *) lfirst(cell);
 
-		if (pg_strcasecmp(def->optname, "oids") == 0)
-			return reloptGetBoolean(def);
+		if (def->defnamespace == NULL &&
+			pg_strcasecmp(def->defname, "oids") == 0)
+			return defGetBoolean(def);
 	}
 
 	/* OIDS option was not specified, so use default. */

src/include/commands/defrem.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/defrem.h,v 1.93 2009/02/02 19:31:40 alvherre Exp $
+ * $PostgreSQL: pgsql/src/include/commands/defrem.h,v 1.94 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -145,8 +145,6 @@ extern int64 defGetInt64(DefElem *def);
 extern List *defGetQualifiedName(DefElem *def);
 extern TypeName *defGetTypeName(DefElem *def);
 extern int	defGetTypeLength(DefElem *def);
-extern char *reloptGetString(ReloptElem *relopt);
-extern bool reloptGetBoolean(ReloptElem *relopt);
-extern ReloptElem *reloptWithOids(bool value);
+extern DefElem *defWithOids(bool value);
 
 #endif   /* DEFREM_H */

src/include/foreign/foreign.h

@@ -6,7 +6,7 @@
  *
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/include/foreign/foreign.h,v 1.3 2009/02/24 10:06:35 petere Exp $
+ * $PostgreSQL: pgsql/src/include/foreign/foreign.h,v 1.4 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -14,7 +14,6 @@
 #define FOREIGN_H
 
 #include "nodes/parsenodes.h"
-#include "nodes/pg_list.h"
 
 
 /* Helper for obtaining username for user mapping */
@@ -27,10 +26,9 @@
  * NB! Thes are treated as flags, so use only powers of two here.
  */
 typedef enum {
-	InvalidOpt = 0,
 	ServerOpt = 1,				/* options applicable to SERVER */
 	UserMappingOpt = 2,			/* options for USER MAPPING */
-	FdwOpt = 4,					/* options for FOREIGN DATA WRAPPER */
+	FdwOpt = 4					/* options for FOREIGN DATA WRAPPER */
 } GenericOptionFlags;
 
 typedef struct ForeignDataWrapper
@@ -70,8 +68,4 @@ extern ForeignDataWrapper *GetForeignDataWrapperByName(const char *name,
 													   bool missing_ok);
 extern Oid GetForeignDataWrapperOidByName(const char *name, bool missing_ok);
 
-/* Foreign data wrapper interface functions */
-extern void _pg_validateOptionList(ForeignDataWrapper *fdw,
-								   GenericOptionFlags flags, List *options);
-
 #endif /* FOREIGN_H */

src/include/nodes/makefuncs.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/makefuncs.h,v 1.66 2009/02/02 19:31:40 alvherre Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/makefuncs.h,v 1.67 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -66,9 +66,7 @@ extern FuncExpr *makeFuncExpr(Oid funcid, Oid rettype,
 			 List *args, CoercionForm fformat);
 
 extern DefElem *makeDefElem(char *name, Node *arg);
-
-extern OptionDefElem *makeOptionDefElem(int op, DefElem *def);
-
-extern ReloptElem *makeReloptElem(char *name, char *namspc, Node *arg);
+extern DefElem *makeDefElemExtended(char *namespace, char *name, Node *arg,
+									DefElemAction defaction);
 
 #endif   /* MAKEFUNC_H */

src/include/nodes/nodes.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/nodes.h,v 1.221 2009/02/25 03:30:37 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/nodes.h,v 1.222 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -361,8 +361,6 @@ typedef enum NodeTag
 	T_IndexElem,
 	T_Constraint,
 	T_DefElem,
-	T_OptionDefElem,
-	T_ReloptElem,
 	T_RangeTblEntry,
 	T_SortGroupClause,
 	T_WindowClause,

src/include/nodes/parsenodes.h

@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.392 2009/02/24 10:06:35 petere Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.393 2009/04/04 21:12:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -50,14 +50,6 @@ typedef enum SortByNulls
 	SORTBY_NULLS_LAST
 } SortByNulls;
 
-/* Alter operations for generic options */
-typedef enum AlterOptionOp
-{
-	ALTER_OPT_DROP = -1,
-	ALTER_OPT_SET,
-	ALTER_OPT_ADD
-} AlterOptionOp;
-
 /*
  * Grantable rights are encoded so that we can OR them together in a bitmask.
  * The present representation of AclItem limits us to 16 distinct rights,
@@ -511,38 +503,32 @@ typedef struct IndexElem
 } IndexElem;
 
 /*
- * DefElem -
- *	  a definition (used in definition lists in the form of defname = arg)
+ * DefElem - a generic "name = value" option definition
+ *
+ * In some contexts the name can be qualified.  Also, certain SQL commands
+ * allow a SET/ADD/DROP action to be attached to option settings, so it's
+ * convenient to carry a field for that too.  (Note: currently, it is our
+ * practice that the grammar allows namespace and action only in statements
+ * where they are relevant; C code can just ignore those fields in other
+ * statements.)
  */
+typedef enum DefElemAction
+{
+	DEFELEM_UNSPEC,				/* no action given */
+	DEFELEM_SET,
+	DEFELEM_ADD,
+	DEFELEM_DROP
+} DefElemAction;
+
 typedef struct DefElem
 {
 	NodeTag		type;
+	char	   *defnamespace;	/* NULL if unqualified name */
 	char	   *defname;
 	Node	   *arg;			/* a (Value *) or a (TypeName *) */
+	DefElemAction defaction;	/* unspecified action, or SET/ADD/DROP */
 } DefElem;
 
-/*
- * Option definition. Used in options definition lists, with optional alter
- * operation.
- */
-typedef struct OptionDefElem
-{
-	NodeTag			type;
-	AlterOptionOp	alter_op;		/* Alter operation: ADD/SET/DROP */
-	DefElem		   *def;			/* The actual definition */
-} OptionDefElem;
-
-/*
- * Reloption definition.  As DefElem, with optional option namespace.
- */
-typedef struct ReloptElem
-{
-	NodeTag		type;
-	char	   *nmspc;
-	char	   *optname;
-	Node	   *arg;
-} ReloptElem;
-
 /*
  * LockingClause - raw representation of FOR UPDATE/SHARE options
  *