On Sat, Oct 28, 2023 at 09:39:53PM -0400, Tom Lane wrote: > Bruce Momjian <br...@momjian.us> writes: > > My apologies, wrong patch attached, right one attached now. > > I think this one is fine as-is: > > /* Only single-byte delimiter strings are supported. */ > if (strlen(opts_out->delim) != 1) > ereport(ERROR, > - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > errmsg("COPY delimiter must be a single > one-byte character"))); > > While we have good implementation reasons for this restriction, > there's nothing illogical about wanting the delimiter to be more > general. It's particularly silly, from an end-user's standpoint, > that for example 'é' is an allowed delimiter in LATIN1 encoding > but not when the server is using UTF8. So I don't see how the > distinction you presented justifies this change.
Agreed, my mistake. > + if (opts_out->freeze && !is_from) > + ereport(ERROR, > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > + errmsg("COPY freeze only available using COPY > FROM"))); > > Not thrilled by the wording here. I don't like the fact that the > keyword FREEZE isn't capitalized, and I think you omitted too many > words for intelligibility to be preserved. Notably, all the adjacent > examples use "must" or "must not", and this decides that that can be > omitted. I think it is modeled after: errmsg("COPY force null only available using COPY FROM"))); > I realize that you probably modeled the non-capitalization on nearby > messages like "COPY delimiter", but there's a difference IMO: > "delimiter" can be read as an English noun, but it's hard to read > "freeze" as a noun. > > How about, say, > > errmsg("COPY FREEZE must not be used in COPY TO"))); > > or perhaps that's redundant and we could write > > errmsg("FREEZE option must not be used in COPY TO"))); I now have: errmsg("COPY FREEZE mode only available using COPY FROM"))); Updated patch attached. -- Bruce Momjian <br...@momjian.us> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml index d12ba96497..82b65543c3 100644 --- a/doc/src/sgml/ref/copy.sgml +++ b/doc/src/sgml/ref/copy.sgml @@ -224,6 +224,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable open and there are no older snapshots held by this transaction. It is currently not possible to perform a <command>COPY FREEZE</command> on a partitioned table. + This option is allowed only in <command>COPY FROM</command>. </para> <para> Note that all other sessions will immediately be able to see the data diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index c5d7d78645..92558b6bb0 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -728,16 +728,22 @@ ProcessCopyOptions(ParseState *pstate, /* Don't allow the delimiter to appear in the null string. */ if (strchr(opts_out->null_print, opts_out->delim[0]) != NULL) ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("COPY delimiter must not appear in the NULL specification"))); /* Don't allow the CSV quote char to appear in the null string. */ if (opts_out->csv_mode && strchr(opts_out->null_print, opts_out->quote[0]) != NULL) ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("CSV quote character must not appear in the NULL specification"))); + /* Check freeze */ + if (opts_out->freeze && !is_from) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("COPY FREEZE mode only available using COPY FROM"))); + if (opts_out->default_print) { if (!is_from)