1 files changed, 174 insertions, 35 deletions

diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index a2b5358e16f..acd40c1f4e9 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -165,7 +165,7 @@ transformGenericOptions(Oid catalogId,
 
 	result = optionListToArray(resultOptions);
 
-	if (fdwvalidator)
+	if (OidIsValid(fdwvalidator) && DatumGetPointer(result) != NULL)
 		OidFunctionCall2(fdwvalidator, result, ObjectIdGetDatum(catalogId));
 
 	return result;
@@ -246,8 +246,9 @@ AlterForeignDataWrapperOwner(const char *name, Oid newOwnerId)
 								newOwnerId);
 	}
 
-	heap_close(rel, NoLock);
 	heap_freetuple(tup);
+
+	heap_close(rel, RowExclusiveLock);
 }
 
 
@@ -308,25 +309,98 @@ AlterForeignServerOwner(const char *name, Oid newOwnerId)
 								newOwnerId);
 	}
 
-	heap_close(rel, NoLock);
 	heap_freetuple(tup);
+
+	heap_close(rel, RowExclusiveLock);
 }
 
 
 /*
+ * Convert a handler function name passed from the parser to an Oid.
+ */
+static Oid
+lookup_fdw_handler_func(DefElem *handler)
+{
+	Oid			handlerOid;
+
+	if (handler == NULL || handler->arg == NULL)
+		return InvalidOid;
+
+	/* handlers have no arguments */
+	handlerOid = LookupFuncName((List *) handler->arg, 0, NULL, false);
+
+	/* check that handler has correct return type */
+	if (get_func_rettype(handlerOid) != FDW_HANDLEROID)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("function %s must return type \"fdw_handler\"",
+						NameListToString((List *) handler->arg))));
+
+	return handlerOid;
+}
+
+/*
  * Convert a validator function name passed from the parser to an Oid.
  */
 static Oid
-lookup_fdw_validator_func(List *validator)
+lookup_fdw_validator_func(DefElem *validator)
 {
 	Oid			funcargtypes[2];
 
+	if (validator == NULL || validator->arg == NULL)
+		return InvalidOid;
+
+	/* validators take text[], oid */
 	funcargtypes[0] = TEXTARRAYOID;
 	funcargtypes[1] = OIDOID;
-	return LookupFuncName(validator, 2, funcargtypes, false);
-	/* return value is ignored, so we don't check the type */
+
+	return LookupFuncName((List *) validator->arg, 2, funcargtypes, false);
+	/* validator's return value is ignored, so we don't check the type */
 }
 
+/*
+ * Process function options of CREATE/ALTER FDW
+ */
+static void
+parse_func_options(List *func_options,
+				   bool *handler_given, Oid *fdwhandler,
+				   bool *validator_given, Oid *fdwvalidator)
+{
+	ListCell   *cell;
+
+	*handler_given = false;
+	*validator_given = false;
+	/* return InvalidOid if not given */
+	*fdwhandler = InvalidOid;
+	*fdwvalidator = InvalidOid;
+
+	foreach(cell, func_options)
+	{
+		DefElem    *def = (DefElem *) lfirst(cell);
+
+		if (strcmp(def->defname, "handler") == 0)
+		{
+			if (*handler_given)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("conflicting or redundant options")));
+			*handler_given = true;
+			*fdwhandler = lookup_fdw_handler_func(def);
+		}
+		else if (strcmp(def->defname, "validator") == 0)
+		{
+			if (*validator_given)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("conflicting or redundant options")));
+			*validator_given = true;
+			*fdwvalidator = lookup_fdw_validator_func(def);
+		}
+		else
+			elog(ERROR, "option \"%s\" not recognized",
+				 def->defname);
+	}
+}
 
 /*
  * Create a foreign-data wrapper
@@ -339,6 +413,9 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
 	bool		nulls[Natts_pg_foreign_data_wrapper];
 	HeapTuple	tuple;
 	Oid			fdwId;
+	bool		handler_given;
+	bool		validator_given;
+	Oid			fdwhandler;
 	Oid			fdwvalidator;
 	Datum		fdwoptions;
 	Oid			ownerId;
@@ -377,12 +454,13 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
 		DirectFunctionCall1(namein, CStringGetDatum(stmt->fdwname));
 	values[Anum_pg_foreign_data_wrapper_fdwowner - 1] = ObjectIdGetDatum(ownerId);
 
-	if (stmt->validator)
-		fdwvalidator = lookup_fdw_validator_func(stmt->validator);
-	else
-		fdwvalidator = InvalidOid;
+	/* Lookup handler and validator functions, if given */
+	parse_func_options(stmt->func_options,
+					   &handler_given, &fdwhandler,
+					   &validator_given, &fdwvalidator);
 
