diff options
author | Peter Eisentraut <peter@eisentraut.org> | 2022-09-16 14:51:47 +0200 |
---|---|---|
committer | Peter Eisentraut <peter@eisentraut.org> | 2022-09-16 14:53:12 +0200 |
commit | 5ac51c8c9e4434140f4ba45b7bdb38896b48cc64 (patch) | |
tree | 1b55802b0fad234d98c2e6ea0488883cd467875f /src | |
parent | 1e08576691bf1a25c0e28745e5e800c44f2a1c76 (diff) | |
download | postgresql-5ac51c8c9e4434140f4ba45b7bdb38896b48cc64.tar.gz postgresql-5ac51c8c9e4434140f4ba45b7bdb38896b48cc64.zip |
Adjust assorted hint messages that list all valid options.
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
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/foreign/foreign.c | 26 | ||||
-rw-r--r-- | src/backend/utils/adt/varlena.c | 82 | ||||
-rw-r--r-- | src/include/utils/varlena.h | 12 | ||||
-rw-r--r-- | src/test/regress/expected/foreign_data.out | 6 |
4 files changed, 113 insertions, 13 deletions
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c index cf222fc3e99..353e20a0cf7 100644 --- a/src/backend/foreign/foreign.c +++ b/src/backend/foreign/foreign.c @@ -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); } diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 816c66b7e77..1f6e0908216 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -6198,6 +6198,88 @@ rest_of_char_same(const char *s1, const char *s2, int len) /* + * 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 */ diff --git a/src/include/utils/varlena.h b/src/include/utils/varlena.h index c45208a2048..a4a0566622c 100644 --- a/src/include/utils/varlena.h +++ b/src/include/utils/varlena.h @@ -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 diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index 33505352cc5..9d7610b9484 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -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'); |