Adjust assorted hint messages that list all valid options.
authorPeter Eisentraut <peter@eisentraut.org>
Fri, 16 Sep 2022 12:51:47 +0000 (14:51 +0200)
committerPeter Eisentraut <peter@eisentraut.org>
Fri, 16 Sep 2022 12:53:12 +0000 (14:53 +0200)
Instead of listing all valid options, we now try to provide one
that looks similar.  Since this may be useful elsewhere, this
change introduces a new set of functions that can be reused for
similar purposes.

Author: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/b1f9f399-3a1a-b554-283f-4ae7f34608e2@enterprisedb.com

contrib/dblink/dblink.c
contrib/dblink/expected/dblink.out
contrib/file_fdw/expected/file_fdw.out
contrib/file_fdw/file_fdw.c
contrib/postgres_fdw/expected/postgres_fdw.out
contrib/postgres_fdw/option.c
src/backend/foreign/foreign.c
src/backend/utils/adt/varlena.c
src/include/utils/varlena.h
src/test/regress/expected/foreign_data.out

index 3df3f9bbe9fa6be5fcb782f4e9dc7ede3de010b9..41cf45e87833c44b6d41af1215d891a037a05c3d 100644 (file)
@@ -2008,27 +2008,32 @@ dblink_fdw_validator(PG_FUNCTION_ARGS)
        {
            /*
             * Unknown option, or invalid option for the context specified, so
-            * complain about it.  Provide a hint with list of valid options
-            * for the context.
+            * complain about it.  Provide a hint with a valid option that
+            * looks similar, if there is one.
             */
-           StringInfoData buf;
            const PQconninfoOption *opt;
+           const char *closest_match;
+           ClosestMatchState match_state;
+           bool        has_valid_options = false;
 
-           initStringInfo(&buf);
+           initClosestMatch(&match_state, def->defname, 4);
            for (opt = options; opt->keyword; opt++)
            {
                if (is_valid_dblink_option(options, opt->keyword, context))
-                   appendStringInfo(&buf, "%s%s",
-                                    (buf.len > 0) ? ", " : "",
-                                    opt->keyword);
+               {
+                   has_valid_options = true;
+                   updateClosestMatch(&match_state, opt->keyword);
+               }
            }
+
+           closest_match = getClosestMatch(&match_state);
            ereport(ERROR,
                    (errcode(ERRCODE_FDW_OPTION_NAME_NOT_FOUND),
                     errmsg("invalid option \"%s\"", def->defname),
-                    buf.len > 0
-                    ? errhint("Valid options in this context are: %s",
-                              buf.data)
-                    errhint("There are no valid options in this context.")));
+                    has_valid_options ? closest_match ?
+                    errhint("Perhaps you meant the option \"%s\".",
+                            closest_match) : 0 :
+                    errhint("There are no valid options in this context.")));
        }
    }
 
index c7bde6ad076de2a452152c55c5f8cc3c821cd30a..14d015e4d5e5cc2108d75335ca1398c953276657 100644 (file)
@@ -897,7 +897,6 @@ $d$;
 CREATE USER MAPPING FOR public SERVER fdtest
   OPTIONS (server 'localhost');  -- fail, can't specify server here
 ERROR:  invalid option "server"
-HINT:  Valid options in this context are: user, password, sslpassword
 CREATE USER MAPPING FOR public SERVER fdtest OPTIONS (user :'USER');
 GRANT USAGE ON FOREIGN SERVER fdtest TO regress_dblink_user;
 GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO regress_dblink_user;
index 261af1a8b5fe5f2986c24a6ee6301245bb3de0f6..36d76ba26c305851a473e42d43d1f6de6685fa5f 100644 (file)
@@ -166,7 +166,6 @@ ERROR:  invalid option "force_not_null"
 HINT:  There are no valid options in this context.
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (force_not_null '*'); -- ERROR
 ERROR:  invalid option "force_not_null"
-HINT:  Valid options in this context are: filename, program, format, header, delimiter, quote, escape, null, encoding
 -- force_null is not allowed to be specified at any foreign object level:
 ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (ADD force_null '*'); -- ERROR
 ERROR:  invalid option "force_null"
@@ -179,7 +178,6 @@ ERROR:  invalid option "force_null"
 HINT:  There are no valid options in this context.
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (force_null '*'); -- ERROR
 ERROR:  invalid option "force_null"
