From 773c3d5cfddfc3c9551a8e0d37d58808b60dfdcc Mon Sep 17 00:00:00 2001 From: Shinya Sugamoto Date: Sat, 22 Nov 2025 22:03:21 +0900 Subject: [PATCH] Add error hints for invalid COPY options Add error hints for invalid COPY options. Use ClosestMatch when many valid options exist (option names), or list all values when few exist (option values like format, on_error). Also update the misleading comment for the convert_selectively option. The comment previously stated it was "not-accessible-from-SQL", but actually it has been accessible from SQL due to PostgreSQL's generic option parser. The updated comment clarifies that while technically accessible, it's intended for internal use by file_fdw and not recommended for end-user use due to potential data loss. --- src/backend/commands/copy.c | 46 +++++++++++++++++++++++++++-- src/test/regress/expected/copy2.out | 27 +++++++++++++++++ src/test/regress/sql/copy2.sql | 7 +++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 28e878c3688..14bb1756746 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -38,6 +38,7 @@ #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/rls.h" +#include "utils/varlena.h" /* * DoCopy executes the SQL COPY statement @@ -467,6 +468,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 */ } @@ -525,6 +527,7 @@ 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 */ } @@ -587,6 +590,7 @@ 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) @@ -681,9 +685,9 @@ ProcessCopyOptions(ParseState *pstate, else if (strcmp(defel->defname, "convert_selectively") == 0) { /* - * Undocumented, not-accessible-from-SQL option: convert only the - * named columns to binary form, storing the rest as NULLs. It's - * allowed for the column list to be NIL. + * Undocumented option for file_fdw internal use. Converts only + * the listed columns; all others become NULL. The column list + * can be NIL. */ if (opts_out->convert_selectively) errorConflictingDefElem(defel, pstate); @@ -731,11 +735,47 @@ ProcessCopyOptions(ParseState *pstate, opts_out->reject_limit = defGetCopyRejectLimitOption(defel); } else + { + ClosestMatchState match_state; + const char *closest_match; + int i; + /* Valid COPY option names for closest match suggestions */ + static const char *const valid_copy_options[] = { + "default", + "delimiter", + "encoding", + "escape", + "force_not_null", + "force_null", + "force_quote", + "format", + "freeze", + "header", + "log_verbosity", + "null", + "on_error", + "quote", + "reject_limit", + NULL + }; + + /* + * Unknown option specified, complain about it. Provide a hint + * with a valid option that looks similar, if there is one. + */ + initClosestMatch(&match_state, defel->defname, 4); + for (i = 0; valid_copy_options[i] != NULL; i++) + updateClosestMatch(&match_state, valid_copy_options[i]); + + 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 f3fdce23459..a4b4e33248f 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); @@ -138,6 +140,31 @@ COPY x from stdin with (header 2.5); ERROR: header requires a Boolean value, a non-negative integer, 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 (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 cef45868db5..b8c9d2439ef 100644 --- a/src/test/regress/sql/copy2.sql +++ b/src/test/regress/sql/copy2.sql @@ -94,6 +94,13 @@ COPY x from stdin 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 (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)