Merge branch 'REL9_6_STABLE' into PGPRO9_6

Author: vbwagner

config/perl.m4

@@ -49,6 +49,31 @@ AC_DEFUN([PGAC_CHECK_PERL_CONFIGS],
 [m4_foreach([pgac_item], [$1], [PGAC_CHECK_PERL_CONFIG(pgac_item)])])
 
 
+# PGAC_CHECK_PERL_EMBED_CCFLAGS
+# -----------------------------
+# We selectively extract stuff from $Config{ccflags}.  We don't really need
+# anything except -D switches, and other sorts of compiler switches can
+# actively break things if Perl was compiled with a different compiler.
+# Moreover, although Perl likes to put stuff like -D_LARGEFILE_SOURCE and
+# -D_FILE_OFFSET_BITS=64 here, it would be fatal to try to compile PL/Perl
+# to a different libc ABI than core Postgres uses.  The available information
+# says that all the symbols that affect Perl's own ABI begin with letters,
+# so it should be sufficient to adopt -D switches for symbols not beginning
+# with underscore.
+# For debugging purposes, let's have the configure output report the raw
+# ccflags value as well as the set of flags we chose to adopt.
+AC_DEFUN([PGAC_CHECK_PERL_EMBED_CCFLAGS],
+[AC_REQUIRE([PGAC_PATH_PERL])
+AC_MSG_CHECKING([for CFLAGS recommended by Perl])
+perl_ccflags=`$PERL -MConfig -e ['print $Config{ccflags}']`
+AC_MSG_RESULT([$perl_ccflags])
+AC_MSG_CHECKING([for CFLAGS to compile embedded Perl])
+perl_embed_ccflags=`$PERL -MConfig -e ['foreach $f (split(" ",$Config{ccflags})) {print $f, " " if ($f =~ /^-D[^_]/)}']`
+AC_SUBST(perl_embed_ccflags)dnl
+AC_MSG_RESULT([$perl_embed_ccflags])
+])# PGAC_CHECK_PERL_EMBED_CCFLAGS
+
+
 # PGAC_CHECK_PERL_EMBED_LDFLAGS
 # -----------------------------
 # We are after Embed's ldopts, but without the subset mentioned in

configure

@@ -672,6 +672,7 @@ python_version
 python_majorversion
 PYTHON
 perl_embed_ldflags
+perl_embed_ccflags
 perl_useshrplib
 perl_privlibexp
 perl_archlibexp
@@ -7576,6 +7577,18 @@ documentation for details.  Use --without-perl to disable building
 PL/Perl." "$LINENO" 5
   fi
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for CFLAGS recommended by Perl" >&5
+$as_echo_n "checking for CFLAGS recommended by Perl... " >&6; }
+perl_ccflags=`$PERL -MConfig -e 'print $Config{ccflags}'`
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $perl_ccflags" >&5
+$as_echo "$perl_ccflags" >&6; }
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for CFLAGS to compile embedded Perl" >&5
+$as_echo_n "checking for CFLAGS to compile embedded Perl... " >&6; }
+perl_embed_ccflags=`$PERL -MConfig -e 'foreach $f (split(" ",$Config{ccflags})) {print $f, " " if ($f =~ /^-D[^_]/)}'`
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $perl_embed_ccflags" >&5
+$as_echo "$perl_embed_ccflags" >&6; }
+
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for flags to link embedded Perl" >&5
 $as_echo_n "checking for flags to link embedded Perl... " >&6; }
 if test "$PORTNAME" = "win32" ; then

configure.in

@@ -948,6 +948,7 @@ You might have to rebuild your Perl installation.  Refer to the
 documentation for details.  Use --without-perl to disable building
 PL/Perl.])
   fi
+  PGAC_CHECK_PERL_EMBED_CCFLAGS
   PGAC_CHECK_PERL_EMBED_LDFLAGS
 fi
 

contrib/hstore_plperl/Makefile

@@ -43,4 +43,4 @@ endif
 # last, probably because it sometimes contains some header files with names
 # that clash with some of ours, or with some that we include, notably on
 # Windows.
-override CPPFLAGS := $(CPPFLAGS) -I$(perl_archlibexp)/CORE
+override CPPFLAGS := $(CPPFLAGS) $(perl_embed_ccflags) -I$(perl_archlibexp)/CORE

contrib/hstore_plperl/hstore_plperl.c

