Merge pull request #1661 from citusdata/fix_crash

Trim trailing characters & copy the message returned by `PQerrorMessage()`

src/backend/distributed/commands/multi_copy.c

@@ -1205,17 +1205,8 @@ ReportCopyError(MultiConnection *connection, PGresult *result)
 	}
 	else
 	{
-		/* probably a connection problem, get the message from the connection */
+		/* trim the trailing characters */
-		char *lastNewlineIndex = NULL;
+		remoteMessage = pchomp(PQerrorMessage(connection->pgConn));
-
-		remoteMessage = PQerrorMessage(connection->pgConn);
-		lastNewlineIndex = strrchr(remoteMessage, '\n');
-
-		/* trim trailing newline, if any */
-		if (lastNewlineIndex != NULL)
-		{
-			*lastNewlineIndex = '\0';
-		}
 
 		ereport(ERROR, (errcode(ERRCODE_IO_ERROR),
 						errmsg("failed to complete COPY on %s:%d", connection->hostname,

src/backend/distributed/connection/remote_commands.c

@@ -18,6 +18,7 @@
 #include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "storage/latch.h"
+#include "utils/palloc.h"
 
 
 /* GUC, determining whether statements sent to remote nodes are logged */
@@ -191,7 +192,7 @@ ReportConnectionError(MultiConnection *connection, int elevel)
 	int nodePort = connection->port;
 
 	ereport(elevel, (errmsg("connection error: %s:%d", nodeName, nodePort),
-					 errdetail("%s", PQerrorMessage(connection->pgConn))));
+					 errdetail("%s", pchomp(PQerrorMessage(connection->pgConn)))));
 }
 
 
@@ -229,16 +230,7 @@ ReportResultError(MultiConnection *connection, PGresult *result, int elevel)
 		 */
 		if (messagePrimary == NULL)
 		{
-			char *lastNewlineIndex = NULL;
+			messagePrimary = pchomp(PQerrorMessage(connection->pgConn));
-
-			messagePrimary = PQerrorMessage(connection->pgConn);
-			lastNewlineIndex = strrchr(messagePrimary, '\n');
-
-			/* trim trailing newline, if any */
-			if (lastNewlineIndex != NULL)
-			{
-				*lastNewlineIndex = '\0';
-			}
 		}
 
 		ereport(elevel, (errcode(sqlState), errmsg("%s", messagePrimary),
@@ -257,6 +249,28 @@ ReportResultError(MultiConnection *connection, PGresult *result, int elevel)
 }
 
 
+/* *INDENT-OFF* */
+#if (PG_VERSION_NUM < 100000)
+
+/*
+ * Make copy of string with all trailing newline characters removed.
+ */
+char *
+pchomp(const char *in)
+{
+	size_t		n;
+
+	n = strlen(in);
+	while (n > 0 && in[n - 1] == '\n')
+		n--;
+	return pnstrdup(in, n);
+}
+
+#endif
+
+/* *INDENT-ON* */
+
+
 /*
  * LogRemoteCommand logs commands send to remote nodes if
  * citus.log_remote_commands wants us to do so.

src/backend/distributed/executor/multi_client_executor.c

@@ -305,7 +305,7 @@ MultiClientSendQuery(int32 connectionId, const char *query)
 	querySent = PQsendQuery(connection->pgConn, query);
 	if (querySent == 0)
 	{
-		char *errorMessage = PQerrorMessage(connection->pgConn);
+		char *errorMessage = pchomp(PQerrorMessage(connection->pgConn));
 		ereport(WARNING, (errmsg("could not send remote query \"%s\"", query),
 						  errdetail("Client error: %s", errorMessage)));
 

src/backend/distributed/master/master_citus_tools.c

@@ -444,7 +444,12 @@ StoreErrorMessage(MultiConnection *connection, StringInfo queryResultString)
 	char *errorMessage = PQerrorMessage(connection->pgConn);
 	if (errorMessage != NULL)
 	{
-		char *firstNewlineIndex = strchr(errorMessage, '\n');
+		char *firstNewlineIndex = NULL;
+
+		/* copy the error message to a writable memory */
+		errorMessage = pnstrdup(errorMessage, strlen(errorMessage));
+
+		firstNewlineIndex = strchr(errorMessage, '\n');
 
 		/* trim the error message at the line break */
 		if (firstNewlineIndex != NULL)

src/backend/distributed/utils/colocation_utils.c

@@ -112,6 +112,9 @@ get_colocated_shard_array(PG_FUNCTION_ARGS)
 	Oid arrayTypeId = OIDOID;
 	int colocatedShardIndex = 0;
 
+	/* sort to get consistent output */
+	colocatedShardList = SortList(colocatedShardList, CompareShardIntervalsById);
+
 	foreach(colocatedShardCell, colocatedShardList)
 	{
 		ShardInterval *colocatedShardInterval = (ShardInterval *) lfirst(

src/include/distributed/remote_commands.h

@@ -33,6 +33,7 @@ extern bool SqlStateMatchesCategory(char *sqlStateString, int category);
 extern void ReportConnectionError(MultiConnection *connection, int elevel);
 extern void ReportResultError(MultiConnection *connection, struct pg_result *result,
 							  int elevel);
+extern char * pchomp(const char *in);
 extern void LogRemoteCommand(MultiConnection *connection, const char *command);
 
 /* wrappers around libpq functions, with command logging support */

src/test/regress/expected/multi_colocation_utils.out

@@ -266,39 +266,39 @@ SELECT shards_colocated(1300000, 1300020);
 (1 row)
 
 -- check co-located table list
-SELECT UNNEST(get_colocated_table_array('table1_group1'))::regclass;
+SELECT UNNEST(get_colocated_table_array('table1_group1'))::regclass ORDER BY 1;
     unnest     
 ---------------
- table2_group1
  table1_group1
+ table2_group1
 (2 rows)
 
-SELECT UNNEST(get_colocated_table_array('table5_groupX'))::regclass;
+SELECT UNNEST(get_colocated_table_array('table5_groupX'))::regclass ORDER BY 1;
     unnest     
 ---------------
  table5_groupx
 (1 row)
 
-SELECT UNNEST(get_colocated_table_array('table6_append'))::regclass;
+SELECT UNNEST(get_colocated_table_array('table6_append'))::regclass ORDER BY 1;
     unnest     
 ---------------
  table6_append
 (1 row)
 
 -- check co-located shard list
-SELECT get_colocated_shard_array(1300000);
+SELECT get_colocated_shard_array(1300000) ORDER BY 1;
  get_colocated_shard_array 
 ---------------------------
- {1300004,1300000}
+ {1300000,1300004}
 (1 row)
 
-SELECT get_colocated_shard_array(1300016);
+SELECT get_colocated_shard_array(1300016) ORDER BY 1;
  get_colocated_shard_array 
 ---------------------------
  {1300016}
 (1 row)
 
-SELECT get_colocated_shard_array(1300020);
+SELECT get_colocated_shard_array(1300020) ORDER BY 1;
  get_colocated_shard_array 
 ---------------------------
  {1300020}

src/test/regress/expected/multi_router_planner.out

@@ -2152,7 +2152,6 @@ BEGIN;
 INSERT INTO failure_test VALUES (1, 1);
 WARNING:  connection error: localhost:57638
 DETAIL:  no connection to the server
-
 SELECT shardid, shardstate, nodename, nodeport FROM pg_dist_shard_placement
 	WHERE shardid IN (
 		SELECT shardid FROM pg_dist_shard
@@ -2171,7 +2170,6 @@ ROLLBACK;
 INSERT INTO failure_test VALUES (2, 1);
 WARNING:  connection error: localhost:57638
 DETAIL:  no connection to the server
-
 SELECT shardid, shardstate, nodename, nodeport FROM pg_dist_shard_placement
 	WHERE shardid IN (
 		SELECT shardid FROM pg_dist_shard

src/test/regress/output/multi_copy.source

@@ -881,19 +881,15 @@ ALTER USER test_user WITH nologin;
 COPY numbers_hash FROM STDIN WITH (FORMAT 'csv');
 WARNING:  connection error: localhost:57637
 DETAIL:  FATAL:  role "test_user" is not permitted to log in
-
 CONTEXT:  COPY numbers_hash, line 1: "1,1"
 WARNING:  connection error: localhost:57637
 DETAIL:  FATAL:  role "test_user" is not permitted to log in
-
 CONTEXT:  COPY numbers_hash, line 2: "2,2"
 WARNING:  connection error: localhost:57637
 DETAIL:  FATAL:  role "test_user" is not permitted to log in
-
 CONTEXT:  COPY numbers_hash, line 3: "3,3"
 WARNING:  connection error: localhost:57637
 DETAIL:  FATAL:  role "test_user" is not permitted to log in
-
 CONTEXT:  COPY numbers_hash, line 6: "6,6"
 -- verify shards in the first worker as marked invalid
 SELECT shardid, shardstate, nodename, nodeport
@@ -915,7 +911,6 @@ SELECT shardid, shardstate, nodename, nodeport
 COPY numbers_reference FROM STDIN WITH (FORMAT 'csv');
 ERROR:  connection error: localhost:57637
 DETAIL:  FATAL:  role "test_user" is not permitted to log in
-
 CONTEXT:  COPY numbers_reference, line 1: "3,1"
 -- verify shards for reference table are still valid
 SELECT shardid, shardstate, nodename, nodeport
@@ -933,11 +928,9 @@ SELECT shardid, shardstate, nodename, nodeport
 COPY numbers_hash_other FROM STDIN WITH (FORMAT 'csv');
 WARNING:  connection error: localhost:57637
 DETAIL:  FATAL:  role "test_user" is not permitted to log in
-
 CONTEXT:  COPY numbers_hash_other, line 1: "1,1"
 WARNING:  connection error: localhost:57637
 DETAIL:  FATAL:  role "test_user" is not permitted to log in
-
 CONTEXT:  COPY numbers_hash_other, line 1: "1,1"
 ERROR:  could not connect to any active placements
 CONTEXT:  COPY numbers_hash_other, line 1: "1,1"

src/test/regress/sql/multi_colocation_utils.sql

@@ -134,14 +134,14 @@ SELECT shards_colocated(1300000, 1300016);
 SELECT shards_colocated(1300000, 1300020);
 
 -- check co-located table list
-SELECT UNNEST(get_colocated_table_array('table1_group1'))::regclass;
+SELECT UNNEST(get_colocated_table_array('table1_group1'))::regclass ORDER BY 1;
-SELECT UNNEST(get_colocated_table_array('table5_groupX'))::regclass;
+SELECT UNNEST(get_colocated_table_array('table5_groupX'))::regclass ORDER BY 1;
-SELECT UNNEST(get_colocated_table_array('table6_append'))::regclass;
+SELECT UNNEST(get_colocated_table_array('table6_append'))::regclass ORDER BY 1;
 
 -- check co-located shard list
-SELECT get_colocated_shard_array(1300000);
+SELECT get_colocated_shard_array(1300000) ORDER BY 1;
-SELECT get_colocated_shard_array(1300016);
+SELECT get_colocated_shard_array(1300016) ORDER BY 1;
-SELECT get_colocated_shard_array(1300020);
+SELECT get_colocated_shard_array(1300020) ORDER BY 1;
 
 -- check FindShardIntervalIndex function
 SELECT find_shard_interval_index(1300000);