Sane error message if connection to myself in alter_system_c failed.

Also, yet another bug in check_sub_sync covered: sometimes learning sub lsn
returns row, but the lsn itself is null.

Author: arssher

src/copypart.c

@@ -930,39 +930,53 @@ int
 check_sub_sync(const char *subname, PGconn **conn, XLogRecPtr ref_lsn,
 			   const char *log_pref)
 {
-	PGresult *res;
+	PGresult *res = NULL;
 	char *received_lsn_str;
 	XLogRecPtr received_lsn;
 	char *sql = received_lsn_sql(subname);
 
 	res = PQexec(*conn, sql);
-	pfree(sql);
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
 		shmn_elog(LOG, "%s: failed to learn sub lsn on src: %s",
 				  log_pref, PQerrorMessage(*conn));
 		reset_pqconn_and_res(conn, res);
-		return -1;
+		res = NULL; /* TODO: reset_res */
+		goto fail;
 	}
 	/* FIXME: this should never be true, but sometimes it is. */
 	if (PQntuples(res) != 1)
 	{
 		shmn_elog(LOG, "learning sub %s lsn returned %d rows, query %s",
 				  subname, PQntuples(res), sql);
-		PQclear(res);
-		return -1;
+		goto fail;
+	}
+	/* FIXME: this should never be true, but sometimes it is. */
+	if (PQgetisnull(res, 0, 0))
+	{
+		shmn_elog(LOG, "learning sub %s lsn returned NULL received_lsn, query %s",
+				  subname, sql);
+		goto fail;
 	}
 	received_lsn_str = PQgetvalue(res, 0, 0);
 	shmn_elog(DEBUG1, "%s: received_lsn is %s", log_pref, received_lsn_str);
 	received_lsn = pg_lsn_in_c(received_lsn_str);
-	PQclear(res);
 	if (received_lsn < ref_lsn)
 	{
 		shmn_elog(DEBUG1, "%s: sub is not yet synced, received_lsn is %lu, "
 				  " but we wait for %lu", log_pref, received_lsn, ref_lsn);
-		return -1;
+		goto fail;
 	}
+
+	PQclear(res);
+	pfree(sql);
 	return 0;
+
+fail:
+	/* TODO: reset_res */
+	PQclear(res);
+	pfree(sql);
+	return -1;
 }
 
 /*

src/pg_shardman.c

@@ -306,6 +306,7 @@ listen_cmd_log_inserts(void)
 {
 	PGresult   *res;
 
+	/* TODO: make sure we connect to ourselves, probably cmp ports */
 	conn = PQconnectdb(shardman_shardlord_connstring);
 	/* Check to see that the backend connection was successfully made */
 	if (PQstatus(conn) != CONNECTION_OK)
@@ -864,13 +865,13 @@ rm_node(Cmd *cmd)
 
 /*
  * Finish pq connection and set ptr to NULL. You must be sure that the
- * connection exists!
+ * connection exists or ptr is NULL.
  */
 void
 reset_pqconn(PGconn **conn) { PQfinish(*conn); *conn = NULL; }
 /*
  * Same, but also clear res. You must be sure that both connection and res
- * exist.
+ * exist or they are NULL ptrs.
  */
 void
 reset_pqconn_and_res(PGconn **conn, PGresult *res)

src/udf.c

@@ -415,20 +415,23 @@ alter_system_c(PG_FUNCTION_ARGS)
 	conn = PQconnectdb(connstr);
 	if (PQstatus(conn) != CONNECTION_OK)
 	{
-		PQfinish(conn);
-		elog(ERROR, "Connection to myself with connstr %s failed", connstr);
-
+		elog(WARNING, "Connection to myself with connstr %s failed: %s",
+			 connstr, PQerrorMessage(conn));
+		goto fail;
 	}
 	SPI_finish(); /* Can't do earlier since connstr is allocated there */
 	res = PQexec(conn, cmd);
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 	{
-		PQclear(res);
-		PQfinish(conn);
-		elog(ERROR, "setting %s to %s failed", opt, val);
+		elog(WARNING, "setting %s to %s failed", opt, val);
+		goto fail;
 	}
 
 	PQclear(res);
 	PQfinish(conn);
 	PG_RETURN_VOID();
+
+fail:
+	reset_pqconn_and_res(&conn, res);
+	elog(ERROR, "alter_system_c failed");
 }