Volkan YAZICI <[EMAIL PROTECTED]> writes: > I tried to prepare a patch for these TODO items: > - Have COPY return the number of rows loaded/unloaded? > - Update [pg_dump and] psql to use the new COPY libpq API.
> ! cstate->raw_buf_index = cstate->raw_buf_len = 0; > ! cstate->raw_buf_index = cstate->raw_buf_len = cstate->processed = 0; Minor stylistic gripe: processed is unrelated to those two other variables and not even the same datatype. It'd be better to zero it in a separate statement. > ! { > ! 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); > ! } 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. ! for (c = p; isdigit((int) *c); ++c) Wrong. Use (unsigned char). regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match