From c923fce80ee0d6cb8bc37f0dd893abc069a01c26 Mon Sep 17 00:00:00 2001 From: Shinya Sugamoto Date: Sat, 22 Nov 2025 22:03:21 +0900 Subject: [PATCH] Refactor COPY option handling and improve error hints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Restructure ProcessCopyOptions() so that each option has its own processor function. This makes option handling more modular and easier to extend. - Improve error messages by adding hints that show valid values when users specify unrecognized option values. --- contrib/file_fdw/expected/file_fdw.out | 3 + src/backend/commands/copy.c | 597 ++++++++++++++----------- src/test/regress/expected/copy2.out | 27 ++ src/test/regress/sql/copy2.sql | 7 + 4 files changed, 375 insertions(+), 259 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..804810c2272 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 @@ -365,133 +366,328 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, } /* - * Extract the CopyFormatOptions.header_line value from a DefElem. - * - * Parses the HEADER option for COPY, which can be a boolean, a non-negative - * integer (number of lines to skip), or the special value "match". + * Context structure for COPY option processing. */ -static int -defGetCopyHeaderOption(DefElem *def, bool is_from) +typedef struct CopyOptionContext { - /* - * If no parameter value given, assume "true" is meant. - */ - if (def->arg == NULL) - return COPY_HEADER_TRUE; + ParseState *pstate; + CopyFormatOptions *opts_out; + bool is_from; + bool format_specified; + bool freeze_specified; + bool header_specified; + bool on_error_specified; + bool log_verbosity_specified; + bool reject_limit_specified; +} CopyOptionContext; + +/* Individual option processors */ + +static void +processOptionFormat(CopyOptionContext *ctx, DefElem *defel) +{ + char *fmt = defGetString(defel); + + if (ctx->format_specified) + errorConflictingDefElem(defel, ctx->pstate); + ctx->format_specified = true; + + if (strcmp(fmt, "text") == 0) + /* default format */ ; + else if (strcmp(fmt, "csv") == 0) + ctx->opts_out->csv_mode = true; + else if (strcmp(fmt, "binary") == 0) + ctx->opts_out->binary = true; + else + 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(ctx->pstate, defel->location))); +} - /* - * Allow 0, 1, "true", "false", "on", "off", a non-negative integer, or - * "match". - */ - switch (nodeTag(def->arg)) +static void +processOptionFreeze(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->freeze_specified) + errorConflictingDefElem(defel, ctx->pstate); + ctx->freeze_specified = true; + ctx->opts_out->freeze = defGetBoolean(defel); +} + +static void +processOptionDelimiter(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->delim) + errorConflictingDefElem(defel, ctx->pstate); + ctx->opts_out->delim = defGetString(defel); +} + +static void +processOptionNull(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->null_print) + errorConflictingDefElem(defel, ctx->pstate); + ctx->opts_out->null_print = defGetString(defel); +} + +static void +processOptionDefault(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->default_print) + errorConflictingDefElem(defel, ctx->pstate); + ctx->opts_out->default_print = defGetString(defel); +} + +static void +processOptionHeader(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->header_specified) + errorConflictingDefElem(defel, ctx->pstate); + ctx->header_specified = true; + + if (defel->arg == NULL) + { + ctx->opts_out->header_line = COPY_HEADER_TRUE; + return; + } + switch (nodeTag(defel->arg)) { case T_Integer: { - int ival = intVal(def->arg); + int ival = intVal(defel->arg); if (ival < 0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("a negative integer value cannot be " - "specified for %s", def->defname))); + "specified for %s", defel->defname))); - if (!is_from && ival > 1) + if (!ctx->is_from && ival > 1) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot use multi-line header in COPY TO"))); - return ival; + ctx->opts_out->header_line = ival; + return; } - break; default: { - char *sval = defGetString(def); + char *sval = defGetString(defel); - /* - * The set of strings accepted here should match up with the - * grammar's opt_boolean_or_string production. - */ - if (pg_strcasecmp(sval, "true") == 0) - return COPY_HEADER_TRUE; - if (pg_strcasecmp(sval, "false") == 0) - return COPY_HEADER_FALSE; - if (pg_strcasecmp(sval, "on") == 0) - return COPY_HEADER_TRUE; - if (pg_strcasecmp(sval, "off") == 0) - return COPY_HEADER_FALSE; + if (pg_strcasecmp(sval, "true") == 0 || + pg_strcasecmp(sval, "on") == 0) + { + ctx->opts_out->header_line = COPY_HEADER_TRUE; + return; + } + if (pg_strcasecmp(sval, "false") == 0 || + pg_strcasecmp(sval, "off") == 0) + { + ctx->opts_out->header_line = COPY_HEADER_FALSE; + return; + } if (pg_strcasecmp(sval, "match") == 0) { - if (!is_from) + if (!ctx->is_from) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot use \"%s\" with HEADER in COPY TO", sval))); - return COPY_HEADER_MATCH; + ctx->opts_out->header_line = COPY_HEADER_MATCH; + return; } } break; } + ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("%s requires a Boolean value, a non-negative integer, " "or the string \"match\"", - def->defname))); - return COPY_HEADER_FALSE; /* keep compiler quiet */ + defel->defname))); } -/* - * Extract a CopyOnErrorChoice value from a DefElem. - */ -static CopyOnErrorChoice -defGetCopyOnErrorChoice(DefElem *def, ParseState *pstate, bool is_from) +static void +processOptionQuote(CopyOptionContext *ctx, DefElem *defel) { - char *sval = defGetString(def); + if (ctx->opts_out->quote) + errorConflictingDefElem(defel, ctx->pstate); + ctx->opts_out->quote = defGetString(defel); +} - if (!is_from) +static void +processOptionEscape(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->escape) + errorConflictingDefElem(defel, ctx->pstate); + ctx->opts_out->escape = defGetString(defel); +} + +static void +processOptionForceQuote(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->force_quote || ctx->opts_out->force_quote_all) + errorConflictingDefElem(defel, ctx->pstate); + + if (defel->arg && IsA(defel->arg, A_Star)) + ctx->opts_out->force_quote_all = true; + else if (defel->arg && IsA(defel->arg, List)) + ctx->opts_out->force_quote = castNode(List, defel->arg); + else ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR, - second %s is a COPY with direction, e.g. COPY TO */ - errmsg("COPY %s cannot be used with %s", "ON_ERROR", "COPY TO"), - parser_errposition(pstate, def->location))); + errmsg("argument to option \"%s\" must be a list of column names", + defel->defname), + parser_errposition(ctx->pstate, defel->location))); +} + +static void +processOptionForceNotNull(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->force_notnull || ctx->opts_out->force_notnull_all) + errorConflictingDefElem(defel, ctx->pstate); + + if (defel->arg && IsA(defel->arg, A_Star)) + ctx->opts_out->force_notnull_all = true; + else if (defel->arg && IsA(defel->arg, List)) + ctx->opts_out->force_notnull = castNode(List, defel->arg); + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("argument to option \"%s\" must be a list of column names", + defel->defname), + parser_errposition(ctx->pstate, defel->location))); +} + +static void +processOptionForceNull(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->force_null || ctx->opts_out->force_null_all) + errorConflictingDefElem(defel, ctx->pstate); + + if (defel->arg && IsA(defel->arg, A_Star)) + ctx->opts_out->force_null_all = true; + else if (defel->arg && IsA(defel->arg, List)) + ctx->opts_out->force_null = castNode(List, defel->arg); + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("argument to option \"%s\" must be a list of column names", + defel->defname), + parser_errposition(ctx->pstate, defel->location))); +} +static void +processOptionConvertSelectively(CopyOptionContext *ctx, DefElem *defel) +{ /* - * Allow "stop", or "ignore" values. + * 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. */ + if (ctx->opts_out->convert_selectively) + errorConflictingDefElem(defel, ctx->pstate); + ctx->opts_out->convert_selectively = true; + + if (defel->arg == NULL || IsA(defel->arg, List)) + ctx->opts_out->convert_select = castNode(List, defel->arg); + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("argument to option \"%s\" must be a list of column names", + defel->defname), + parser_errposition(ctx->pstate, defel->location))); +} + +static void +processOptionEncoding(CopyOptionContext *ctx, DefElem *defel) +{ + if (ctx->opts_out->file_encoding >= 0) + errorConflictingDefElem(defel, ctx->pstate); + + ctx->opts_out->file_encoding = pg_char_to_encoding(defGetString(defel)); + if (ctx->opts_out->file_encoding < 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("argument to option \"%s\" must be a valid encoding name", + defel->defname), + parser_errposition(ctx->pstate, defel->location))); +} + +/* Process ON_ERROR option: "stop" or "ignore" */ +static void +processOptionOnError(CopyOptionContext *ctx, DefElem *defel) +{ + char *sval; + + if (ctx->on_error_specified) + errorConflictingDefElem(defel, ctx->pstate); + ctx->on_error_specified = true; + + if (!ctx->is_from) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("COPY %s cannot be used with %s", "ON_ERROR", "COPY TO"), + parser_errposition(ctx->pstate, defel->location))); + + sval = defGetString(defel); if (pg_strcasecmp(sval, "stop") == 0) - return COPY_ON_ERROR_STOP; - if (pg_strcasecmp(sval, "ignore") == 0) - return COPY_ON_ERROR_IGNORE; + ctx->opts_out->on_error = COPY_ON_ERROR_STOP; + else if (pg_strcasecmp(sval, "ignore") == 0) + ctx->opts_out->on_error = COPY_ON_ERROR_IGNORE; + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("COPY %s \"%s\" not recognized", "ON_ERROR", sval), + errhint("Valid values are \"%s\" and \"%s\".", "ignore", "stop"), + parser_errposition(ctx->pstate, defel->location))); +} - ereport(ERROR, - (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), - parser_errposition(pstate, def->location))); - return COPY_ON_ERROR_STOP; /* keep compiler quiet */ +static void +processOptionLogVerbosity(CopyOptionContext *ctx, DefElem *defel) +{ + char *sval; + + if (ctx->log_verbosity_specified) + errorConflictingDefElem(defel, ctx->pstate); + ctx->log_verbosity_specified = true; + + sval = defGetString(defel); + if (pg_strcasecmp(sval, "silent") == 0) + ctx->opts_out->log_verbosity = COPY_LOG_VERBOSITY_SILENT; + else if (pg_strcasecmp(sval, "default") == 0) + ctx->opts_out->log_verbosity = COPY_LOG_VERBOSITY_DEFAULT; + else if (pg_strcasecmp(sval, "verbose") == 0) + ctx->opts_out->log_verbosity = COPY_LOG_VERBOSITY_VERBOSE; + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("COPY %s \"%s\" not recognized", "LOG_VERBOSITY", sval), + errhint("Valid values are \"%s\", \"%s\", and \"%s\".", + "default", "silent", "verbose"), + parser_errposition(ctx->pstate, defel->location))); } -/* - * Extract REJECT_LIMIT value from a DefElem. - * - * REJECT_LIMIT can be specified in two ways: as an int64 for the COPY command - * option or as a single-quoted string for the foreign table option using - * file_fdw. Therefore this function needs to handle both formats. - */ -static int64 -defGetCopyRejectLimitOption(DefElem *def) +static void +processOptionRejectLimit(CopyOptionContext *ctx, DefElem *defel) { int64 reject_limit; - if (def->arg == NULL) + if (ctx->reject_limit_specified) + errorConflictingDefElem(defel, ctx->pstate); + ctx->reject_limit_specified = true; + + if (defel->arg == NULL) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("%s requires a numeric value", - def->defname))); - else if (nodeTag(def->arg) == T_String) - reject_limit = pg_strtoint64(strVal(def->arg)); + errmsg("%s requires a numeric value", defel->defname))); + + if (nodeTag(defel->arg) == T_String) + reject_limit = pg_strtoint64(strVal(defel->arg)); else - reject_limit = defGetInt64(def); + reject_limit = defGetInt64(defel); if (reject_limit <= 0) ereport(ERROR, @@ -499,35 +695,45 @@ defGetCopyRejectLimitOption(DefElem *def) errmsg("REJECT_LIMIT (%" PRId64 ") must be greater than zero", reject_limit))); - return reject_limit; + ctx->opts_out->reject_limit = reject_limit; } /* - * Extract a CopyLogVerbosityChoice value from a DefElem. + * Definition of a COPY option. */ -static CopyLogVerbosityChoice -defGetCopyLogVerbosityChoice(DefElem *def, ParseState *pstate) +typedef struct CopyOptionDef { - char *sval; + const char *name; + void (*processor) (CopyOptionContext *ctx, DefElem *defel); + bool suggest_in_hints; +} CopyOptionDef; - /* - * Allow "silent", "default", or "verbose" values. - */ - sval = defGetString(def); - if (pg_strcasecmp(sval, "silent") == 0) - return COPY_LOG_VERBOSITY_SILENT; - if (pg_strcasecmp(sval, "default") == 0) - return COPY_LOG_VERBOSITY_DEFAULT; - if (pg_strcasecmp(sval, "verbose") == 0) - return COPY_LOG_VERBOSITY_VERBOSE; - - ereport(ERROR, - (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), - parser_errposition(pstate, def->location))); - return COPY_LOG_VERBOSITY_DEFAULT; /* keep compiler quiet */ -} +/* + * Table of all valid COPY options. + * + * This is the single source of truth for valid COPY options. When adding a + * new option, add an entry here and implement its processor function above. + */ +static const CopyOptionDef copy_options[] = +{ + {"default", processOptionDefault, true}, + {"delimiter", processOptionDelimiter, true}, + {"encoding", processOptionEncoding, true}, + {"escape", processOptionEscape, true}, + {"force_not_null", processOptionForceNotNull, true}, + {"force_null", processOptionForceNull, true}, + {"force_quote", processOptionForceQuote, true}, + {"format", processOptionFormat, true}, + {"freeze", processOptionFreeze, true}, + {"header", processOptionHeader, true}, + {"log_verbosity", processOptionLogVerbosity, true}, + {"null", processOptionNull, true}, + {"on_error", processOptionOnError, true}, + {"quote", processOptionQuote, true}, + {"reject_limit", processOptionRejectLimit, true}, + {"convert_selectively", processOptionConvertSelectively, false}, + {NULL, NULL, false} +}; /* * Process the statement option list for COPY. @@ -551,12 +757,7 @@ ProcessCopyOptions(ParseState *pstate, bool is_from, List *options) { - bool format_specified = false; - bool freeze_specified = false; - bool header_specified = false; - bool on_error_specified = false; - bool log_verbosity_specified = false; - bool reject_limit_specified = false; + CopyOptionContext ctx; ListCell *option; /* Support external use for option sanity checking */ @@ -565,177 +766,55 @@ ProcessCopyOptions(ParseState *pstate, opts_out->file_encoding = -1; + /* Initialize option processing context */ + memset(&ctx, 0, sizeof(ctx)); + ctx.pstate = pstate; + ctx.opts_out = opts_out; + ctx.is_from = is_from; + /* Extract options from the statement node tree */ foreach(option, options) { DefElem *defel = lfirst_node(DefElem, option); + bool found = false; - if (strcmp(defel->defname, "format") == 0) - { - char *fmt = defGetString(defel); - - if (format_specified) - errorConflictingDefElem(defel, pstate); - format_specified = true; - if (strcmp(fmt, "text") == 0) - /* default format */ ; - else if (strcmp(fmt, "csv") == 0) - opts_out->csv_mode = true; - else if (strcmp(fmt, "binary") == 0) - opts_out->binary = true; - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("COPY format \"%s\" not recognized", fmt), - parser_errposition(pstate, defel->location))); - } - else if (strcmp(defel->defname, "freeze") == 0) - { - if (freeze_specified) - errorConflictingDefElem(defel, pstate); - freeze_specified = true; - opts_out->freeze = defGetBoolean(defel); - } - else if (strcmp(defel->defname, "delimiter") == 0) - { - if (opts_out->delim) - errorConflictingDefElem(defel, pstate); - opts_out->delim = defGetString(defel); - } - else if (strcmp(defel->defname, "null") == 0) - { - if (opts_out->null_print) - errorConflictingDefElem(defel, pstate); - opts_out->null_print = defGetString(defel); - } - else if (strcmp(defel->defname, "default") == 0) - { - if (opts_out->default_print) - errorConflictingDefElem(defel, pstate); - opts_out->default_print = defGetString(defel); - } - else if (strcmp(defel->defname, "header") == 0) - { - if (header_specified) - errorConflictingDefElem(defel, pstate); - header_specified = true; - opts_out->header_line = defGetCopyHeaderOption(defel, is_from); - } - else if (strcmp(defel->defname, "quote") == 0) - { - if (opts_out->quote) - errorConflictingDefElem(defel, pstate); - opts_out->quote = defGetString(defel); - } - else if (strcmp(defel->defname, "escape") == 0) - { - if (opts_out->escape) - errorConflictingDefElem(defel, pstate); - opts_out->escape = defGetString(defel); - } - else if (strcmp(defel->defname, "force_quote") == 0) - { - if (opts_out->force_quote || opts_out->force_quote_all) - errorConflictingDefElem(defel, pstate); - if (defel->arg && IsA(defel->arg, A_Star)) - opts_out->force_quote_all = true; - else if (defel->arg && IsA(defel->arg, List)) - opts_out->force_quote = castNode(List, defel->arg); - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("argument to option \"%s\" must be a list of column names", - defel->defname), - parser_errposition(pstate, defel->location))); - } - else if (strcmp(defel->defname, "force_not_null") == 0) - { - if (opts_out->force_notnull || opts_out->force_notnull_all) - errorConflictingDefElem(defel, pstate); - if (defel->arg && IsA(defel->arg, A_Star)) - opts_out->force_notnull_all = true; - else if (defel->arg && IsA(defel->arg, List)) - opts_out->force_notnull = castNode(List, defel->arg); - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("argument to option \"%s\" must be a list of column names", - defel->defname), - parser_errposition(pstate, defel->location))); - } - else if (strcmp(defel->defname, "force_null") == 0) + /* Look up the option in copy_options table */ + for (int i = 0; copy_options[i].name != NULL; i++) { - if (opts_out->force_null || opts_out->force_null_all) - errorConflictingDefElem(defel, pstate); - if (defel->arg && IsA(defel->arg, A_Star)) - opts_out->force_null_all = true; - else if (defel->arg && IsA(defel->arg, List)) - opts_out->force_null = castNode(List, defel->arg); - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("argument to option \"%s\" must be a list of column names", - defel->defname), - parser_errposition(pstate, defel->location))); + if (strcmp(copy_options[i].name, defel->defname) == 0) + { + copy_options[i].processor(&ctx, defel); + found = true; + break; + } } - else if (strcmp(defel->defname, "convert_selectively") == 0) + + if (!found) { /* - * 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. + * When an unknown option is specified, complain about it. + * Provide a hint with a valid option that looks similar, if there + * is one. */ - if (opts_out->convert_selectively) - errorConflictingDefElem(defel, pstate); - opts_out->convert_selectively = true; - if (defel->arg == NULL || IsA(defel->arg, List)) - opts_out->convert_select = castNode(List, defel->arg); - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("argument to option \"%s\" must be a list of column names", - defel->defname), - parser_errposition(pstate, defel->location))); - } - else if (strcmp(defel->defname, "encoding") == 0) - { - if (opts_out->file_encoding >= 0) - errorConflictingDefElem(defel, pstate); - opts_out->file_encoding = pg_char_to_encoding(defGetString(defel)); - if (opts_out->file_encoding < 0) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("argument to option \"%s\" must be a valid encoding name", - defel->defname), - parser_errposition(pstate, defel->location))); - } - else if (strcmp(defel->defname, "on_error") == 0) - { - 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) - { - 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) - { - if (reject_limit_specified) - errorConflictingDefElem(defel, pstate); - reject_limit_specified = true; - opts_out->reject_limit = defGetCopyRejectLimitOption(defel); - } - else + ClosestMatchState match_state; + const char *closest_match; + + initClosestMatch(&match_state, defel->defname, 4); + for (int i = 0; copy_options[i].name != NULL; i++) + { + if (copy_options[i].suggest_in_hints) + updateClosestMatch(&match_state, copy_options[i].name); + } + + 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)