Re: tuple count and v3 functions in psql for COPY

On Dec 22 01:52, Tom Lane wrote:
> > ! {
> > ! uint64 processed = DoCopy((CopyStmt *) parsetree);
> > ! char buf[21];
> > !
> > ! snprintf(buf, sizeof(buf), UINT64_FORMAT, processed);
> > ! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
> > ! "COPY %s", buf);
> > ! }
>
> This is ugly and unnecessary. Why not
>
> > ! {
> > ! uint64 processed = DoCopy((CopyStmt *) parsetree);
> > !
> > ! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
> > ! "COPY " UINT64_FORMAT, processed);
> > ! }

At the beginning I used the style as above, but after looking at the
source code for INT64_FORMAT usage, saw that nearly all of them use
a seperate buffer.

Furthermore, in the source code (for instance
backend/commands/sequence.c) nearly every buffer size used for
INT64_FORMAT is 100. AFAIK, 20+1 is enough for a 64 bit integer.

> Also, I think you've broken the psql-side handling of COPY IN. There's
> no check for I/O error on the input, and the test for terminator (\.\n)
> mistakenly assumes that the terminator could only appear at the start
> of the buffer, not to mention assuming that it couldn't span across two
> bufferloads.
>
> The check for \r is wrong too (counterexample: first input line exceeds
> 8K), but actually you should just dispense with that entirely, because
> if you're going to use PQputCopyEnd then it is not your responsibility
> to send the terminator.
>
> But the *real* problem with this coding is that it will fetch data past
> the \., which breaks files that include both COPY data and SQL. Thus
> for example this patch breaks pg_dump files.

8K limit is caused by a mis-understanding of me. Thanks so much for the
review, I'll try to fix above missing parts ASAP.

> ! for (c = p; isdigit((int) *c); ++c)
>
> Wrong. Use (unsigned char).

I found above from "The Open Group Base Specifications Issue 6". I'll
fix that too.

Regards.

--
"We are the middle children of history, raised by television to believe
that someday we'll be millionaires and movie stars and rock stars, but
we won't. And we're just learning this fact," Tyler said. "So don't
fuck with us."