-	values[Anum_pg_foreign_data_wrapper_fdwvalidator - 1] = fdwvalidator;
+	values[Anum_pg_foreign_data_wrapper_fdwhandler - 1] = ObjectIdGetDatum(fdwhandler);
+	values[Anum_pg_foreign_data_wrapper_fdwvalidator - 1] = ObjectIdGetDatum(fdwvalidator);
 
 	nulls[Anum_pg_foreign_data_wrapper_fdwacl - 1] = true;
 
@@ -408,7 +486,15 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
 	myself.objectId = fdwId;
 	myself.objectSubId = 0;
 
-	if (fdwvalidator)
+	if (OidIsValid(fdwhandler))
+	{
+		referenced.classId = ProcedureRelationId;
+		referenced.objectId = fdwhandler;
+		referenced.objectSubId = 0;
+		recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+	}
+
+	if (OidIsValid(fdwvalidator))
 	{
 		referenced.classId = ProcedureRelationId;
 		referenced.objectId = fdwvalidator;
@@ -425,7 +511,7 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
 	InvokeObjectAccessHook(OAT_POST_CREATE,
 						   ForeignDataWrapperRelationId, fdwId, 0);
 
-	heap_close(rel, NoLock);
+	heap_close(rel, RowExclusiveLock);
 }
 
 
@@ -437,12 +523,16 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
 {
 	Relation	rel;
 	HeapTuple	tp;
+	Form_pg_foreign_data_wrapper fdwForm;
 	Datum		repl_val[Natts_pg_foreign_data_wrapper];
 	bool		repl_null[Natts_pg_foreign_data_wrapper];
 	bool		repl_repl[Natts_pg_foreign_data_wrapper];
 	Oid			fdwId;
 	bool		isnull;
 	Datum		datum;
+	bool		handler_given;
+	bool		validator_given;
+	Oid			fdwhandler;
 	Oid			fdwvalidator;
 
 	/* Must be super user */
@@ -461,15 +551,32 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
 				(errcode(ERRCODE_UNDEFINED_OBJECT),
 		errmsg("foreign-data wrapper \"%s\" does not exist", stmt->fdwname)));
 
+	fdwForm = (Form_pg_foreign_data_wrapper) GETSTRUCT(tp);
 	fdwId = HeapTupleGetOid(tp);
 
 	memset(repl_val, 0, sizeof(repl_val));
 	memset(repl_null, false, sizeof(repl_null));
 	memset(repl_repl, false, sizeof(repl_repl));
 
