From 33523fab166648c00dda7de4554b7b94a03ff93a Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Wed, 6 May 2026 11:54:47 +0800 Subject: [PATCH v1] COPY: validate option presence rather than option values Several COPY options were validated using their parsed value rather than whether the user had specified the option at all. That allowed statements such as `FORCE_ARRAY FALSE`, `HEADER FALSE`, and `FREEZE FALSE` to be accepted in modes where those options are documented as illegal. Also, some string-valued options were parsed eagerly, so malformed or incomplete arguments could mask a clearer mode error. For example, `COPY ... (FORMAT JSON, ESCAPE)` reported `escape requires a parameter` instead of rejecting `ESCAPE` outright for non-CSV modes. Fix this by: - validating `FORCE_ARRAY`, `HEADER`, and `FREEZE` by option presence - deferring parsing of `DELIMITER`, `NULL`, `DEFAULT`, `QUOTE`, and `ESCAPE` until after format checks, so unsupported-mode errors win when appropriate Add regression coverage for the new failure cases. This makes `COPY` option validation consistent with the documented restrictions for JSON, binary, CSV, `COPY TO`, and `COPY FROM`. Author: Chao Li Reviewed-by: Discussion: https://postgr.es/m/ --- src/backend/commands/copy.c | 82 ++++++++++++++++++++---------- src/test/regress/expected/copy.out | 38 ++++++++++++++ src/test/regress/sql/copy.sql | 19 +++++++ 3 files changed, 113 insertions(+), 26 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 003b70852bb..9b44dfebf83 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -590,6 +590,11 @@ ProcessCopyOptions(ParseState *pstate, bool log_verbosity_specified = false; bool reject_limit_specified = false; bool force_array_specified = false; + DefElem *delim_el = NULL; + DefElem *null_el = NULL; + DefElem *default_el = NULL; + DefElem *quote_el = NULL; + DefElem *escape_el = NULL; ListCell *option; /* Support external use for option sanity checking */ @@ -635,21 +640,21 @@ ProcessCopyOptions(ParseState *pstate, } else if (strcmp(defel->defname, "delimiter") == 0) { - if (opts_out->delim) + if (delim_el) errorConflictingDefElem(defel, pstate); - opts_out->delim = defGetString(defel); + delim_el = defel; } else if (strcmp(defel->defname, "null") == 0) { - if (opts_out->null_print) + if (null_el) errorConflictingDefElem(defel, pstate); - opts_out->null_print = defGetString(defel); + null_el = defel; } else if (strcmp(defel->defname, "default") == 0) { - if (opts_out->default_print) + if (default_el) errorConflictingDefElem(defel, pstate); - opts_out->default_print = defGetString(defel); + default_el = defel; } else if (strcmp(defel->defname, "header") == 0) { @@ -660,15 +665,15 @@ ProcessCopyOptions(ParseState *pstate, } else if (strcmp(defel->defname, "quote") == 0) { - if (opts_out->quote) + if (quote_el) errorConflictingDefElem(defel, pstate); - opts_out->quote = defGetString(defel); + quote_el = defel; } else if (strcmp(defel->defname, "escape") == 0) { - if (opts_out->escape) + if (escape_el) errorConflictingDefElem(defel, pstate); - opts_out->escape = defGetString(defel); + escape_el = defel; } else if (strcmp(defel->defname, "force_quote") == 0) { @@ -783,10 +788,9 @@ ProcessCopyOptions(ParseState *pstate, } /* - * Check for incompatible options (must do these three before inserting - * defaults) + * Check for incompatible options before inserting defaults. */ - if (opts_out->delim && + if (delim_el && (opts_out->format == COPY_FORMAT_BINARY || opts_out->format == COPY_FORMAT_JSON)) ereport(ERROR, @@ -795,7 +799,7 @@ ProcessCopyOptions(ParseState *pstate, ? errmsg("cannot specify %s in BINARY mode", "DELIMITER") : errmsg("cannot specify %s in JSON mode", "DELIMITER")); - if (opts_out->null_print && + if (null_el && (opts_out->format == COPY_FORMAT_BINARY || opts_out->format == COPY_FORMAT_JSON)) ereport(ERROR, @@ -804,7 +808,7 @@ ProcessCopyOptions(ParseState *pstate, ? errmsg("cannot specify %s in BINARY mode", "NULL") : errmsg("cannot specify %s in JSON mode", "NULL")); - if (opts_out->default_print && + if (default_el && (opts_out->format == COPY_FORMAT_BINARY || opts_out->format == COPY_FORMAT_JSON)) ereport(ERROR, @@ -813,7 +817,39 @@ ProcessCopyOptions(ParseState *pstate, ? errmsg("cannot specify %s in BINARY mode", "DEFAULT") : errmsg("cannot specify %s in JSON mode", "DEFAULT")); - /* Set defaults for omitted options */ + if (opts_out->format != COPY_FORMAT_CSV && quote_el != NULL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */ + errmsg("COPY %s requires CSV mode", "QUOTE"))); + + if (opts_out->format != COPY_FORMAT_CSV && escape_el != NULL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */ + errmsg("COPY %s requires CSV mode", "ESCAPE"))); + + /* + * Extract option values. + */ + if (delim_el != NULL) + opts_out->delim = defGetString(delim_el); + + if (null_el != NULL) + opts_out->null_print = defGetString(null_el); + + if (default_el != NULL) + opts_out->default_print = defGetString(default_el); + + if (quote_el != NULL) + opts_out->quote = defGetString(quote_el); + + if (escape_el != NULL) + opts_out->escape = defGetString(escape_el); + + /* + * Set defaults for omitted options. + */ if (!opts_out->delim) opts_out->delim = (opts_out->format == COPY_FORMAT_CSV) ? "," : "\t"; @@ -877,7 +913,7 @@ ProcessCopyOptions(ParseState *pstate, errmsg("COPY delimiter cannot be \"%s\"", opts_out->delim))); /* Check header */ - if (opts_out->header_line != COPY_HEADER_FALSE && + if (header_specified && (opts_out->format == COPY_FORMAT_BINARY || opts_out->format == COPY_FORMAT_JSON)) ereport(ERROR, @@ -888,12 +924,6 @@ ProcessCopyOptions(ParseState *pstate, : errmsg("cannot specify %s in JSON mode", "HEADER")); /* Check quote */ - if (opts_out->format != COPY_FORMAT_CSV && opts_out->quote != NULL) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */ - errmsg("COPY %s requires CSV mode", "QUOTE"))); - if (opts_out->format == COPY_FORMAT_CSV && strlen(opts_out->quote) != 1) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -981,7 +1011,7 @@ ProcessCopyOptions(ParseState *pstate, "NULL"))); /* Check freeze */ - if (opts_out->freeze && !is_from) + if (freeze_specified && !is_from) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR, @@ -995,12 +1025,12 @@ ProcessCopyOptions(ParseState *pstate, errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("COPY %s is not supported for %s", "FORMAT JSON", "COPY FROM")); - if (opts_out->format != COPY_FORMAT_JSON && opts_out->force_array) + if (opts_out->format != COPY_FORMAT_JSON && force_array_specified) ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("COPY %s can only be used with JSON mode", "FORCE_ARRAY")); - if (opts_out->default_print) + if (default_el != NULL) { if (!is_from) ereport(ERROR, diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out index 1714faab39c..caa3a0708f5 100644 --- a/src/test/regress/expected/copy.out +++ b/src/test/regress/expected/copy.out @@ -111,18 +111,48 @@ copy (select * from copytest) to stdout (format json); -- all of the following should yield error copy copytest to stdout (format json, delimiter '|'); ERROR: cannot specify DELIMITER in JSON mode +copy copytest to stdout (format json, delimiter); +ERROR: cannot specify DELIMITER in JSON mode copy copytest to stdout (format json, null '\N'); ERROR: cannot specify NULL in JSON mode +copy copytest to stdout (format json, null); +ERROR: cannot specify NULL in JSON mode copy copytest to stdout (format json, default '|'); ERROR: cannot specify DEFAULT in JSON mode +copy copytest to stdout (format json, default); +ERROR: cannot specify DEFAULT in JSON mode copy copytest to stdout (format json, header); ERROR: cannot specify HEADER in JSON mode copy copytest to stdout (format json, header 1); ERROR: cannot specify HEADER in JSON mode +copy copytest to stdout (format json, header false); +ERROR: cannot specify HEADER in JSON mode +copy copytest to stdout (format json, header 0); +ERROR: cannot specify HEADER in JSON mode +copy copytest to stdout (format binary, header false); +ERROR: cannot specify HEADER in BINARY mode +copy copytest to stdout (format json, escape); +ERROR: COPY ESCAPE requires CSV mode +copy copytest to stdout (format json, quote); +ERROR: COPY QUOTE requires CSV mode copy copytest to stdout (format json, quote '"'); ERROR: COPY QUOTE requires CSV mode copy copytest to stdout (format json, escape '"'); ERROR: COPY ESCAPE requires CSV mode +copy copytest to stdout (format json, quote); +ERROR: COPY QUOTE requires CSV mode +copy copytest to stdout (format json, escape); +ERROR: COPY ESCAPE requires CSV mode +copy copytest to stdout (delimiter); +ERROR: delimiter requires a parameter +copy copytest to stdout (null); +ERROR: null requires a parameter +copy copytest from stdin (default); +ERROR: default requires a parameter +copy copytest to stdout (format csv, quote); +ERROR: quote requires a parameter +copy copytest to stdout (format csv, escape); +ERROR: escape requires a parameter copy copytest to stdout (format json, force_quote *); ERROR: COPY FORCE_QUOTE requires CSV mode copy copytest to stdout (format json, force_not_null *); @@ -137,6 +167,8 @@ copy copytest to stdout (format json, reject_limit 1); ERROR: COPY REJECT_LIMIT requires ON_ERROR to be set to IGNORE copy copytest from stdin(format json); ERROR: COPY FORMAT JSON is not supported for COPY FROM +copy copytest to stdout (freeze false); +ERROR: COPY FREEZE cannot be used with COPY TO -- all of the above should yield error -- column list with json format copy copytest (style, test, filler) to stdout (format json); @@ -147,6 +179,12 @@ copy copytest (style, test, filler) to stdout (format json); -- should fail: force_array requires json format copy copytest to stdout (format csv, force_array true); ERROR: COPY FORCE_ARRAY can only be used with JSON mode +copy copytest to stdout (format csv, force_array false); +ERROR: COPY FORCE_ARRAY can only be used with JSON mode +copy copytest to stdout (force_array false); +ERROR: COPY FORCE_ARRAY can only be used with JSON mode +copy copytest from stdin (force_array false); +ERROR: COPY FORCE_ARRAY can only be used with JSON mode -- force_array variants copy copytest to stdout (format json, force_array); [ diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql index eaad290b257..e3cf2c0d295 100644 --- a/src/test/regress/sql/copy.sql +++ b/src/test/regress/sql/copy.sql @@ -94,18 +94,34 @@ copy (select * from copytest) to stdout (format json); -- all of the following should yield error copy copytest to stdout (format json, delimiter '|'); +copy copytest to stdout (format json, delimiter); copy copytest to stdout (format json, null '\N'); +copy copytest to stdout (format json, null); copy copytest to stdout (format json, default '|'); +copy copytest to stdout (format json, default); copy copytest to stdout (format json, header); copy copytest to stdout (format json, header 1); +copy copytest to stdout (format json, header false); +copy copytest to stdout (format json, header 0); +copy copytest to stdout (format binary, header false); +copy copytest to stdout (format json, escape); +copy copytest to stdout (format json, quote); copy copytest to stdout (format json, quote '"'); copy copytest to stdout (format json, escape '"'); +copy copytest to stdout (format json, quote); +copy copytest to stdout (format json, escape); +copy copytest to stdout (delimiter); +copy copytest to stdout (null); +copy copytest from stdin (default); +copy copytest to stdout (format csv, quote); +copy copytest to stdout (format csv, escape); copy copytest to stdout (format json, force_quote *); copy copytest to stdout (format json, force_not_null *); copy copytest to stdout (format json, force_null *); copy copytest to stdout (format json, on_error ignore); copy copytest to stdout (format json, reject_limit 1); copy copytest from stdin(format json); +copy copytest to stdout (freeze false); -- all of the above should yield error -- column list with json format @@ -113,6 +129,9 @@ copy copytest (style, test, filler) to stdout (format json); -- should fail: force_array requires json format copy copytest to stdout (format csv, force_array true); +copy copytest to stdout (format csv, force_array false); +copy copytest to stdout (force_array false); +copy copytest from stdin (force_array false); -- force_array variants copy copytest to stdout (format json, force_array); -- 2.50.1 (Apple Git-155)