alloc non-global conn. params in provided context

Having DATA-segment string literals made blindly freeing the keywords/
values difficult, so I've switched to allocating all in the provided
context; because of this (and with the knowledge of the end point of
the global parameters), we can safely pfree non-global parameters when
we come across an invalid connection parameter entry.

src/backend/distributed/connection/connection_configuration.c

@@ -80,6 +80,7 @@ ResetConnParams()
 
 	for (paramIdx = 0; paramIdx < ConnParams.size; paramIdx++)
 	{
+		/* FIXME: People still have references to these! */
 		free((void *) ConnParams.keywords[paramIdx]);
 		free((void *) ConnParams.values[paramIdx]);
 
@@ -94,15 +95,14 @@ ResetConnParams()
 
 /*
  * AddConnParam adds a parameter setting to the global libpq settings according
- * to the provided keyword and value. Under assert-enabled builds, array bounds
- * checking is performed.
+ * to the provided keyword and value.
  */
 void
 AddConnParam(const char *keyword, const char *value)
 {
 	if (ConnParams.size + 1 >= ConnParams.maxSize)
 	{
-		/* we expect developers to see that error messages */
+		/* hopefully this error is only seen by developers */
 		ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_RESOURCES),
 						errmsg("ConnParams arrays bound check failed")));
 	}
@@ -227,7 +227,7 @@ CheckConninfo(const char *conninfo, const char **whitelist,
  */
 void
 GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
-			  MemoryContext context)
+			  Index *nonGlobalParamStart, MemoryContext context)
 {
 	/* make space for the port as a string: sign, 10 digits, NUL */
 	char *nodePortString = MemoryContextAlloc(context, 12 * sizeof(char *));
@@ -241,16 +241,24 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
 	 * The global parameters have already been assigned from a GUC, so begin by
 	 * calculating the key-specific parameters (basically just the fields of
 	 * the key and the active database encoding).
+	 *
+	 * We allocate everything in the provided context so as to facilitate using
+	 * pfree on all runtime parameters when connections using these entries are
+	 * invalidated during config reloads.
 	 */
 	const char *runtimeKeywords[] = {
-		"host", "port", "dbname", "user", "client_encoding"
+		MemoryContextStrdup(context, "host"),
+		MemoryContextStrdup(context, "port"),
+		MemoryContextStrdup(context, "dbname"),
+		MemoryContextStrdup(context, "user"),
+		MemoryContextStrdup(context, "client_encoding")
 	};
 	const char *runtimeValues[] = {
 		MemoryContextStrdup(context, key->hostname),
 		nodePortString,
 		MemoryContextStrdup(context, key->database),
 		MemoryContextStrdup(context, key->user),
-		GetDatabaseEncodingName()
+		MemoryContextStrdup(context, GetDatabaseEncodingName())
 	};
 
 	/*
@@ -265,12 +273,12 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
 	/* auth keywords will begin after global and runtime ones are appended */
 	Index authParamsIdx = ConnParams.size + lengthof(runtimeKeywords);
 
-	int paramIndex = 0;
-	int runtimeParamIndex = 0;
+	Index paramIndex = 0;
+	Index runtimeParamIndex = 0;
 
 	if (ConnParams.size + lengthof(runtimeKeywords) >= ConnParams.maxSize)
 	{
-		/* unexpected, intended as developers rather than users */
+		/* hopefully this error is only seen by developers */
 		ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 						errmsg("too many connParams entries")));
 	}
@@ -302,6 +310,7 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
 
 	*keywords = connKeywords;
 	*values = connValues;
+	*nonGlobalParamStart = ConnParams.size;
 }
 
 
@@ -312,7 +321,7 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
 const char *
 GetConnParam(const char *keyword)
 {
-	int i = 0;
+	Index i = 0;
 
 	for (i = 0; i < ConnParams.size; i++)
 	{

src/backend/distributed/connection/connection_management.c

@@ -694,8 +694,30 @@ StartConnectionEstablishment(ConnectionHashKey *key)
 	entry = hash_search(ConnParamsHash, key, HASH_ENTER, &found);
 	if (!found || !entry->isValid)
 	{
-		/* if they're not found, compute them from GUC, runtime, etc. */
-		GetConnParams(key, &entry->keywords, &entry->values, ConnectionContext);
+		if (found && !entry->isValid)
+		{
+			char **keyword = &entry->keywords[entry->nonGlobalParamStart];
+			char **value = &entry->values[entry->nonGlobalParamStart];
+
+			while (*keyword != NULL)
+			{
+				pfree(*keyword);
+				keyword++;
+			}
+
+			while (*value != NULL)
+			{
+				pfree(*value);
+				value++;
+			}
+
+			pfree(entry->keywords);
+			pfree(entry->values);
+		}
+
+		/* if not found or not valid, compute them from GUC, runtime, etc. */
+		GetConnParams(key, &entry->keywords, &entry->values, &entry->nonGlobalParamStart,
+					  ConnectionContext);
 
 		entry->isValid = true;
 	}

src/include/distributed/connection_management.h

@@ -118,6 +118,7 @@ typedef struct ConnParamsHashEntry
 {
 	ConnectionHashKey key;
 	bool isValid;
+	Index nonGlobalParamStart;
 	char **keywords;
 	char **values;
 } ConnParamsHashEntry;
@@ -144,7 +145,7 @@ extern void InitConnParams(void);
 extern void ResetConnParams(void);
 extern void AddConnParam(const char *keyword, const char *value);
 extern void GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values,
-						  MemoryContext context);
+						  Index *nonGlobalParamStart, MemoryContext context);
 extern const char * GetConnParam(const char *keyword);
 extern bool CheckConninfo(const char *conninfo, const char **whitelist,
 						  Size whitelistLength, char **errmsg);