From 8891cce94963a1432858cc01ebad6b000f1a0333 Mon Sep 17 00:00:00 2001 From: Shinya Sugamoto Date: Fri, 6 Feb 2026 20:33:57 +0900 Subject: [PATCH] Improve error hints for invalid COPY options Add closest-match hints for unrecognized COPY option names (e.g., "foramt" suggests "format") using the existing ClosestMatchState infrastructure. Also add HINT messages listing valid values when an invalid value is given for the FORMAT, ON_ERROR, or LOG_VERBOSITY options. Introduce a copy_option_matches() helper that combines strcmp and updateClosestMatch in a single call, replacing bare strcmp checks in ProcessCopyOptions(). The undocumented convert_selectively option intentionally uses plain strcmp so it is never suggested in hints. --- contrib/file_fdw/expected/file_fdw.out | 3 ++ src/backend/commands/copy.c | 68 ++++++++++++++++++++------ src/test/regress/expected/copy2.out | 31 ++++++++++++ src/test/regress/sql/copy2.sql | 8 +++ 4 files changed, 94 insertions(+), 16 deletions(-) diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out index 251f00bd258..6800f3c7c03 100644 --- a/contrib/file_fdw/expected/file_fdw.out +++ b/contrib/file_fdw/expected/file_fdw.out @@ -54,6 +54,7 @@ CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS ("a=b" 'true'); -- ERROR ERROR: invalid option name "a=b": must not contain "=" CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml'); -- ERROR ERROR: COPY format "xml" not recognized +HINT: Valid formats are "binary", "csv", and "text". CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':'); -- ERROR ERROR: COPY QUOTE requires CSV mode CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', escape ':'); -- ERROR @@ -96,10 +97,12 @@ CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'csv', null ' ERROR: COPY null representation cannot use newline or carriage return CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (on_error 'unsupported'); -- ERROR ERROR: COPY ON_ERROR "unsupported" not recognized +HINT: Valid values are "ignore" and "stop". CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'binary', on_error 'ignore'); -- ERROR ERROR: only ON_ERROR STOP is allowed in BINARY mode CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (log_verbosity 'unsupported'); -- ERROR ERROR: COPY LOG_VERBOSITY "unsupported" not recognized +HINT: Valid values are "default", "silent", and "verbose". CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (reject_limit '1'); -- ERROR ERROR: COPY REJECT_LIMIT requires ON_ERROR to be set to IGNORE CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (on_error 'ignore', reject_limit '0'); -- ERROR diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 155a79a70c5..e2c323bd666 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -39,6 +39,23 @@ #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/rls.h" +#include "utils/varlena.h" + +/* + * Helper to match a COPY option name during option extraction. + * + * If the option matches, return true. Otherwise, register it as a candidate + * for closest-match hints and return false. + */ +static bool +copy_option_matches(const char *defname, const char *option, + ClosestMatchState *match_state) +{ + if (strcmp(defname, option) == 0) + return true; + updateClosestMatch(match_state, option); + return false; +} /* * DoCopy executes the SQL COPY statement @@ -480,6 +497,7 @@ defGetCopyOnErrorChoice(DefElem *def, ParseState *pstate, bool is_from) (errcode(ERRCODE_INVALID_PARAMETER_VALUE), /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR */ errmsg("COPY %s \"%s\" not recognized", "ON_ERROR", sval), + errhint("Valid values are \"%s\" and \"%s\".", "ignore", "stop"), parser_errposition(pstate, def->location))); return COPY_ON_ERROR_STOP; /* keep compiler quiet */ } @@ -538,6 +556,8 @@ defGetCopyLogVerbosityChoice(DefElem *def, ParseState *pstate) (errcode(ERRCODE_INVALID_PARAMETER_VALUE), /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR */ errmsg("COPY %s \"%s\" not recognized", "LOG_VERBOSITY", sval), + errhint("Valid values are \"%s\", \"%s\", and \"%s\".", + "default", "silent", "verbose"), parser_errposition(pstate, def->location))); return COPY_LOG_VERBOSITY_DEFAULT; /* keep compiler quiet */ } @@ -582,8 +602,12 @@ ProcessCopyOptions(ParseState *pstate, foreach(option, options) { DefElem *defel = lfirst_node(DefElem, option); + ClosestMatchState match_state; + const char *closest_match; + + initClosestMatch(&match_state, defel->defname, 4); - if (strcmp(defel->defname, "format") == 0) + if (copy_option_matches(defel->defname, "format", &match_state)) { char *fmt = defGetString(defel); @@ -600,53 +624,55 @@ ProcessCopyOptions(ParseState *pstate, ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("COPY format \"%s\" not recognized", fmt), + errhint("Valid formats are \"%s\", \"%s\", and \"%s\".", + "binary", "csv", "text"), parser_errposition(pstate, defel->location))); } - else if (strcmp(defel->defname, "freeze") == 0) + else if (copy_option_matches(defel->defname, "freeze", &match_state)) { if (freeze_specified) errorConflictingDefElem(defel, pstate); freeze_specified = true; opts_out->freeze = defGetBoolean(defel); } - else if (strcmp(defel->defname, "delimiter") == 0) + else if (copy_option_matches(defel->defname, "delimiter", &match_state)) { if (opts_out->delim) errorConflictingDefElem(defel, pstate); opts_out->delim = defGetString(defel); } - else if (strcmp(defel->defname, "null") == 0) + else if (copy_option_matches(defel->defname, "null", &match_state)) { if (opts_out->null_print) errorConflictingDefElem(defel, pstate); opts_out->null_print = defGetString(defel); } - else if (strcmp(defel->defname, "default") == 0) + else if (copy_option_matches(defel->defname, "default", &match_state)) { if (opts_out->default_print) errorConflictingDefElem(defel, pstate); opts_out->default_print = defGetString(defel); } - else if (strcmp(defel->defname, "header") == 0) + else if (copy_option_matches(defel->defname, "header", &match_state)) { if (header_specified) errorConflictingDefElem(defel, pstate); header_specified = true; opts_out->header_line = defGetCopyHeaderOption(defel, is_from); } - else if (strcmp(defel->defname, "quote") == 0) + else if (copy_option_matches(defel->defname, "quote", &match_state)) { if (opts_out->quote) errorConflictingDefElem(defel, pstate); opts_out->quote = defGetString(defel); } - else if (strcmp(defel->defname, "escape") == 0) + else if (copy_option_matches(defel->defname, "escape", &match_state)) { if (opts_out->escape) errorConflictingDefElem(defel, pstate); opts_out->escape = defGetString(defel); } - else if (strcmp(defel->defname, "force_quote") == 0) + else if (copy_option_matches(defel->defname, "force_quote", &match_state)) { if (opts_out->force_quote || opts_out->force_quote_all) errorConflictingDefElem(defel, pstate); @@ -661,7 +687,7 @@ ProcessCopyOptions(ParseState *pstate, defel->defname), parser_errposition(pstate, defel->location))); } - else if (strcmp(defel->defname, "force_not_null") == 0) + else if (copy_option_matches(defel->defname, "force_not_null", &match_state)) { if (opts_out->force_notnull || opts_out->force_notnull_all) errorConflictingDefElem(defel, pstate); @@ -676,7 +702,7 @@ ProcessCopyOptions(ParseState *pstate, defel->defname), parser_errposition(pstate, defel->location))); } - else if (strcmp(defel->defname, "force_null") == 0) + else if (copy_option_matches(defel->defname, "force_null", &match_state)) { if (opts_out->force_null || opts_out->force_null_all) errorConflictingDefElem(defel, pstate); @@ -694,7 +720,11 @@ ProcessCopyOptions(ParseState *pstate, else if (strcmp(defel->defname, "convert_selectively") == 0) { /* - * Undocumented, not-accessible-from-SQL option: convert only the + * Undocumented, not-accessible-from-SQL option. Use plain strcmp + * rather than copy_option_matches so that it is not suggested in + * closest-match hints for unrecognized options. + * + * Convert only the * named columns to binary form, storing the rest as NULLs. It's * allowed for the column list to be NIL. */ @@ -710,7 +740,7 @@ ProcessCopyOptions(ParseState *pstate, defel->defname), parser_errposition(pstate, defel->location))); } - else if (strcmp(defel->defname, "encoding") == 0) + else if (copy_option_matches(defel->defname, "encoding", &match_state)) { if (opts_out->file_encoding >= 0) errorConflictingDefElem(defel, pstate); @@ -722,21 +752,21 @@ ProcessCopyOptions(ParseState *pstate, defel->defname), parser_errposition(pstate, defel->location))); } - else if (strcmp(defel->defname, "on_error") == 0) + else if (copy_option_matches(defel->defname, "on_error", &match_state)) { if (on_error_specified) errorConflictingDefElem(defel, pstate); on_error_specified = true; opts_out->on_error = defGetCopyOnErrorChoice(defel, pstate, is_from); } - else if (strcmp(defel->defname, "log_verbosity") == 0) + else if (copy_option_matches(defel->defname, "log_verbosity", &match_state)) { if (log_verbosity_specified) errorConflictingDefElem(defel, pstate); log_verbosity_specified = true; opts_out->log_verbosity = defGetCopyLogVerbosityChoice(defel, pstate); } - else if (strcmp(defel->defname, "reject_limit") == 0) + else if (copy_option_matches(defel->defname, "reject_limit", &match_state)) { if (reject_limit_specified) errorConflictingDefElem(defel, pstate); @@ -744,11 +774,17 @@ ProcessCopyOptions(ParseState *pstate, opts_out->reject_limit = defGetCopyRejectLimitOption(defel); } else + { + closest_match = getClosestMatch(&match_state); ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("option \"%s\" not recognized", defel->defname), + closest_match ? + errhint("Perhaps you meant \"%s\".", + closest_match) : 0, parser_errposition(pstate, defel->location))); + } } /* diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out index 3145b314e48..a463570b1b8 100644 --- a/src/test/regress/expected/copy2.out +++ b/src/test/regress/expected/copy2.out @@ -96,6 +96,7 @@ COPY x from stdin (on_error unsupported); ERROR: COPY ON_ERROR "unsupported" not recognized LINE 1: COPY x from stdin (on_error unsupported); ^ +HINT: Valid values are "ignore" and "stop". COPY x from stdin (format TEXT, force_quote(a)); ERROR: COPY FORCE_QUOTE requires CSV mode COPY x from stdin (format TEXT, force_quote *); @@ -128,6 +129,7 @@ COPY x from stdin (log_verbosity unsupported); ERROR: COPY LOG_VERBOSITY "unsupported" not recognized LINE 1: COPY x from stdin (log_verbosity unsupported); ^ +HINT: Valid values are "default", "silent", and "verbose". COPY x from stdin with (reject_limit 1); ERROR: COPY REJECT_LIMIT requires ON_ERROR to be set to IGNORE COPY x from stdin with (on_error ignore, reject_limit 0); @@ -144,6 +146,35 @@ COPY x from stdin with (header '2.5'); ERROR: header requires a Boolean value, an integer value greater than or equal to zero, or the string "match" COPY x to stdout with (header '2'); ERROR: cannot use multi-line header in COPY TO +-- test error hints for invalid COPY options +COPY x from stdin (foramt CSV); -- error, suggests "format" +ERROR: option "foramt" not recognized +LINE 1: COPY x from stdin (foramt CSV); + ^ +HINT: Perhaps you meant "format". +COPY x from stdin (convert_selectivel (a)); -- error, should NOT suggest convert_selectively +ERROR: option "convert_selectivel" not recognized +LINE 1: COPY x from stdin (convert_selectivel (a)); + ^ +COPY x from stdin (completely_invalid_option 'value'); -- error, no suggestion +ERROR: option "completely_invalid_option" not recognized +LINE 1: COPY x from stdin (completely_invalid_option 'value'); + ^ +COPY x from stdin (format cvs); -- error, lists valid formats +ERROR: COPY format "cvs" not recognized +LINE 1: COPY x from stdin (format cvs); + ^ +HINT: Valid formats are "binary", "csv", and "text". +COPY x from stdin (format CSV, on_error ignor); -- error, lists valid on_error values +ERROR: COPY ON_ERROR "ignor" not recognized +LINE 1: COPY x from stdin (format CSV, on_error ignor); + ^ +HINT: Valid values are "ignore" and "stop". +COPY x from stdin (format CSV, log_verbosity verbos); -- error, lists valid log_verbosity values +ERROR: COPY LOG_VERBOSITY "verbos" not recognized +LINE 1: COPY x from stdin (format CSV, log_verbosity verbos); + ^ +HINT: Valid values are "default", "silent", and "verbose". -- too many columns in column list: should fail COPY x (a, b, c, d, e, d, c) from stdin; ERROR: column "d" specified more than once diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql index 66435167500..ce19406f759 100644 --- a/src/test/regress/sql/copy2.sql +++ b/src/test/regress/sql/copy2.sql @@ -97,6 +97,14 @@ COPY x to stdout with (header '-1'); COPY x from stdin with (header '2.5'); COPY x to stdout with (header '2'); +-- test error hints for invalid COPY options +COPY x from stdin (foramt CSV); -- error, suggests "format" +COPY x from stdin (convert_selectivel (a)); -- error, should NOT suggest convert_selectively +COPY x from stdin (completely_invalid_option 'value'); -- error, no suggestion +COPY x from stdin (format cvs); -- error, lists valid formats +COPY x from stdin (format CSV, on_error ignor); -- error, lists valid on_error values +COPY x from stdin (format CSV, log_verbosity verbos); -- error, lists valid log_verbosity values + -- too many columns in column list: should fail COPY x (a, b, c, d, e, d, c) from stdin; -- 2.50.1 (Apple Git-155)