On Tue, Aug 2, 2022 at 05:17:35PM +0900, Kyotaro Horiguchi wrote:
> At Tue, 2 Aug 2022 14:17:46 +0800, Julien Rouhaud <[email protected]> wrote
> in
> > Hi,
> >
> > On Tue, Aug 02, 2022 at 01:30:46PM +0900, Kyotaro Horiguchi wrote:
> > > I noticed that COPY TO accepts FREEZE option but it is pointless.
> > >
> > > Don't we reject that option as the first-attached does?
> >
> > I agree that we should reject it, +1 for the patch.
>
> Thanks for looking it!
>
> > > By the way, most of the invalid option combinations for COPY are
> > > marked as ERRCODE_FEATURE_NOT_SUPPORTED. I looks to me saying that
> > > "that feature is theoretically possible or actually realized
> > > elsewhere, but impossible now or here".
> > >
> > > If it is correct, aren't they better be ERRCODE_INVALID_PARAMETER_VALUE?
> > > The
> > > code is being used for similar messages "unrecognized parameter <name>"
> > > and
> > > "parameter <name> specified more than once" (or some others?). At least a
> > > quote string longer than a single character seems like to fit
> > > INVALID_PARAMETER_VALUE. (I believe we don't mean to support
> > > multicharacter
> > > (or even multibyte) escape/quote character anddelimiter). That being
> > > said,
> > > I'm not sure if the change will be worth the trouble.
> >
> > I also feel weird about it. I raised the same point recently about COPY
> > FROM +
> > HEADER MATCH (1), and at that time there wasn't a real consensus on the way
> > to
> > go, just keep the things consistent. I'm +0.5 on that patch for the same
> > reason as back then. My only concern is that it can in theory break things
> > if
> > you rely on the current sqlstate, but given the errors I don't think it's
> > really a problem.
>
> Exactly. That is the exact reason for my to say "I'm not sure if..".
>
> > [1]:
> > https://www.postgresql.org/message-id/flat/20220614091319.jk4he5migtpwyd7r%40jrouhaud#b18bf3705fb9f69d0112b6febf0fa1be
>
> > Maybe that's just me but I understand "not supported" as "this makes
> > sense, but this is currently a limitation that might be lifted
> > later".
>
> FWIW I understand it the same way.
I would like to apply the attached patch to master. Looking at your
adjustments for ERRCODE_FEATURE_NOT_SUPPORTED to
ERRCODE_INVALID_PARAMETER_VALUE, I only changed the cases where it would
be illogical to implement the feature, not just that we have no
intention of implementing the feature. I read "invalid" as "illogical".
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index d94e3cacfc..cc7d797159 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1119,6 +1119,10 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
destination, because all data must pass through the client/server
connection. For large amounts of data the <acronym>SQL</acronym>
command might be preferable.
+ Also, because of this pass-through method, <literal>\copy
+ ... from</literal> in <acronym>CSV</acronym> mode will erroneously
+ treat a <literal>\.</literal> data value alone on a line as an
+ end-of-input marker.
</para>
</tip>
diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index b3cc3d9a29..8d6ce4cedd 100644
--- a/src/bin/psql/copy.c
+++ b/src/bin/psql/copy.c
@@ -627,6 +627,7 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res)
* This code erroneously assumes '\.' on a line alone
* inside a quoted CSV string terminates the \copy.
* https://www.postgresql.org/message-id/[email protected]
+ * https://www.postgresql.org/message-id/[email protected]
*/
if ((linelen == 3 && memcmp(fgresult, "\\.\n", 3) == 0) ||
(linelen == 4 && memcmp(fgresult, "\\.\r\n", 4) == 0))
