From 287f472d2429927d92857dfcaf74f52019631841 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Mon, 19 Mar 2018 13:12:16 +0100 Subject: [PATCH] Align error messaging with project style guide Details and hints should have leading capitalization and end with a period. Context should not be capitalized and should not end with period. To support well formatted error contexts in dblink, extend dblink error reporting to take a format rather than a hardcoded string. Also update test .out files to match. --- contrib/dblink/dblink.c | 40 +++++++++++++++----------- contrib/dblink/expected/dblink.out | 24 ++++++++-------- contrib/file_fdw/file_fdw.c | 4 +-- contrib/pgcrypto/px.c | 4 +-- contrib/postgres_fdw/connection.c | 2 +- contrib/postgres_fdw/expected/postgres_fdw.out | 14 ++++----- contrib/postgres_fdw/option.c | 2 +- src/backend/access/transam/xlog.c | 2 +- src/backend/replication/logical/logical.c | 2 +- 9 files changed, 51 insertions(+), 43 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index ae7e24ad08..6af960b0ff 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -113,7 +113,7 @@ static char *generate_relation_name(Relation rel); static void dblink_connstr_check(const char *connstr); static void dblink_security_check(PGconn *conn, remoteConn *rconn); static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res, - const char *dblink_context_msg, bool fail); + bool fail, const char *fmt, ...); static char *get_connect_string(const char *servername); static char *escape_param_str(const char *from); static void validate_pkattnums(Relation rel, @@ -441,7 +441,8 @@ dblink_open(PG_FUNCTION_ARGS) res = PQexec(conn, buf.data); if (!res || PQresultStatus(res) != PGRES_COMMAND_OK) { - dblink_res_error(conn, conname, res, "could not open cursor", fail); + dblink_res_error(conn, conname, res, fail, + "while opening cursor \"%s\"", curname); PG_RETURN_TEXT_P(cstring_to_text("ERROR")); } @@ -509,7 +510,8 @@ dblink_close(PG_FUNCTION_ARGS) res = PQexec(conn, buf.data); if (!res || PQresultStatus(res) != PGRES_COMMAND_OK) { - dblink_res_error(conn, conname, res, "could not close cursor", fail); + dblink_res_error(conn, conname, res, fail, + "while closing cursor \"%s\"", curname); PG_RETURN_TEXT_P(cstring_to_text("ERROR")); } @@ -612,8 +614,8 @@ dblink_fetch(PG_FUNCTION_ARGS) (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK)) { - dblink_res_error(conn, conname, res, - "could not fetch from cursor", fail); + dblink_res_error(conn, conname, res, fail, + "while fetching from cursor \"%s\"", curname); return (Datum) 0; } else if (PQresultStatus(res) == PGRES_COMMAND_OK) @@ -763,8 +765,8 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool is_async) if (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK) { - dblink_res_error(conn, conname, res, - "could not execute query", fail); + dblink_res_error(conn, conname, res, fail, + "while executing query"); /* if fail isn't set, we'll return an empty query result */ } else @@ -1009,8 +1011,8 @@ materializeQueryResult(FunctionCallInfo fcinfo, PGresult *res1 = res; res = NULL; - dblink_res_error(conn, conname, res1, - "could not execute query", fail); + dblink_res_error(conn, conname, res1, fail, + "while executing query"); /* if fail isn't set, we'll return an empty query result */ } else if (PQresultStatus(res) == PGRES_COMMAND_OK) @@ -1438,8 +1440,8 @@ dblink_exec(PG_FUNCTION_ARGS) (PQresultStatus(res) != PGRES_COMMAND_OK && PQresultStatus(res) != PGRES_TUPLES_OK)) { - dblink_res_error(conn, conname, res, - "could not execute command", fail); + dblink_res_error(conn, conname, res, fail, + "while executing command"); /* * and save a copy of the command status string to return as our @@ -1980,7 +1982,7 @@ dblink_fdw_validator(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_FDW_OUT_OF_MEMORY), errmsg("out of memory"), - errdetail("could not get libpq's default connection options"))); + errdetail("Could not get libpq's default connection options."))); } /* Validate each supplied option. */ @@ -2678,7 +2680,7 @@ dblink_connstr_check(const char *connstr) static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res, - const char *dblink_context_msg, bool fail) + bool fail, const char *fmt, ...) { int level; char *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE); @@ -2692,6 +2694,8 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res, char *message_hint; char *message_context; const char *dblink_context_conname = "unnamed"; + va_list ap; + char dblink_context_msg[512]; if (fail) level = ERROR; @@ -2726,6 +2730,10 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res, if (conname) dblink_context_conname = conname; + va_start(ap, fmt); + vsnprintf(dblink_context_msg, sizeof(dblink_context_msg), fmt, ap); + va_end(ap); + ereport(level, (errcode(sqlstate), message_primary ? errmsg_internal("%s", message_primary) : @@ -2733,8 +2741,8 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res, message_detail ? errdetail_internal("%s", message_detail) : 0, message_hint ? errhint("%s", message_hint) : 0, message_context ? errcontext("%s", message_context) : 0, - errcontext("Error occurred on dblink connection named \"%s\": %s.", - dblink_context_conname, dblink_context_msg))); + errcontext("%s on dblink connection named \"%s\"", + dblink_context_msg, dblink_context_conname))); } /* @@ -2769,7 +2777,7 @@ get_connect_string(const char *servername) ereport(ERROR, (errcode(ERRCODE_FDW_OUT_OF_MEMORY), errmsg("out of memory"), - errdetail("could not get libpq's default connection options"))); + errdetail("Could not get libpq's default connection options."))); } /* first gather the server connstr options */ diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index 511691e57f..08bacbb7c6 100644 --- a/contrib/dblink/expected/dblink.out +++ b/contrib/dblink/expected/dblink.out @@ -155,7 +155,7 @@ WHERE t.a > 7; -- open a cursor with bad SQL and fail_on_error set to false SELECT dblink_open('rmt_foo_cursor','SELECT * FROM foobar',false); NOTICE: relation "foobar" does not exist -CONTEXT: Error occurred on dblink connection named "unnamed": could not open cursor. +CONTEXT: while opening cursor "rmt_foo_cursor" on dblink connection named "unnamed" dblink_open ------------- ERROR @@ -223,7 +223,7 @@ FROM dblink_fetch('rmt_foo_cursor',4) AS t(a int, b text, c text[]); SELECT * FROM dblink_fetch('rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]); NOTICE: cursor "rmt_foobar_cursor" does not exist -CONTEXT: Error occurred on dblink connection named "unnamed": could not fetch from cursor. +CONTEXT: while fetching from cursor "rmt_foobar_cursor" on dblink connection named "unnamed" a | b | c ---+---+--- (0 rows) @@ -238,7 +238,7 @@ SELECT dblink_exec('ABORT'); -- close the wrong cursor SELECT dblink_close('rmt_foobar_cursor',false); NOTICE: cursor "rmt_foobar_cursor" does not exist -CONTEXT: Error occurred on dblink connection named "unnamed": could not close cursor. +CONTEXT: while closing cursor "rmt_foobar_cursor" on dblink connection named "unnamed" dblink_close -------------- ERROR @@ -248,12 +248,12 @@ CONTEXT: Error occurred on dblink connection named "unnamed": could not close c SELECT * FROM dblink_fetch('rmt_foo_cursor',4) AS t(a int, b text, c text[]); ERROR: cursor "rmt_foo_cursor" does not exist -CONTEXT: Error occurred on dblink connection named "unnamed": could not fetch from cursor. +CONTEXT: while fetching from cursor "rmt_foo_cursor" on dblink connection named "unnamed" -- this time, 'cursor "rmt_foo_cursor" not found' as a notice SELECT * FROM dblink_fetch('rmt_foo_cursor',4,false) AS t(a int, b text, c text[]); NOTICE: cursor "rmt_foo_cursor" does not exist -CONTEXT: Error occurred on dblink connection named "unnamed": could not fetch from cursor. +CONTEXT: while fetching from cursor "rmt_foo_cursor" on dblink connection named "unnamed" a | b | c ---+---+--- (0 rows) @@ -316,7 +316,7 @@ FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[]); SELECT * FROM dblink('SELECT * FROM foobar',false) AS t(a int, b text, c text[]); NOTICE: relation "foobar" does not exist -CONTEXT: Error occurred on dblink connection named "unnamed": could not execute query. +CONTEXT: while executing query on dblink connection named "unnamed" a | b | c ---+---+--- (0 rows) @@ -340,7 +340,7 @@ WHERE a = 11; -- botch a change to some other data SELECT dblink_exec('UPDATE foobar SET f3[2] = ''b99'' WHERE f1 = 11',false); NOTICE: relation "foobar" does not exist -CONTEXT: Error occurred on dblink connection named "unnamed": could not execute command. +CONTEXT: while executing command on dblink connection named "unnamed" dblink_exec ------------- ERROR @@ -400,7 +400,7 @@ SELECT * FROM dblink('myconn','SELECT * FROM foobar',false) AS t(a int, b text, c text[]) WHERE t.a > 7; NOTICE: relation "foobar" does not exist -CONTEXT: Error occurred on dblink connection named "myconn": could not execute query. +CONTEXT: while executing query on dblink connection named "myconn" a | b | c ---+---+--- (0 rows) @@ -437,7 +437,7 @@ SELECT dblink_disconnect('myconn2'); -- open a cursor incorrectly SELECT dblink_open('myconn','rmt_foo_cursor','SELECT * FROM foobar',false); NOTICE: relation "foobar" does not exist -CONTEXT: Error occurred on dblink connection named "myconn": could not open cursor. +CONTEXT: while opening cursor "rmt_foo_cursor" on dblink connection named "myconn" dblink_open ------------- ERROR @@ -523,7 +523,7 @@ SELECT dblink_close('myconn','rmt_foo_cursor'); -- this should fail because there is no open transaction SELECT dblink_exec('myconn','DECLARE xact_test CURSOR FOR SELECT * FROM foo'); ERROR: DECLARE CURSOR can only be used in transaction blocks -CONTEXT: Error occurred on dblink connection named "myconn": could not execute command. +CONTEXT: while executing command on dblink connection named "myconn" -- reset remote transaction state SELECT dblink_exec('myconn','ABORT'); dblink_exec @@ -573,7 +573,7 @@ FROM dblink_fetch('myconn','rmt_foo_cursor',4) AS t(a int, b text, c text[]); SELECT * FROM dblink_fetch('myconn','rmt_foobar_cursor',4,false) AS t(a int, b text, c text[]); NOTICE: cursor "rmt_foobar_cursor" does not exist -CONTEXT: Error occurred on dblink connection named "myconn": could not fetch from cursor. +CONTEXT: while fetching from cursor "rmt_foobar_cursor" on dblink connection named "myconn" a | b | c ---+---+--- (0 rows) @@ -589,7 +589,7 @@ SELECT dblink_exec('myconn','ABORT'); SELECT * FROM dblink_fetch('myconn','rmt_foo_cursor',4) AS t(a int, b text, c text[]); ERROR: cursor "rmt_foo_cursor" does not exist -CONTEXT: Error occurred on dblink connection named "myconn": could not fetch from cursor. +CONTEXT: while fetching from cursor "rmt_foo_cursor" on dblink connection named "myconn" -- close the named persistent connection SELECT dblink_disconnect('myconn'); dblink_disconnect diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index dce3be2617..3df6fc741d 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -277,7 +277,7 @@ file_fdw_validator(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("conflicting or redundant options"), - errhint("option \"force_not_null\" supplied more than once for a column"))); + errhint("Option \"force_not_null\" supplied more than once for a column."))); force_not_null = def; /* Don't care what the value is, as long as it's a legal boolean */ (void) defGetBoolean(def); @@ -289,7 +289,7 @@ file_fdw_validator(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("conflicting or redundant options"), - errhint("option \"force_null\" supplied more than once for a column"))); + errhint("Option \"force_null\" supplied more than once for a column."))); force_null = def; (void) defGetBoolean(def); } diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c index 8ec920224a..aea8e863af 100644 --- a/contrib/pgcrypto/px.c +++ b/contrib/pgcrypto/px.c @@ -105,8 +105,8 @@ px_THROW_ERROR(int err) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("generating random data is not supported by this build"), - errdetail("This functionality requires a source of strong random numbers"), - errhint("You need to rebuild PostgreSQL using --enable-strong-random"))); + errdetail("This functionality requires a source of strong random numbers."), + errhint("You need to rebuild PostgreSQL using --enable-strong-random."))); #endif } else diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 00c926b983..fe4893a8e0 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -630,7 +630,7 @@ pgfdw_report_error(int elevel, PGresult *res, PGconn *conn, message_detail ? errdetail_internal("%s", message_detail) : 0, message_hint ? errhint("%s", message_hint) : 0, message_context ? errcontext("%s", message_context) : 0, - sql ? errcontext("Remote SQL command: %s", sql) : 0)); + sql ? errcontext("remote SQL command: %s", sql) : 0)); } PG_CATCH(); { diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index a2b13846e0..2d6e387d63 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -4126,7 +4126,7 @@ FETCH c; SAVEPOINT s; SELECT * FROM ft1 WHERE 1 / (c1 - 1) > 0; -- ERROR ERROR: division by zero -CONTEXT: Remote SQL command: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (((1 / ("C 1" - 1)) > 0)) +CONTEXT: remote SQL command: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (((1 / ("C 1" - 1)) > 0)) ROLLBACK TO s; FETCH c; c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 @@ -5737,7 +5737,7 @@ ALTER TABLE "S 1"."T 1" ADD CONSTRAINT c2positive CHECK (c2 >= 0); INSERT INTO ft1(c1, c2) VALUES(11, 12); -- duplicate key ERROR: duplicate key value violates unique constraint "t1_pkey" DETAIL: Key ("C 1")=(11) already exists. -CONTEXT: Remote SQL command: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) +CONTEXT: remote SQL command: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) INSERT INTO ft1(c1, c2) VALUES(11, 12) ON CONFLICT DO NOTHING; -- works INSERT INTO ft1(c1, c2) VALUES(11, 12) ON CONFLICT (c1, c2) DO NOTHING; -- unsupported ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification @@ -5746,11 +5746,11 @@ ERROR: there is no unique or exclusion constraint matching the ON CONFLICT spec INSERT INTO ft1(c1, c2) VALUES(1111, -2); -- c2positive ERROR: new row for relation "T 1" violates check constraint "c2positive" DETAIL: Failing row contains (1111, -2, null, null, null, null, ft1 , null). -CONTEXT: Remote SQL command: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) +CONTEXT: remote SQL command: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) UPDATE ft1 SET c2 = -c2 WHERE c1 = 1; -- c2positive ERROR: new row for relation "T 1" violates check constraint "c2positive" DETAIL: Failing row contains (1, -1, 00001_trig_update, 1970-01-02 08:00:00+00, 1970-01-02 00:00:00, 1, 1 , foo). -CONTEXT: Remote SQL command: UPDATE "S 1"."T 1" SET c2 = (- c2) WHERE (("C 1" = 1)) +CONTEXT: remote SQL command: UPDATE "S 1"."T 1" SET c2 = (- c2) WHERE (("C 1" = 1)) -- Test savepoint/rollback behavior select c2, count(*) from ft2 where c2 < 500 group by 1 order by 1; c2 | count @@ -5909,7 +5909,7 @@ savepoint s3; update ft2 set c2 = -2 where c2 = 42 and c1 = 10; -- fail on remote side ERROR: new row for relation "T 1" violates check constraint "c2positive" DETAIL: Failing row contains (10, -2, 00010_trig_update_trig_update, 1970-01-11 08:00:00+00, 1970-01-11 00:00:00, 0, 0 , foo). -CONTEXT: Remote SQL command: UPDATE "S 1"."T 1" SET c2 = (-2) WHERE ((c2 = 42)) AND (("C 1" = 10)) +CONTEXT: remote SQL command: UPDATE "S 1"."T 1" SET c2 = (-2) WHERE ((c2 = 42)) AND (("C 1" = 10)) rollback to savepoint s3; select c2, count(*) from ft2 where c2 < 500 group by 1 order by 1; c2 | count @@ -6126,11 +6126,11 @@ RESET constraint_exclusion; INSERT INTO ft1(c1, c2) VALUES(1111, -2); -- c2positive ERROR: new row for relation "T 1" violates check constraint "c2positive" DETAIL: Failing row contains (1111, -2, null, null, null, null, ft1 , null). -CONTEXT: Remote SQL command: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) +CONTEXT: remote SQL command: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) UPDATE ft1 SET c2 = -c2 WHERE c1 = 1; -- c2positive ERROR: new row for relation "T 1" violates check constraint "c2positive" DETAIL: Failing row contains (1, -1, 00001_trig_update, 1970-01-02 08:00:00+00, 1970-01-02 00:00:00, 1, 1 , foo). -CONTEXT: Remote SQL command: UPDATE "S 1"."T 1" SET c2 = (- c2) WHERE (("C 1" = 1)) +CONTEXT: remote SQL command: UPDATE "S 1"."T 1" SET c2 = (- c2) WHERE (("C 1" = 1)) ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c2positive; -- But inconsistent check constraints provide inconsistent results ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c2negative CHECK (c2 < 0); diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index 082f79ae04..6854f1bd91 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -196,7 +196,7 @@ InitPgFdwOptions(void) ereport(ERROR, (errcode(ERRCODE_FDW_OUT_OF_MEMORY), errmsg("out of memory"), - errdetail("could not get libpq's default connection options"))); + errdetail("Could not get libpq's default connection options."))); /* Count how many libpq options are available. */ num_libpq_opts = 0; diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 47a6c4d895..cb9c2a29cb 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -9328,7 +9328,7 @@ CreateRestartPoint(int flags) ereport((log_checkpoints ? LOG : DEBUG2), (errmsg("recovery restart point at %X/%X", (uint32) (lastCheckPoint.redo >> 32), (uint32) lastCheckPoint.redo), - xtime ? errdetail("last completed transaction was at log time %s", + xtime ? errdetail("Last completed transaction was at log time %s.", timestamptz_to_str(xtime)) : 0)); LWLockRelease(CheckpointLock); diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index e2e39f4577..3d8ad7ddf8 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -417,7 +417,7 @@ CreateDecodingContext(XLogRecPtr start_lsn, ereport(LOG, (errmsg("starting logical decoding for slot \"%s\"", NameStr(slot->data.name)), - errdetail("streaming transactions committing after %X/%X, reading WAL from %X/%X", + errdetail("Streaming transactions committing after %X/%X, reading WAL from %X/%X.", (uint32) (slot->data.confirmed_flush >> 32), (uint32) slot->data.confirmed_flush, (uint32) (slot->data.restart_lsn >> 32), -- 2.14.1.145.gb3622a4ee