-HINT:  Valid options in this context are: filename, program, format, header, delimiter, quote, escape, null, encoding
 -- basic query tests
 SELECT * FROM agg_text WHERE b > 10.0 ORDER BY a;
   a  |   b    
index 4773cadec09d3cc049abcc5690fbb2db45468b8e..de0b9a109c2659d76afb4d8be7f6b2c343026807 100644 (file)
@@ -37,6 +37,7 @@
 #include "utils/memutils.h"
 #include "utils/rel.h"
 #include "utils/sampling.h"
+#include "utils/varlena.h"
 
 PG_MODULE_MAGIC;
 
@@ -214,27 +215,32 @@ file_fdw_validator(PG_FUNCTION_ARGS)
        if (!is_valid_option(def->defname, catalog))
        {
            const struct FileFdwOption *opt;
-           StringInfoData buf;
+           const char *closest_match;
+           ClosestMatchState match_state;
+           bool        has_valid_options = false;
 
            /*
             * Unknown option specified, complain about it. Provide a hint
-            * with list of valid options for the object.
+            * with a valid option that looks similar, if there is one.
             */
-           initStringInfo(&buf);
+           initClosestMatch(&match_state, def->defname, 4);
            for (opt = valid_options; opt->optname; opt++)
            {
                if (catalog == opt->optcontext)
-                   appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "",
-                                    opt->optname);
+               {
+                   has_valid_options = true;
+                   updateClosestMatch(&match_state, opt->optname);
+               }
            }
 
+           closest_match = getClosestMatch(&match_state);
            ereport(ERROR,
                    (errcode(ERRCODE_FDW_INVALID_OPTION_NAME),
                     errmsg("invalid option \"%s\"", def->defname),
-                    buf.len > 0
-                    ? errhint("Valid options in this context are: %s",
-                              buf.data)
-                    errhint("There are no valid options in this context.")));
+                    has_valid_options ? closest_match ?
+                    errhint("Perhaps you meant the option \"%s\".",
+                            closest_match) : 0 :
+                    errhint("There are no valid options in this context.")));
        }
 
        /*
index 1cf54456e5ea293ec3064c6d8cfe999ac99a18ee..ad0910e8672c1f0a389ea4d4de05a6eb7223ccd2 100644 (file)
@@ -188,7 +188,6 @@ ALTER USER MAPPING FOR public SERVER testserver1
 ALTER USER MAPPING FOR public SERVER testserver1
    OPTIONS (ADD sslmode 'require');
 ERROR:  invalid option "sslmode"
-HINT:  Valid options in this context are: user, password, sslpassword, password_required, sslcert, sslkey
 -- But we can add valid ones fine
 ALTER USER MAPPING FOR public SERVER testserver1
    OPTIONS (ADD sslpassword 'dummy');
@@ -9706,7 +9705,7 @@ DO $d$
     END;
 $d$;
 ERROR:  invalid option "password"
-HINT:  Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, truncatable, fetch_size, batch_size, async_capable, parallel_commit, keep_connections
+HINT:  Perhaps you meant the option "passfile".
 CONTEXT:  SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
 PL/pgSQL function inline_code_block line 3 at EXECUTE
 -- If we add a password for our user mapping instead, we should get a different
index 95dde056ebaaf92448957cf458893936f133031d..fa80ee2a55ed725e1d80f54921d5a891bc307f07 100644 (file)
@@ -90,26 +90,31 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
        {
            /*
             * Unknown option specified, complain about it. Provide a hint
-            * with list of valid options for the object.
+            * with a valid option that looks similar, if there is one.
             */
            PgFdwOption *opt;
-           StringInfoData buf;
+           const char *closest_match;
+           ClosestMatchState match_state;
+           bool        has_valid_options = false;
 
-           initStringInfo(&buf);
+           initClosestMatch(&match_state, def->defname, 4);
            for (opt = postgres_fdw_options; opt->keyword; opt++)
            {
                if (catalog == opt->optcontext)
-                   appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "",
-                                    opt->keyword);
+               {
+                   has_valid_options = true;
+                   updateClosestMatch(&match_state, opt->keyword);
+               }
            }
 