-	if (stmt->change_validator)
+	parse_func_options(stmt->func_options,
+					   &handler_given, &fdwhandler,
+					   &validator_given, &fdwvalidator);
+
+	if (handler_given)
+	{
+		repl_val[Anum_pg_foreign_data_wrapper_fdwhandler - 1] = ObjectIdGetDatum(fdwhandler);
+		repl_repl[Anum_pg_foreign_data_wrapper_fdwhandler - 1] = true;
+
+		/*
+		 * It could be that the behavior of accessing foreign table changes
+		 * with the new handler.  Warn about this.
+		 */
+		ereport(WARNING,
+				(errmsg("changing the foreign-data wrapper handler can change behavior of existing foreign tables")));
+	}
+
+	if (validator_given)
 	{
-		fdwvalidator = stmt->validator ? lookup_fdw_validator_func(stmt->validator) : InvalidOid;
 		repl_val[Anum_pg_foreign_data_wrapper_fdwvalidator - 1] = ObjectIdGetDatum(fdwvalidator);
 		repl_repl[Anum_pg_foreign_data_wrapper_fdwvalidator - 1] = true;
 
@@ -477,26 +584,21 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
 		 * It could be that the options for the FDW, SERVER and USER MAPPING
 		 * are no longer valid with the new validator.	Warn about this.
 		 */
-		if (stmt->validator)
+		if (OidIsValid(fdwvalidator))
 			ereport(WARNING,
-			 (errmsg("changing the foreign-data wrapper validator can cause "
-					 "the options for dependent objects to become invalid")));
+					(errmsg("changing the foreign-data wrapper validator can cause "
+							"the options for dependent objects to become invalid")));
 	}
 	else
 	{
 		/*
 		 * Validator is not changed, but we need it for validating options.
 		 */
-		datum = SysCacheGetAttr(FOREIGNDATAWRAPPEROID,
-								tp,
-								Anum_pg_foreign_data_wrapper_fdwvalidator,
-								&isnull);
-		Assert(!isnull);
-		fdwvalidator = DatumGetObjectId(datum);
+		fdwvalidator = fdwForm->fdwvalidator;
 	}
 
 	/*
-	 * Options specified, validate and update.
+	 * If options specified, validate and update.
 	 */
 	if (stmt->options)
 	{
@@ -532,8 +634,46 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
 	simple_heap_update(rel, &tp->t_self, tp);
 	CatalogUpdateIndexes(rel, tp);
 
-	heap_close(rel, RowExclusiveLock);
 	heap_freetuple(tp);
+
+	/* Update function dependencies if we changed them */
+	if (handler_given || validator_given)
+	{
+		ObjectAddress myself;
+		ObjectAddress referenced;
+
+		/*
+		 * Flush all existing dependency records of this FDW on functions;
+		 * we assume there can be none other than the ones we are fixing.
+		 */
+		deleteDependencyRecordsForClass(ForeignDataWrapperRelationId,
+										fdwId,
+										ProcedureRelationId,
+										DEPENDENCY_NORMAL);
+
+		/* And build new ones. */
+		myself.classId = ForeignDataWrapperRelationId;
+		myself.objectId = fdwId;
+		myself.objectSubId = 0;
+
+		if (OidIsValid(fdwhandler))
+		{
+			referenced.classId = ProcedureRelationId;
+			referenced.objectId = fdwhandler;
+			referenced.objectSubId = 0;
+			recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+		}
+
+		if (OidIsValid(fdwvalidator))
+		{
+			referenced.classId = ProcedureRelationId;
+			referenced.objectId = fdwvalidator;
+			referenced.objectSubId = 0;
+			recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+		}
+	}
+
+	heap_close(rel, RowExclusiveLock);
 }
 
 
@@ -712,7 +852,7 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
 	/* Post creation hook for new foreign server */
 	InvokeObjectAccessHook(OAT_POST_CREATE, ForeignServerRelationId, srvId, 0);
 
-	heap_close(rel, NoLock);
+	heap_close(rel, RowExclusiveLock);
 }
 
 
@@ -804,8 +944,9 @@ AlterForeignServer(AlterForeignServerStmt *stmt)
 	simple_heap_update(rel, &tp->t_self, tp);
 	CatalogUpdateIndexes(rel, tp);
 
-	heap_close(rel, RowExclusiveLock);
 	heap_freetuple(tp);
+
+	heap_close(rel, RowExclusiveLock);
 }
 
 
@@ -991,7 +1132,7 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
 	/* Post creation hook for new user mapping */
 	InvokeObjectAccessHook(OAT_POST_CREATE, UserMappingRelationId, umId, 0);
 
-	heap_close(rel, NoLock);
+	heap_close(rel, RowExclusiveLock);
 }
 
 
@@ -1076,8 +1217,9 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
 	simple_heap_update(rel, &tp->t_self, tp);
 	CatalogUpdateIndexes(rel, tp);
 
-	heap_close(rel, RowExclusiveLock);
 	heap_freetuple(tp);
+
+	heap_close(rel, RowExclusiveLock);
 }
 
 
@@ -1184,7 +1326,6 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid)
 	Datum		values[Natts_pg_foreign_table];
 	bool		nulls[Natts_pg_foreign_table];
 	HeapTuple	tuple;
-	Oid			ftId;
 	AclResult	aclresult;
 	ObjectAddress myself;
 	ObjectAddress referenced;
@@ -1237,9 +1378,7 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid)
 
 	tuple = heap_form_tuple(ftrel->rd_att, values, nulls);
 
-	/* pg_foreign_table don't have OID */
-	ftId = simple_heap_insert(ftrel, tuple);
-
+	simple_heap_insert(ftrel, tuple);
 	CatalogUpdateIndexes(ftrel, tuple);
 
 	heap_freetuple(tuple);
@@ -1254,5 +1393,5 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid relid)
 	referenced.objectSubId = 0;
 	recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 
-	heap_close(ftrel, NoLock);
+	heap_close(ftrel, RowExclusiveLock);
 }