From 92788a0d9ca70fb5ebf76be57dc03a9dc9d34c38 Mon Sep 17 00:00:00 2001 From: Amos Bird Date: Sun, 15 May 2016 19:09:49 +0800 Subject: [PATCH 1/2] Remove redundant implementations of error funcs. This patch does some basic cleaning jobs. It removes duplicated implementations of ReportRemoteError() and related ones and adjusts regression tests. --- .../executor/multi_client_executor.c | 106 +----------------- .../expected/multi_index_statements.out | 8 +- .../multi_alter_table_statements.source | 16 +-- src/test/regress/output/multi_copy.source | 4 +- 4 files changed, 19 insertions(+), 115 deletions(-) diff --git a/src/backend/distributed/executor/multi_client_executor.c b/src/backend/distributed/executor/multi_client_executor.c index 361688348..c3cb4edea 100644 --- a/src/backend/distributed/executor/multi_client_executor.c +++ b/src/backend/distributed/executor/multi_client_executor.c @@ -19,6 +19,7 @@ #include "commands/dbcommands.h" #include "distributed/metadata_cache.h" +#include "distributed/connection_cache.h" #include "distributed/multi_client_executor.h" #include @@ -49,9 +50,6 @@ static PostgresPollingStatusType ClientPollingStatusArray[MAX_CONNECTION_COUNT]; static void ClearRemainingResults(PGconn *connection); static bool ClientConnectionReady(PGconn *connection, PostgresPollingStatusType pollingStatus); -static void ReportRemoteError(PGconn *connection, PGresult *result); -static void ReportConnectionError(PGconn *connection); -static char * ConnectionGetOptionValue(PGconn *connection, char *optionKeyword); /* AllocateConnectionId returns a connection id from the connection pool. */ @@ -142,7 +140,7 @@ MultiClientConnect(const char *nodeName, uint32 nodePort, const char *nodeDataba } else { - ReportConnectionError(connection); + ReportRemoteError(connection, NULL); PQfinish(connection); connectionId = INVALID_CONNECTION_ID; @@ -195,7 +193,7 @@ MultiClientConnectStart(const char *nodeName, uint32 nodePort, const char *nodeD } else { - ReportConnectionError(connection); + ReportRemoteError(connection, NULL); PQfinish(connection); connectionId = INVALID_CONNECTION_ID; @@ -244,7 +242,7 @@ MultiClientConnectPoll(int32 connectionId) } else if (pollingStatus == PGRES_POLLING_FAILED) { - ReportConnectionError(connection); + ReportRemoteError(connection, NULL); connectStatus = CLIENT_CONNECTION_BAD; } @@ -680,7 +678,7 @@ MultiClientCopyData(int32 connectionId, int32 fileDescriptor) /* received an error */ copyStatus = CLIENT_COPY_FAILED; - ReportConnectionError(connection); + ReportRemoteError(connection, NULL); } /* if copy out completed, make sure we drain all results from libpq */ @@ -806,97 +804,3 @@ ClientConnectionReady(PGconn *connection, PostgresPollingStatusType pollingStatu return clientConnectionReady; } - - -/* - * ReportRemoteError retrieves various error fields from the a remote result and - * produces an error report at the WARNING level. - */ -static void -ReportRemoteError(PGconn *connection, PGresult *result) -{ - char *sqlStateString = PQresultErrorField(result, PG_DIAG_SQLSTATE); - char *remoteMessage = PQresultErrorField(result, PG_DIAG_MESSAGE_PRIMARY); - char *nodeName = ConnectionGetOptionValue(connection, "host"); - char *nodePort = ConnectionGetOptionValue(connection, "port"); - char *errorPrefix = "could not connect to node"; - int sqlState = ERRCODE_CONNECTION_FAILURE; - - if (sqlStateString != NULL) - { - sqlState = MAKE_SQLSTATE(sqlStateString[0], sqlStateString[1], sqlStateString[2], - sqlStateString[3], sqlStateString[4]); - - /* use more specific error prefix for result failures */ - if (sqlState != ERRCODE_CONNECTION_FAILURE) - { - errorPrefix = "could not receive query results from"; - } - } - - /* - * If the PGresult did not contain a message, the connection may provide a - * suitable top level one. At worst, this is an empty string. - */ - if (remoteMessage == NULL) - { - char *lastNewlineIndex = NULL; - - remoteMessage = PQerrorMessage(connection); - lastNewlineIndex = strrchr(remoteMessage, '\n'); - - /* trim trailing newline, if any */ - if (lastNewlineIndex != NULL) - { - *lastNewlineIndex = '\0'; - } - } - - ereport(WARNING, (errcode(sqlState), - errmsg("%s %s:%s", errorPrefix, nodeName, nodePort), - errdetail("Client error: %s", remoteMessage))); -} - - -/* - * ReportConnectionError raises a WARNING and reports that we could not - * establish the given connection. - */ -static void -ReportConnectionError(PGconn *connection) -{ - char *nodeName = ConnectionGetOptionValue(connection, "host"); - char *nodePort = ConnectionGetOptionValue(connection, "port"); - char *errorMessage = PQerrorMessage(connection); - - ereport(WARNING, (errcode(ERRCODE_CONNECTION_FAILURE), - errmsg("could not connect to node %s:%s", nodeName, nodePort), - errdetail("Client error: %s", errorMessage))); -} - - -/* - * ConnectionGetOptionValue inspects the provided connection for an option with - * a given keyword and returns a new palloc'd string with that options's value. - * The function returns NULL if the connection has no setting for an option with - * the provided keyword. - */ -static char * -ConnectionGetOptionValue(PGconn *connection, char *optionKeyword) -{ - char *optionValue = NULL; - PQconninfoOption *option = NULL; - PQconninfoOption *conninfoOptions = PQconninfo(connection); - - for (option = conninfoOptions; option->keyword != NULL; option++) - { - if (strncmp(option->keyword, optionKeyword, NAMEDATALEN) == 0) - { - optionValue = pstrdup(option->val); - } - } - - PQconninfoFree(conninfoOptions); - - return optionValue; -} diff --git a/src/test/regress/expected/multi_index_statements.out b/src/test/regress/expected/multi_index_statements.out index f32d15cf6..2fd7a89e6 100644 --- a/src/test/regress/expected/multi_index_statements.out +++ b/src/test/regress/expected/multi_index_statements.out @@ -137,12 +137,12 @@ ERROR: creating unique indexes on append-partitioned tables is currently unsupp CREATE INDEX lineitem_orderkey_index ON lineitem (l_orderkey); ERROR: relation "lineitem_orderkey_index" already exists CREATE INDEX try_index ON lineitem USING gist (l_orderkey); -WARNING: could not receive query results from localhost:57637 -DETAIL: Client error: data type bigint has no default operator class for access method "gist" +WARNING: Bad result from localhost:57637 +DETAIL: Remote message: data type bigint has no default operator class for access method "gist" ERROR: could not execute DDL command on worker node shards CREATE INDEX try_index ON lineitem (non_existent_column); -WARNING: could not receive query results from localhost:57637 -DETAIL: Client error: column "non_existent_column" does not exist +WARNING: Bad result from localhost:57637 +DETAIL: Remote message: column "non_existent_column" does not exist ERROR: could not execute DDL command on worker node shards CREATE INDEX ON lineitem (l_orderkey); ERROR: creating index without a name on a distributed table is currently unsupported diff --git a/src/test/regress/output/multi_alter_table_statements.source b/src/test/regress/output/multi_alter_table_statements.source index 7bab12bf2..78f0b380c 100644 --- a/src/test/regress/output/multi_alter_table_statements.source +++ b/src/test/regress/output/multi_alter_table_statements.source @@ -260,8 +260,8 @@ ALTER TABLE IF EXISTS non_existent_table ADD COLUMN new_column INTEGER; NOTICE: relation "non_existent_table" does not exist, skipping ALTER TABLE IF EXISTS lineitem_alter ALTER COLUMN int_column2 SET DATA TYPE INTEGER; ALTER TABLE lineitem_alter DROP COLUMN non_existent_column; -WARNING: could not receive query results from localhost:57638 -DETAIL: Client error: column "non_existent_column" of relation "lineitem_alter_103000" does not exist +WARNING: Bad result from localhost:57638 +DETAIL: Remote message: column "non_existent_column" of relation "lineitem_alter_103000" does not exist ERROR: could not execute DDL command on worker node shards ALTER TABLE lineitem_alter DROP COLUMN IF EXISTS non_existent_column; NOTICE: column "non_existent_column" of relation "lineitem_alter" does not exist, skipping @@ -360,16 +360,16 @@ DETAIL: Only ADD|DROP COLUMN, SET|DROP NOT NULL, SET|DROP DEFAULT and TYPE subc -- Verify that we error out in case of postgres errors on supported statement -- types ALTER TABLE lineitem_alter ADD COLUMN new_column non_existent_type; -WARNING: could not receive query results from localhost:57638 -DETAIL: Client error: type "non_existent_type" does not exist +WARNING: Bad result from localhost:57638 +DETAIL: Remote message: type "non_existent_type" does not exist ERROR: could not execute DDL command on worker node shards ALTER TABLE lineitem_alter ALTER COLUMN null_column SET NOT NULL; -WARNING: could not receive query results from localhost:57638 -DETAIL: Client error: column "null_column" contains null values +WARNING: Bad result from localhost:57638 +DETAIL: Remote message: column "null_column" contains null values ERROR: could not execute DDL command on worker node shards ALTER TABLE lineitem_alter ALTER COLUMN l_partkey SET DEFAULT 'a'; -WARNING: could not receive query results from localhost:57638 -DETAIL: Client error: invalid input syntax for integer: "a" +WARNING: Bad result from localhost:57638 +DETAIL: Remote message: invalid input syntax for integer: "a" ERROR: could not execute DDL command on worker node shards -- Verify that we error out on statements involving RENAME ALTER TABLE lineitem_alter RENAME TO lineitem_renamed; diff --git a/src/test/regress/output/multi_copy.source b/src/test/regress/output/multi_copy.source index 76c77415c..8a5d49a31 100644 --- a/src/test/regress/output/multi_copy.source +++ b/src/test/regress/output/multi_copy.source @@ -358,8 +358,8 @@ COPY customer_worker_copy_append FROM '@abs_srcdir@/data/customer.1.data' with ( COPY customer_worker_copy_append FROM '@abs_srcdir@/data/customer.2.data' with (delimiter '|', master_host 'localhost', master_port 57636); -- Test if there is no relation to copy data with the worker copy COPY lineitem_copy_none FROM '@abs_srcdir@/data/lineitem.1.data' with (delimiter '|', master_host 'localhost', master_port 57636); -WARNING: could not receive query results from localhost:57636 -DETAIL: Client error: relation "lineitem_copy_none" does not exist +WARNING: Bad result from localhost:57636 +DETAIL: Remote message: relation "lineitem_copy_none" does not exist ERROR: could not run copy from the worker node -- Connect back to the master node \c - - - 57636 From e774f22ed407277837639f94d5b4492353d6b54d Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Fri, 27 May 2016 15:13:28 -0600 Subject: [PATCH 2/2] Fix formatting Checking in citus_indent output. --- src/backend/distributed/executor/multi_client_executor.c | 4 ++-- .../distributed/master/master_modify_multiple_shards.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/executor/multi_client_executor.c b/src/backend/distributed/executor/multi_client_executor.c index c3cb4edea..76c53de1e 100644 --- a/src/backend/distributed/executor/multi_client_executor.c +++ b/src/backend/distributed/executor/multi_client_executor.c @@ -140,7 +140,7 @@ MultiClientConnect(const char *nodeName, uint32 nodePort, const char *nodeDataba } else { - ReportRemoteError(connection, NULL); + ReportRemoteError(connection, NULL); PQfinish(connection); connectionId = INVALID_CONNECTION_ID; @@ -678,7 +678,7 @@ MultiClientCopyData(int32 connectionId, int32 fileDescriptor) /* received an error */ copyStatus = CLIENT_COPY_FAILED; - ReportRemoteError(connection, NULL); + ReportRemoteError(connection, NULL); } /* if copy out completed, make sure we drain all results from libpq */ diff --git a/src/backend/distributed/master/master_modify_multiple_shards.c b/src/backend/distributed/master/master_modify_multiple_shards.c index e4144e6b1..65aef5aa8 100644 --- a/src/backend/distributed/master/master_modify_multiple_shards.c +++ b/src/backend/distributed/master/master_modify_multiple_shards.c @@ -64,8 +64,8 @@ PG_FUNCTION_INFO_V1(master_modify_multiple_shards); /* - * master_modify_multiple_shards takes in a DELETE or UPDATE query string and - * pushes the query to shards. It finds shards that match the criteria defined + * master_modify_multiple_shards takes in a DELETE or UPDATE query string and + * pushes the query to shards. It finds shards that match the criteria defined * in the delete command, generates the same delete query string for each of the * found shards with distributed table name replaced with the shard name and * sends the queries to the workers. It uses one-phase or two-phase commit