From b7dd821ed89dcd2846169f46ff983e051b75ef5e Mon Sep 17 00:00:00 2001 From: Shinya Sugamoto Date: Fri, 5 Dec 2025 00:16:55 +0900 Subject: [PATCH] Add option name validation and hints for COPY command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add validation for COPY option names with helpful error messages: - Suggest similar option names for typos (e.g., "foramt" -> "format") - Show valid values for format, on_error, and log_verbosity options - Internal options like convert_selectively are excluded from suggestions --- contrib/file_fdw/expected/file_fdw.out | 3 + src/backend/commands/copy.c | 91 ++++++++++++++++++++++++-- src/test/regress/expected/copy2.out | 31 +++++++++ src/test/regress/sql/copy2.sql | 8 +++ 4 files changed, 128 insertions(+), 5 deletions(-) diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out index 5121e27dce5..bf953c62a49 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 28e878c3688..1d48035e3e0 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -35,9 +35,83 @@ #include "parser/parse_relation.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/elog.h" #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/rls.h" +#include "utils/varlena.h" + +/* + * List of valid COPY option names, used for validation and error hints. + * + * Options with suggest_in_hints=true will be suggested in error hints when a + * user specifies an unrecognized option name. Internal/undocumented options + * should set this to false. + */ +typedef struct CopyOptionDef +{ + const char *name; + bool suggest_in_hints; +} CopyOptionDef; + +static const CopyOptionDef valid_copy_options[] = { + {"default", true}, + {"delimiter", true}, + {"encoding", true}, + {"escape", true}, + {"force_not_null", true}, + {"force_null", true}, + {"force_quote", true}, + {"format", true}, + {"freeze", true}, + {"header", true}, + {"log_verbosity", true}, + {"null", true}, + {"on_error", true}, + {"quote", true}, + {"reject_limit", true}, + {"convert_selectively", false}, /* internal, undocumented */ + {NULL, false} +}; + +/* + * Validate a COPY option name. + * + * If the option name is not recognized, report an error with a hint + * suggesting a similar valid option name if one exists. + */ +static void +validate_copy_option(const char *option, ParseState *pstate, int location) +{ + ClosestMatchState match_state; + const char *closest_match; + + /* Check if it's a valid option */ + for (int i = 0; valid_copy_options[i].name != NULL; i++) + { + if (strcmp(valid_copy_options[i].name, option) == 0) + return; /* valid option */ + } + + /* + * Unknown option - build an error with a hint suggesting a similar option + * name if there is one. + */ + initClosestMatch(&match_state, option, 4); + for (int i = 0; valid_copy_options[i].name != NULL; i++) + { + if (valid_copy_options[i].suggest_in_hints) + updateClosestMatch(&match_state, valid_copy_options[i].name); + } + closest_match = getClosestMatch(&match_state); + + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("option \"%s\" not recognized", option), + closest_match ? + errhint("Perhaps you meant \"%s\".", closest_match) : 0, + parser_errposition(pstate, location))); +} /* * DoCopy executes the SQL COPY statement @@ -467,6 +541,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 +600,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 */ } @@ -570,6 +647,9 @@ ProcessCopyOptions(ParseState *pstate, { DefElem *defel = lfirst_node(DefElem, option); + /* Validate the option name; errors out if unrecognized */ + validate_copy_option(defel->defname, pstate, defel->location); + if (strcmp(defel->defname, "format") == 0) { char *fmt = defGetString(defel); @@ -587,6 +667,8 @@ 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) @@ -731,11 +813,10 @@ ProcessCopyOptions(ParseState *pstate, opts_out->reject_limit = defGetCopyRejectLimitOption(defel); } else - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("option \"%s\" not recognized", - defel->defname), - parser_errposition(pstate, defel->location))); + ereport(FATAL, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg("unhandled COPY option \"%s\"", defel->defname))); + } /* diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out index f3fdce23459..7480903023f 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,35 @@ 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 (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 cef45868db5..39d8e3fd992 100644 --- a/src/test/regress/sql/copy2.sql +++ b/src/test/regress/sql/copy2.sql @@ -94,6 +94,14 @@ 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 (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)