+           closest_match = getClosestMatch(&match_state);
            ereport(ERROR,
                    (errcode(ERRCODE_FDW_INVALID_OPTION_NAME),
                     errmsg("invalid option \"%s\"", def->defname),
-                    buf.len > 0
-                    ? errhint("Valid options in this context are: %s",
-                              buf.data)
-                    errhint("There are no valid options in this context.")));
+                    has_valid_options ? closest_match ?
+                    errhint("Perhaps you meant the option \"%s\".",
+                            closest_match) : 0 :
+                    errhint("There are no valid options in this context.")));
        }
 
        /*
index cf222fc3e998fcf5293bf72f64771f7e6d97f945..353e20a0cf7089705d303c857e340ec665183422 100644 (file)
@@ -27,6 +27,7 @@
 #include "utils/memutils.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
+#include "utils/varlena.h"
 
 
 /*
@@ -621,25 +622,32 @@ postgresql_fdw_validator(PG_FUNCTION_ARGS)
        if (!is_conninfo_option(def->defname, catalog))
        {
            const struct ConnectionOption *opt;
-           StringInfoData buf;
+           const char *closest_match;
+           ClosestMatchState match_state;
+           bool        has_valid_options = false;
 
            /*
             * Unknown option specified, complain about it. Provide a hint
-            * with list of valid options for the object.
+            * with a valid option that looks similar, if there is one.
             */
-           initStringInfo(&buf);
+           initClosestMatch(&match_state, def->defname, 4);
            for (opt = libpq_conninfo_options; opt->optname; opt++)
+           {
                if (catalog == opt->optcontext)
-                   appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "",
-                                    opt->optname);
+               {
+                   has_valid_options = true;
+                   updateClosestMatch(&match_state, opt->optname);
+               }
+           }
 
+           closest_match = getClosestMatch(&match_state);
            ereport(ERROR,
                    (errcode(ERRCODE_SYNTAX_ERROR),
                     errmsg("invalid option \"%s\"", def->defname),
-                    buf.len > 0
-                    ? errhint("Valid options in this context are: %s",
-                              buf.data)
-                    errhint("There are no valid options in this context.")));
+                    has_valid_options ? closest_match ?
+                    errhint("Perhaps you meant the option \"%s\".",
+                            closest_match) : 0 :
+                    errhint("There are no valid options in this context.")));
 
            PG_RETURN_BOOL(false);
        }
index 816c66b7e77a73cdd09bf3242368bd831cbe6006..1f6e09082163b2dff46ed2a1a84ff12c14451b8f 100644 (file)
@@ -6197,6 +6197,88 @@ rest_of_char_same(const char *s1, const char *s2, int len)
 #include "levenshtein.c"
 
 
+/*
+ * The following *ClosestMatch() functions can be used to determine whether a
+ * user-provided string resembles any known valid values, which is useful for
+ * providing hints in log messages, among other things.  Use these functions
+ * like so:
+ *
+ *     initClosestMatch(&state, source_string, max_distance);
+ *
+ *     for (int i = 0; i < num_valid_strings; i++)
+ *         updateClosestMatch(&state, valid_strings[i]);
+ *
+ *     closestMatch = getClosestMatch(&state);
+ */
+
+/*
+ * Initialize the given state with the source string and maximum Levenshtein
+ * distance to consider.
+ */
+void
+initClosestMatch(ClosestMatchState *state, const char *source, int max_d)
+{
+   Assert(state);
+   Assert(max_d >= 0);
+
+   state->source = source;
+   state->min_d = -1;
+   state->max_d = max_d;
+   state->match = NULL;
+}
+
+/*
+ * If the candidate string is a closer match than the current one saved (or
+ * there is no match saved), save it as the closest match.
+ *
+ * If the source or candidate string is NULL, empty, or too long, this function
+ * takes no action.  Likewise, if the Levenshtein distance exceeds the maximum
+ * allowed or more than half the characters are different, no action is taken.
+ */
+void
+updateClosestMatch(ClosestMatchState *state, const char *candidate)
+{
+   int         dist;
+
+   Assert(state);
+
+   if (state->source == NULL || state->source[0] == '\0' ||
+       candidate == NULL || candidate[0] == '\0')
+       return;
+
+   /*
+    * To avoid ERROR-ing, we check the lengths here instead of setting
+    * 'trusted' to false in the call to varstr_levenshtein_less_equal().
+    */
+   if (strlen(state->source) > MAX_LEVENSHTEIN_STRLEN ||
+       strlen(candidate) > MAX_LEVENSHTEIN_STRLEN)
+       return;
+
+   dist = varstr_levenshtein_less_equal(state->source, strlen(state->source),
+                                        candidate, strlen(candidate), 1, 1, 1,
+                                        state->max_d, true);
+   if (dist <= state->max_d &&
+       dist <= strlen(state->source) / 2 &&
+       (state->min_d == -1 || dist < state->min_d))
+   {
+       state->min_d = dist;
+       state->match = candidate;
+   }
+}
+
+/*
+ * Return the closest match.  If no suitable candidates were provided via
+ * updateClosestMatch(), return NULL.
+ */
+const char *
+getClosestMatch(ClosestMatchState *state)
+{
+   Assert(state);
+
+   return state->match;
+}
+
+
 /*
  * Unicode support
  */
