Re: [PATCHES] tuple count and v3 functions in psql for COPY
Applied by Tom Lane. Thanks. --- Volkan YAZICI wrote: > On Feb 11 11:24, Bruce Momjian wrote: > > I have updated the patch to match CVS (attached), but am seeing the > > following regression differences where the COPY error messages are now > > missing. > > I've revised the patch (attached) and now it seems to be ok - passes > regression tests too. Any other comments are welcome. > > > Regards. [ Attachment, skipping... ] > > ---(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 -- Bruce Momjian http://candle.pha.pa.us SRA OSS, Inc. http://www.sraoss.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] tuple count and v3 functions in psql for COPY
On Feb 11 11:24, Bruce Momjian wrote:
> I have updated the patch to match CVS (attached), but am seeing the
> following regression differences where the COPY error messages are now
> missing.
I've revised the patch (attached) and now it seems to be ok - passes
regression tests too. Any other comments are welcome.
Regards.
Index: src/backend/commands/copy.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.258
diff -c -r1.258 copy.c
*** src/backend/commands/copy.c 3 Feb 2006 12:41:07 - 1.258
--- src/backend/commands/copy.c 1 Mar 2006 21:46:28 -
***
*** 102,107
--- 102,108
int client_encoding;/* remote side's
character encoding */
boolneed_transcoding; /* client encoding diff
from server? */
boolencoding_embeds_ascii; /* ASCII can be non-first byte?
*/
+ uint64 processed; /* # of tuples
processed */
/* parameters from the COPY command */
Relationrel;/* relation to copy to or from
*/
***
*** 710,716
* Do not allow the copy if user doesn't have proper permission to access
* the table.
*/
! void
DoCopy(const CopyStmt *stmt)
{
CopyState cstate;
--- 711,717
* Do not allow the copy if user doesn't have proper permission to access
* the table.
*/
! uint64
DoCopy(const CopyStmt *stmt)
{
CopyState cstate;
***
*** 724,729
--- 725,731
AclMode required_access = (is_from ? ACL_INSERT : ACL_SELECT);
AclResult aclresult;
ListCell *option;
+ uint64 processed;
/* Allocate workspace and zero all fields */
cstate = (CopyStateData *) palloc0(sizeof(CopyStateData));
***
*** 1019,1024
--- 1021,1027
cstate->line_buf_converted = false;
cstate->raw_buf = (char *) palloc(RAW_BUF_SIZE + 1);
cstate->raw_buf_index = cstate->raw_buf_len = 0;
+ cstate->processed = 0;
/* Set up encoding conversion info */
cstate->client_encoding = pg_get_client_encoding();
***
*** 1164,1170
--- 1167,1176
pfree(cstate->attribute_buf.data);
pfree(cstate->line_buf.data);
pfree(cstate->raw_buf);
+
+ processed = cstate->processed;
pfree(cstate);
+ return processed;
}
***
*** 1399,1404
--- 1405,1412
}
CopySendEndOfRow(cstate);
+
+ cstate->processed++;
MemoryContextSwitchTo(oldcontext);
}
***
*** 2002,2007
--- 2010,2017
/* AFTER ROW INSERT Triggers */
ExecARInsertTriggers(estate, resultRelInfo, tuple);
+
+ cstate->processed++;
}
}
Index: src/backend/tcop/utility.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/tcop/utility.c,v
retrieving revision 1.252
diff -c -r1.252 utility.c
*** src/backend/tcop/utility.c 12 Feb 2006 19:11:01 - 1.252
--- src/backend/tcop/utility.c 1 Mar 2006 21:46:47 -
***
*** 640,646
break;
case T_CopyStmt:
! DoCopy((CopyStmt *) parsetree);
break;
case T_PrepareStmt:
--- 640,651
break;
case T_CopyStmt:
! {
! uint64 processed = DoCopy((CopyStmt *)
parsetree);
!
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
!"COPY " UINT64_FORMAT,
processed);
! }
break;
case T_PrepareStmt:
Index: src/bin/pg_dump/pg_backup_db.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_db.c,v
retrieving revision 1.69
diff -c -r1.69 pg_backup_db.c
*** src/bin/pg_dump/pg_backup_db.c 12 Feb 2006 06:11:50 - 1.69
--- src/bin/pg_dump/pg_backup_db.c 1 Mar 2006 21:46:50 -
***
*** 391,396
--- 391,398
* enter COPY mode; this allows us to behave reasonably when trying
* to continue after an error in a COPY command.
*/
+ if (AH->pgCopyIn &&
+ PQputCopyData(AH->connection, AH->pgCopyBuf->data,
AH->pgCopyBuf->len) < 0)
if (AH->pgCopyIn && PQputline(AH->connection, AH->pgCopyBuf->data) != 0)
die_horribly(AH, modulename, "error returned by PQputline: %s",
Re: [PATCHES] tuple count and v3 functions in psql for COPY
Volkan YAZICI wrote:
> Here's a new try. This one touches to pg_dump side too - for v3
> COPY functions usage instead of deprecated ones.
>
> I'd be so appreciated if somebody can review attached patch.
I have updated the patch to match CVS (attached), but am seeing the
following regression differences where the COPY error messages are now
missing. Any idea what is causing that?
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/backend/commands/copy.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.258
diff -c -c -r1.258 copy.c
*** src/backend/commands/copy.c 3 Feb 2006 12:41:07 - 1.258
--- src/backend/commands/copy.c 12 Feb 2006 04:22:16 -
***
*** 102,107
--- 102,108
int client_encoding;/* remote side's
character encoding */
boolneed_transcoding; /* client encoding diff
from server? */
boolencoding_embeds_ascii; /* ASCII can be non-first byte?
*/
+ uint64 processed; /* # of tuples
processed */
/* parameters from the COPY command */
Relationrel;/* relation to copy to or from
*/
***
*** 710,716
* Do not allow the copy if user doesn't have proper permission to access
* the table.
*/
! void
DoCopy(const CopyStmt *stmt)
{
CopyState cstate;
--- 711,717
* Do not allow the copy if user doesn't have proper permission to access
* the table.
*/
! uint64
DoCopy(const CopyStmt *stmt)
{
CopyState cstate;
***
*** 724,729
--- 725,731
AclMode required_access = (is_from ? ACL_INSERT : ACL_SELECT);
AclResult aclresult;
ListCell *option;
+ uint64 processed;
/* Allocate workspace and zero all fields */
cstate = (CopyStateData *) palloc0(sizeof(CopyStateData));
***
*** 1019,1024
--- 1021,1027
cstate->line_buf_converted = false;
cstate->raw_buf = (char *) palloc(RAW_BUF_SIZE + 1);
cstate->raw_buf_index = cstate->raw_buf_len = 0;
+ cstate->processed = 0;
/* Set up encoding conversion info */
cstate->client_encoding = pg_get_client_encoding();
***
*** 1164,1170
--- 1167,1176
pfree(cstate->attribute_buf.data);
pfree(cstate->line_buf.data);
pfree(cstate->raw_buf);
+
+ processed = cstate->processed;
pfree(cstate);
+ return processed;
}
***
*** 1396,1401
--- 1402,1409
VARSIZE(outputbytes) - VARHDRSZ);
}
}
+
+ cstate->processed++;
}
CopySendEndOfRow(cstate);
***
*** 2002,2007
--- 2010,2017
/* AFTER ROW INSERT Triggers */
ExecARInsertTriggers(estate, resultRelInfo, tuple);
+
+ cstate->processed++;
}
}
Index: src/backend/tcop/utility.c
===
RCS file: /cvsroot/pgsql/src/backend/tcop/utility.c,v
retrieving revision 1.251
diff -c -c -r1.251 utility.c
*** src/backend/tcop/utility.c 11 Feb 2006 22:17:19 - 1.251
--- src/backend/tcop/utility.c 12 Feb 2006 04:22:17 -
***
*** 640,646
break;
case T_CopyStmt:
! DoCopy((CopyStmt *) parsetree);
break;
case T_PrepareStmt:
--- 640,651
break;
case T_CopyStmt:
! {
! uint64 processed = DoCopy((CopyStmt *)
parsetree);
!
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
!"COPY " UINT64_FORMAT,
processed);
! }
break;
case T_PrepareStmt:
Index: src/bin/pg_dump/pg_backup_db.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_db.c,v
retrieving revision 1.68
diff -c -c -r1.68 pg_backup_db.c
*** src/bin/pg_dump/pg_backup_db.c 9 Feb 2006 18:28:29 - 1.68
--- src/bin/pg_dump/pg_backup_db.c 12 Feb 2006 04:22:17 -
***
*** 388,393
--- 388,395
* enter COPY mode; this allows us to behave re
Re: [PATCHES] tuple count and v3 functions in psql for COPY
I've just realized an exception in the sending data while using PQsendCopyData(). Function's blocking behaviour is not guaranteed, therefore a if (PQisnonblocking()) PQsetnonblocking(conn, 0) kind of phrase required before using PQsendCopyData(). But after a small investigation, found that both psql and pg_dump runs in blocking mode - without any PQsetnonblocking() call. Thus, AFAIC, above phrase isn't necessary. FYI. Just wanted to inform. Any comments are welcome. Regards. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] tuple count and v3 functions in psql for COPY
Here's a new try. This one touches to pg_dump side too - for v3
COPY functions usage instead of deprecated ones.
I'd be so appreciated if somebody can review attached patch.
Regards.
Index: src/backend/commands/copy.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.255
diff -u -c -r1.255 copy.c
*** src/backend/commands/copy.c 22 Nov 2005 18:17:08 - 1.255
--- src/backend/commands/copy.c 23 Dec 2005 13:29:57 -
***
*** 102,107
--- 102,108
int client_encoding;/* remote side's
character encoding */
boolneed_transcoding; /* client encoding diff
from server? */
boolclient_only_encoding; /* encoding not valid on
server? */
+ uint64 processed; /* # of tuples
processed */
/* parameters from the COPY command */
Relationrel;/* relation to copy to or from
*/
***
*** 646,652
* Do not allow the copy if user doesn't have proper permission to access
* the table.
*/
! void
DoCopy(const CopyStmt *stmt)
{
CopyState cstate;
--- 647,653
* Do not allow the copy if user doesn't have proper permission to access
* the table.
*/
! uint64
DoCopy(const CopyStmt *stmt)
{
CopyState cstate;
***
*** 660,665
--- 661,667
AclMode required_access = (is_from ? ACL_INSERT : ACL_SELECT);
AclResult aclresult;
ListCell *option;
+ uint64 processed;
/* Allocate workspace and zero all fields */
cstate = (CopyStateData *) palloc0(sizeof(CopyStateData));
***
*** 936,941
--- 938,944
cstate->line_buf_converted = false;
cstate->raw_buf = (char *) palloc(RAW_BUF_SIZE + 1);
cstate->raw_buf_index = cstate->raw_buf_len = 0;
+ cstate->processed = 0;
/* Set up encoding conversion info */
cstate->client_encoding = pg_get_client_encoding();
***
*** 1080,1086
--- 1083,1092
pfree(cstate->attribute_buf.data);
pfree(cstate->line_buf.data);
pfree(cstate->raw_buf);
+
+ processed = cstate->processed;
pfree(cstate);
+ return processed;
}
***
*** 1310,1315
--- 1316,1323
VARSIZE(outputbytes) - VARHDRSZ);
}
}
+
+ cstate->processed++;
}
CopySendEndOfRow(cstate);
***
*** 1916,1921
--- 1924,1931
/* AFTER ROW INSERT Triggers */
ExecARInsertTriggers(estate, resultRelInfo, tuple);
+
+ cstate->processed++;
}
}
Index: src/backend/tcop/utility.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/tcop/utility.c,v
retrieving revision 1.250
diff -u -c -r1.250 utility.c
*** src/backend/tcop/utility.c 29 Nov 2005 01:25:49 - 1.250
--- src/backend/tcop/utility.c 23 Dec 2005 13:29:59 -
***
*** 640,646
break;
case T_CopyStmt:
! DoCopy((CopyStmt *) parsetree);
break;
case T_PrepareStmt:
--- 640,651
break;
case T_CopyStmt:
! {
! uint64 processed = DoCopy((CopyStmt *)
parsetree);
!
! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
!"COPY " UINT64_FORMAT,
processed);
! }
break;
case T_PrepareStmt:
Index: src/bin/pg_dump/pg_backup_db.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_db.c,v
retrieving revision 1.66
diff -u -c -r1.66 pg_backup_db.c
*** src/bin/pg_dump/pg_backup_db.c 15 Oct 2005 02:49:38 - 1.66
--- src/bin/pg_dump/pg_backup_db.c 23 Dec 2005 13:30:00 -
***
*** 389,396
*-
*/
! if (PQputline(AH->connection, AH->pgCopyBuf->data) != 0)
! die_horribly(AH, modulename, "error returned by PQputline\n");
resetPQExpBuffer(AH->pgCopyBuf);
--- 389,396
*-
*/
! if (PQputCopyData(AH->connection, AH->pgCopyBuf->data,
AH->pgCopyBuf->len) < 0)
! die_horribly(AH, modulename, "error returned by
PQputCopyData\n");
resetPQExpBuffer(AH->pgCopyBuf);
Re: [PATCHES] tuple count and v3 functions in psql for COPY
On Dec 22 01:52, Tom Lane wrote:
> > ! {
> > ! uint64processed = DoCopy((CopyStmt *) parsetree);
> > ! charbuf[21];
> > !
> > ! snprintf(buf, sizeof(buf), UINT64_FORMAT, processed);
> > ! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
> > ! "COPY %s", buf);
> > ! }
>
> This is ugly and unnecessary. Why not
>
> > ! {
> > ! uint64processed = 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."
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] tuple count and v3 functions in psql for COPY
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.
> ! {
> ! uint64processed = DoCopy((CopyStmt *) parsetree);
> ! charbuf[21];
> !
> ! snprintf(buf, sizeof(buf), UINT64_FORMAT, processed);
> ! snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
> ! "COPY %s", buf);
> ! }
This is ugly and unnecessary. Why not
> ! {
> ! uint64processed = 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
Re: [PATCHES] tuple count and v3 functions in psql for COPY
Volkan YAZICI wrote: 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. I'll apply this today or tomorrow, barring any objections. BTW, if we're using an int64 counter for the # of rows modified by COPY, I wonder if it's also worth converting INSERT's rowcount to use an int64. Any objections to doing that? -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