@@ -13,6 +13,7 @@ PG_FUNCTION_INFO_V1(hstore_to_plperl);
 Datum
 hstore_to_plperl(PG_FUNCTION_ARGS)
 {
+	dTHX;
 	HStore	   *in = PG_GETARG_HS(0);
 	int			i;
 	int			count = HS_COUNT(in);
@@ -45,16 +46,15 @@ PG_FUNCTION_INFO_V1(plperl_to_hstore);
 Datum
 plperl_to_hstore(PG_FUNCTION_ARGS)
 {
-	HV		   *hv;
+	dTHX;
+	HV		   *hv = (HV *) SvRV((SV *) PG_GETARG_POINTER(0));
 	HE		   *he;
 	int32		buflen;
 	int32		i;
 	int32		pcount;
 	HStore	   *out;
 	Pairs	   *pairs;
 
-	hv = (HV *) SvRV((SV *) PG_GETARG_POINTER(0));
-
 	pcount = hv_iterinit(hv);
 
 	pairs = palloc(pcount * sizeof(Pairs));

contrib/postgres_fdw/connection.c

@@ -21,6 +21,7 @@
 #include "miscadmin.h"
 #include "storage/latch.h"
 #include "utils/hsearch.h"
+#include "utils/inval.h"
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 
@@ -47,11 +48,15 @@ typedef struct ConnCacheEntry
 {
 	ConnCacheKey key;			/* hash key (must be first) */
 	PGconn	   *conn;			/* connection to foreign server, or NULL */
+	/* Remaining fields are invalid when conn is NULL: */
 	int			xact_depth;		/* 0 = no xact open, 1 = main xact open, 2 =
 								 * one level of subxact open, etc */
 	bool		have_prep_stmt; /* have we prepared any stmts in this xact? */
 	bool		have_error;		/* have any subxacts aborted in this xact? */
 	bool		changing_xact_state;	/* xact state change in process */
+	bool		invalidated;	/* true if reconnect is pending */
+	uint32		server_hashvalue;	/* hash value of foreign server OID */
+	uint32		mapping_hashvalue;	/* hash value of user mapping OID */
 } ConnCacheEntry;
 
 /*
@@ -68,6 +73,7 @@ static bool xact_got_connection = false;
 
 /* prototypes of private functions */
 static PGconn *connect_pg_server(ForeignServer *server, UserMapping *user);
+static void disconnect_pg_server(ConnCacheEntry *entry);
 static void check_conn_params(const char **keywords, const char **values);
 static void configure_remote_session(PGconn *conn);
 static void do_sql_command(PGconn *conn, const char *sql);
@@ -77,6 +83,7 @@ static void pgfdw_subxact_callback(SubXactEvent event,
 					   SubTransactionId mySubid,
 					   SubTransactionId parentSubid,
 					   void *arg);
+static void pgfdw_inval_callback(Datum arg, int cacheid, uint32 hashvalue);
 static void pgfdw_reject_incomplete_xact_state_change(ConnCacheEntry *entry);
 static bool pgfdw_cancel_query(PGconn *conn);
 static bool pgfdw_exec_cleanup_query(PGconn *conn, const char *query,
@@ -94,13 +101,6 @@ static bool pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime,
  * will_prep_stmt must be true if caller intends to create any prepared
  * statements.  Since those don't go away automatically at transaction end
  * (not even on error), we need this flag to cue manual cleanup.
- *
- * XXX Note that caching connections theoretically requires a mechanism to
- * detect change of FDW objects to invalidate already established connections.
- * We could manage that by watching for invalidation events on the relevant
- * syscaches.  For the moment, though, it's not clear that this would really
- * be useful and not mere pedantry.  We could not flush any active connections
- * mid-transaction anyway.
  */
 PGconn *
 GetConnection(UserMapping *user, bool will_prep_stmt)
@@ -129,6 +129,10 @@ GetConnection(UserMapping *user, bool will_prep_stmt)
 		 */
 		RegisterXactCallback(pgfdw_xact_callback, NULL);
 		RegisterSubXactCallback(pgfdw_subxact_callback, NULL);
+		CacheRegisterSyscacheCallback(FOREIGNSERVEROID,
+									  pgfdw_inval_callback, (Datum) 0);
+		CacheRegisterSyscacheCallback(USERMAPPINGOID,
+									  pgfdw_inval_callback, (Datum) 0);
 	}
 
 	/* Set flag that we did GetConnection during the current transaction */
@@ -143,17 +147,27 @@ GetConnection(UserMapping *user, bool will_prep_stmt)
 	entry = hash_search(ConnectionHash, &key, HASH_ENTER, &found);
 	if (!found)
 	{
-		/* initialize new hashtable entry (key is already filled in) */
+		/*
+		 * We need only clear "conn" here; remaining fields will be filled
+		 * later when "conn" is set.
+		 */
 		entry->conn = NULL;
-		entry->xact_depth = 0;
-		entry->have_prep_stmt = false;
-		entry->have_error = false;
-		entry->changing_xact_state = false;
 	}
 
 	/* Reject further use of connections which failed abort cleanup. */
 	pgfdw_reject_incomplete_xact_state_change(entry);
 
+	/*
+	 * If the connection needs to be remade due to invalidation, disconnect as
+	 * soon as we're out of all transactions.
+	 */
+	if (entry->conn != NULL && entry->invalidated && entry->xact_depth == 0)
+	{
+		elog(DEBUG3, "closing connection %p for option changes to take effect",
+			 entry->conn);
+		disconnect_pg_server(entry);
+	}
+
 	/*
 	 * We don't check the health of cached connection here, because it would
 	 * require some overhead.  Broken connection will be detected when the
@@ -163,15 +177,26 @@ GetConnection(UserMapping *user, bool will_prep_stmt)
 	/*
 	 * If cache entry doesn't have a connection, we have to establish a new
 	 * connection.  (If connect_pg_server throws an error, the cache entry
-	 * will be left in a valid empty state.)
+	 * will remain in a valid empty state, ie conn == NULL.)
 	 */
 	if (entry->conn == NULL)
 	{
 		ForeignServer *server = GetForeignServer(user->serverid);
 
-		entry->xact_depth = 0;	/* just to be sure */
+		/* Reset all transient state fields, to be sure all are clean */
+		entry->xact_depth = 0;
 		entry->have_prep_stmt = false;
 		entry->have_error = false;
+		entry->changing_xact_state = false;
+		entry->invalidated = false;
+		entry->server_hashvalue =
+			GetSysCacheHashValue1(FOREIGNSERVEROID,
+								  ObjectIdGetDatum(server->serverid));
+		entry->mapping_hashvalue =
+			GetSysCacheHashValue1(USERMAPPINGOID,
+								  ObjectIdGetDatum(user->umid));
+
+		/* Now try to make the connection */
 		entry->conn = connect_pg_server(server, user);
 
 		elog(DEBUG3, "new postgres_fdw connection %p for server \"%s\" (user mapping oid %u, userid %u)",
@@ -285,6 +310,19 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
 	return conn;
 }
 
+/*
+ * Disconnect any open connection for a connection cache entry.
+ */
+static void
+disconnect_pg_server(ConnCacheEntry *entry)
+{
+	if (entry->conn != NULL)
+	{
+		PQfinish(entry->conn);
+		entry->conn = NULL;
+	}
+}
+
 /*
  * For non-superusers, insist that the connstr specify a password.  This
  * prevents a password from being picked up from .pgpass, a service file,
@@ -786,9 +824,7 @@ pgfdw_xact_callback(XactEvent event, void *arg)
 			entry->changing_xact_state)
 		{
 			elog(DEBUG3, "discarding connection %p", entry->conn);
-			PQfinish(entry->conn);
-			entry->conn = NULL;
-			entry->changing_xact_state = false;
+			disconnect_pg_server(entry);
 		}
 	}
 
@@ -905,6 +941,47 @@ pgfdw_subxact_callback(SubXactEvent event, SubTransactionId mySubid,
 	}
 }
 
+/*
+ * Connection invalidation callback function
+ *
+ * After a change to a pg_foreign_server or pg_user_mapping catalog entry,
+ * mark connections depending on that entry as needing to be remade.
+ * We can't immediately destroy them, since they might be in the midst of
+ * a transaction, but we'll remake them at the next opportunity.
+ *
+ * Although most cache invalidation callbacks blow away all the related stuff
+ * regardless of the given hashvalue, connections are expensive enough that
+ * it's worth trying to avoid that.
+ *
+ * NB: We could avoid unnecessary disconnection more strictly by examining
+ * individual option values, but it seems too much effort for the gain.
+ */
+static void
+pgfdw_inval_callback(Datum arg, int cacheid, uint32 hashvalue)
+{
+	HASH_SEQ_STATUS scan;
+	ConnCacheEntry *entry;
+
+	Assert(cacheid == FOREIGNSERVEROID || cacheid == USERMAPPINGOID);
+
+	/* ConnectionHash must exist already, if we're registered */
+	hash_seq_init(&scan, ConnectionHash);
+	while ((entry = (ConnCacheEntry *) hash_seq_search(&scan)))
+	{
+		/* Ignore invalid entries */
+		if (entry->conn == NULL)
+			continue;
+
+		/* hashvalue == 0 means a cache reset, must clear all state */
+		if (hashvalue == 0 ||
+			(cacheid == FOREIGNSERVEROID &&
+			 entry->server_hashvalue == hashvalue) ||
+			(cacheid == USERMAPPINGOID &&
+			 entry->mapping_hashvalue == hashvalue))
+			entry->invalidated = true;
+	}
+}
+
 /*
  * Raise an error if the given connection cache entry is marked as being
  * in the middle of an xact state change.  This should be called at which no
@@ -922,9 +999,14 @@ pgfdw_reject_incomplete_xact_state_change(ConnCacheEntry *entry)
 	Form_pg_user_mapping umform;
 	ForeignServer *server;
 
-	if (!entry->changing_xact_state)
+	/* nothing to do for inactive entries and entries of sane state */
+	if (entry->conn == NULL || !entry->changing_xact_state)
 		return;
 
+	/* make sure this entry is inactive */
+	disconnect_pg_server(entry);
+
+	/* find server name to be shown in the message below */
 	tup = SearchSysCache1(USERMAPPINGOID,
 						  ObjectIdGetDatum(entry->key));
 	if (!HeapTupleIsValid(tup))