index c45208a20486f24ed57b022d915a760f5f4d748f..a4a0566622c65e1a48b4c451c417b51054d5debf 100644 (file)
@@ -38,4 +38,16 @@ extern text *replace_text_regexp(text *src_text, text *pattern_text,
                                 int cflags, Oid collation,
                                 int search_start, int n);
 
+typedef struct ClosestMatchState
+{
+   const char *source;
+   int         min_d;
+   int         max_d;
+   const char *match;
+}          ClosestMatchState;
+
+extern void initClosestMatch(ClosestMatchState *state, const char *source, int max_d);
+extern void updateClosestMatch(ClosestMatchState *state, const char *candidate);
+extern const char *getClosestMatch(ClosestMatchState *state);
+
 #endif
index 33505352cc5a802282346a508c0e59594d702971..9d7610b948462b30cc204c42194015c48a030781 100644 (file)
@@ -329,7 +329,6 @@ CREATE SERVER s6 VERSION '16.0' FOREIGN DATA WRAPPER foo OPTIONS (host 'a', dbna
 CREATE SERVER s7 TYPE 'oracle' VERSION '17.0' FOREIGN DATA WRAPPER foo OPTIONS (host 'a', dbname 'b');
 CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (foo '1'); -- ERROR
 ERROR:  invalid option "foo"
-HINT:  Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
 CREATE SERVER s8 FOREIGN DATA WRAPPER postgresql OPTIONS (host 'localhost', dbname 's8db');
 \des+
                                                              List of foreign servers
@@ -440,7 +439,6 @@ ERROR:  permission denied for foreign-data wrapper foo
 RESET ROLE;
 ALTER SERVER s8 OPTIONS (foo '1');                          -- ERROR option validation
 ERROR:  invalid option "foo"
-HINT:  Valid options in this context are: authtype, service, connect_timeout, dbname, host, hostaddr, port, tty, options, requiressl, sslmode, gsslib
 ALTER SERVER s8 OPTIONS (connect_timeout '30', SET dbname 'db1', DROP host);
 SET ROLE regress_test_role;
 ALTER SERVER s1 OWNER TO regress_test_indirect;             -- ERROR
@@ -597,7 +595,7 @@ ERROR:  user mapping for "regress_foreign_data_user" already exists for server "
 CREATE USER MAPPING FOR public SERVER s4 OPTIONS ("this mapping" 'is public');
 CREATE USER MAPPING FOR user SERVER s8 OPTIONS (username 'test', password 'secret');    -- ERROR
 ERROR:  invalid option "username"
-HINT:  Valid options in this context are: user, password
+HINT:  Perhaps you meant the option "user".
 CREATE USER MAPPING FOR user SERVER s8 OPTIONS (user 'test', password 'secret');
 ALTER SERVER s5 OWNER TO regress_test_role;
 ALTER SERVER s6 OWNER TO regress_test_indirect;
@@ -636,7 +634,7 @@ ALTER USER MAPPING FOR public SERVER s5 OPTIONS (gotcha 'true');            -- E
 ERROR:  user mapping for "public" does not exist for server "s5"
 ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (username 'test');    -- ERROR
 ERROR:  invalid option "username"
-HINT:  Valid options in this context are: user, password
+HINT:  Perhaps you meant the option "user".
 ALTER USER MAPPING FOR current_user SERVER s8 OPTIONS (DROP user, SET password 'public');
 SET ROLE regress_test_role;
 ALTER USER MAPPING FOR current_user SERVER s5 OPTIONS (ADD modified '1');