unifying error messages - Mailing list pgsql-hackers

Hello,

Commit 2f698d7f4b7b introduced a message that's a bit different from
others of the same ilk,

+CHECKPOINT (MODE WRONG);
+ERROR:  unrecognized MODE option "wrong"
+LINE 1: CHECKPOINT (MODE WRONG);
+                    ^

I'd rather this said
  unrecognized value for CHECKPOINT option "mode": "wrong"

This would be in line with what we use for EXPLAIN and
CREATE_REPLICATION_SLOT.

I also found that REFRESH has a related one, apparently in commit
cc1965a99bf8 (by Kevin Grittner in July 2013), which I'm also changing
here.

Any objections to these changes?


diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index ef7c0d624f1..a5c579ce112 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -210,8 +210,8 @@ RefreshMatViewByOid(Oid matviewOid, bool is_create, bool skipData,
     if (concurrent && skipData)
         ereport(ERROR,
                 (errcode(ERRCODE_SYNTAX_ERROR),
-                 errmsg("%s and %s options cannot be used together",
-                        "CONCURRENTLY", "WITH NO DATA")));
+                 errmsg("%s options %s and %s cannot be used together",
+                        "REFRESH", "CONCURRENTLY", "WITH NO DATA")));
 
     /*
      * Check that everything is correct for a refresh. Problems at this point
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index fa4d7438442..afb3d6d8630 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -1019,7 +1019,8 @@ ExecCheckpoint(ParseState *pstate, CheckPointStmt *stmt)
             else if (strcmp(mode, "fast") != 0)
                 ereport(ERROR,
                         (errcode(ERRCODE_SYNTAX_ERROR),
-                         errmsg("unrecognized MODE option \"%s\"", mode),
+                         errmsg("unrecognized value for %s option \"%s\": \"%s\"",
+                                "CHECKPOINT", "mode", mode),
                          parser_errposition(pstate, opt->location)));
         }
         else if (strcmp(opt->defname, "flush_unlogged") == 0)
diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index d2b7c88657d..0355720dfc6 100644
--- a/src/test/regress/expected/matview.out
+++ b/src/test/regress/expected/matview.out
@@ -294,7 +294,7 @@ DROP MATERIALIZED VIEW IF EXISTS no_such_mv;
 NOTICE:  materialized view "no_such_mv" does not exist, skipping
 -- make sure invalid combination of options is prohibited
 REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_tvmm WITH NO DATA;
-ERROR:  CONCURRENTLY and WITH NO DATA options cannot be used together
+ERROR:  REFRESH options CONCURRENTLY and WITH NO DATA cannot be used together
 -- no tuple locks on materialized views
 SELECT * FROM mvtest_tvvm FOR SHARE;
 ERROR:  cannot lock rows in materialized view "mvtest_tvvm"
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 67e1860e984..cd00f35bf7a 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -965,7 +965,7 @@ ERROR:  unrecognized CHECKPOINT option "wrong"
 LINE 1: CHECKPOINT (WRONG);
                     ^
 CHECKPOINT (MODE WRONG);
-ERROR:  unrecognized MODE option "wrong"
+ERROR:  unrecognized value for CHECKPOINT option "mode": "wrong"
 LINE 1: CHECKPOINT (MODE WRONG);
                     ^
 CHECKPOINT (MODE FAST, FLUSH_UNLOGGED FALSE);

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
Al principio era UNIX, y UNIX habló y dijo: "Hello world\n".
No dijo "Hello New Jersey\n", ni "Hello USA\n".