[PATCHES] tuple count and v3 functions in psql for COPY
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. Added an uint64 processed to struct CopyStateData. It's incremented each time on a tuple send/receive made by CopyTo/CopyFrom. Collected result is added to COPY command's commandTag which can be gathered by PQcmdStatus(). (Also updated PQcmdTuples() to work with it.) When I tried to modify psql to print the COPY's commandTag, I found that its implementation is really disorganized when we still use old COPY commands. Thus replaced old COPY routines with the new ones. (IMHO, modified design for the psql's COPY should work faster than the previous. Because, in this patch, I don't read input one by one with getc(). Just filled the buffer with fgets and made \r search only in the first and last lines.) 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. ? src/Makefile.global ? src/cscope.out ? src/backend/cscope.out ? src/backend/postgres ? src/backend/catalog/postgres.bki ? src/backend/catalog/postgres.description ? src/backend/commands/cscope.out ? src/backend/executor/cscope.out ? src/backend/libpq/cscope.out ? src/backend/utils/mb/conversion_procs/conversion_create.sql ? src/backend/utils/mb/conversion_procs/ascii_and_mic/libascii_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/libcyrillic_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/libeuc_cn_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/libeuc_jp_and_sjis.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/libeuc_kr_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/libeuc_tw_and_big5.so.0.0 ? src/backend/utils/mb/conversion_procs/latin2_and_win1250/liblatin2_and_win1250.so.0.0 ? src/backend/utils/mb/conversion_procs/latin_and_mic/liblatin_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_ascii/libutf8_and_ascii.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_big5/libutf8_and_big5.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/libutf8_and_cyrillic.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/libutf8_and_euc_cn.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_jp/libutf8_and_euc_jp.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_kr/libutf8_and_euc_kr.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_tw/libutf8_and_euc_tw.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_gb18030/libutf8_and_gb18030.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_gbk/libutf8_and_gbk.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_iso8859/libutf8_and_iso8859.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/libutf8_and_iso8859_1.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_johab/libutf8_and_johab.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_sjis/libutf8_and_sjis.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_uhc/libutf8_and_uhc.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_win1250/libutf8_and_win1250.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_win1252/libutf8_and_win1252.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_win1256/libutf8_and_win1256.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_win1258/libutf8_and_win1258.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_win874/libutf8_and_win874.so.0.0 ? src/bin/initdb/initdb ? src/bin/ipcclean/ipcclean ? src/bin/pg_config/pg_config ? src/bin/pg_controldata/pg_controldata ? src/bin/pg_ctl/pg_ctl ? src/bin/pg_dump/pg_dump ? src/bin/pg_dump/pg_dumpall ? src/bin/pg_dump/pg_restore ? src/bin/pg_resetxlog/pg_resetxlog ? src/bin/psql/cscope.out ? src/bin/psql/psql ? src/bin/scripts/clusterdb ? src/bin/scripts/createdb ? src/bin/scripts/createlang ? src/bin/scripts/createuser ? src/bin/scripts/dropdb ? src/bin/scripts/droplang ? src/bin/scripts/dropuser ? src/bin/scripts/reindexdb ? src/bin/scripts/vacuumdb ? src/include/pg_config.h ? src/include/stamp-h ? src/interfaces/ecpg/compatlib/libecpg_compat.so.2.2 ? src/interfaces/ecpg/ecpglib/libecpg.so.5.3 ? src/interfaces/ecpg/pgtypeslib/libpgtypes.so.2.2 ? src/interfaces/ecpg/preproc/ecpg ? src/interfaces/libpq/cscope.out ? src/interfaces/libpq/libpq.so.4.2 ? src/pl/plpgsql/src/libplpgsql.so.1.0 ? src/port/pg_config_paths.h ? src/test/regress/libregress.so.0.0 ? src/test/regress/pg_regress ? src/test/regress/expected/constraints.out ? src/test/regress/expected/copy.out ? src/test/regress/expected/create_function_1.out ? src/test/regress/expected/create_function_2.out ? src/test/regress/expected/misc.out ? src/test/regress/expected/tablespace.out ? src/test/regress/sql/constraints.sql ? src/test/regress/sql/copy.sql ?
Re: [PATCHES] Single-Transaction Utility options
On Sat, 2005-12-17 at 20:03 +0100, Peter Eisentraut wrote: Simon Riggs wrote: Changes as discussed. singletransaction.patch attached. I meant to ask, why is this not the default or only behavior? Historically, it didn't work that way, so I hadn't thought to change that behaviour. We could I suppose... but I'm happy with just an option to do --single-transaction. Your patch does not contain a documentation update, and so the user has no information about why to use this option or not. I was waiting for tech approval of the patch before writing the docs. A doc patch is enclosed. Best Regards, Simon Riggs Index: doc/src/sgml/ref/pg_restore.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/pg_restore.sgml,v retrieving revision 1.56 diff -c -r1.56 pg_restore.sgml *** doc/src/sgml/ref/pg_restore.sgml 1 Nov 2005 21:09:50 - 1.56 --- doc/src/sgml/ref/pg_restore.sgml 18 Dec 2005 18:51:57 - *** *** 448,453 --- 448,466 /para /listitem /varlistentry + + varlistentry + termoption-1/option/term + termoption--single-transaction/option/term + listitem +para + Force the restore to execute as a single transaction. Either all + SQL statements complete successfully, or no changes are applied. This + option also forces --exit-on-error. +/para + /listitem + /varlistentry + /variablelist /para /refsect1 Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.155 diff -c -r1.155 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 9 Dec 2005 19:19:17 - 1.155 --- doc/src/sgml/ref/psql-ref.sgml 18 Dec 2005 18:52:00 - *** *** 463,468 --- 463,480 /listitem /varlistentry + varlistentry + termoption-1/option/term + termoption--single-transaction/option/term + listitem +para + When psql executes a script with the -f option, this additional option + will force the script to execute as a single transaction. Either all + SQL statements complete successfully, or no changes are applied. +/para + /listitem + /varlistentry + varlistentry termoption-?//term termoption--help//term ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Single-Transaction Utility options
Tom Lane wrote: I believe Peter's question was rhetorical: what he meant to point out is that the documentation needs to explain what is the reason for having this switch, ie, in what cases would you use it or not use it? Just saying what it does isn't really adequate docs. I once considered implementing this myself but found it infeasible for some reason I don't remember. Nevertheless I always thought that having an atomic restore ought to be a non-optional feature. Are there situations where one would not want to use it? (And if so, which one is the more normal case?) -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Single-Transaction Utility options
On Sun, 2005-12-18 at 14:04 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Sat, 2005-12-17 at 20:03 +0100, Peter Eisentraut wrote: I meant to ask, why is this not the default or only behavior? Historically, it didn't work that way, so I hadn't thought to change that behaviour. We could I suppose... but I'm happy with just an option to do --single-transaction. I believe Peter's question was rhetorical: what he meant to point out is that the documentation needs to explain what is the reason for having this switch, ie, in what cases would you use it or not use it? Just saying what it does isn't really adequate docs. Well, you know the reason: to allow pg_restore and psql take advantage of the COPY optimization I'm just about to submit. When that patch is accepted, I'll update these docs to explain that. But the two patches are separable, since the -1 still has value anyway. Best Regards, Simon Riggs ---(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] COPY LOCK for WAL bypass
On Sat, 2005-12-10 at 12:07 +, Simon Riggs wrote: Following patch implements COPY ... FROM ... LOCK Patch now updated so that it includes an additional optimization of COPY, so that WAL will not be written in the transaction that created the table. This now gives two fast paths for COPY: 1) COPY LOCK 2) COPY in same transaction (e.g. reloading a pg_dump) Patch passes make check on cvstip. No docs yet, but let me know if this is OK and I'll work on them. [Other copied in from the related patch thread on Single-Transaction Utility options. With this new COPY optimization the --single-transaction option will considerably increase performance.] Performance tests shown on previous post for this thread. Best Regards, Simon Riggs Index: src/backend/access/heap/heapam.c === RCS file: /projects/cvsroot/pgsql/src/backend/access/heap/heapam.c,v retrieving revision 1.205 diff -c -r1.205 heapam.c *** src/backend/access/heap/heapam.c 26 Nov 2005 05:03:06 - 1.205 --- src/backend/access/heap/heapam.c 18 Dec 2005 21:10:52 - *** *** 28,33 --- 28,34 * heap_update - replace a tuple in a relation with another tuple * heap_markpos - mark scan position * heap_restrpos - restore position to marked location + * heap_sync - sync heap, for when no WAL has been written * * NOTES * This file contains the heap_ routines which implement *** *** 49,54 --- 50,56 #include miscadmin.h #include pgstat.h #include storage/procarray.h + #include storage/smgr.h #include utils/inval.h #include utils/relcache.h *** *** 1335,1342 * non-temp relation. Safe usage of this behavior requires that we arrange * that all new tuples go into new pages not containing any tuples from other * transactions, that the relation gets fsync'd before commit, and that the ! * transaction emits at least one WAL record to ensure RecordTransactionCommit ! * will decide to WAL-log the commit. * * use_fsm is passed directly to RelationGetBufferForTuple, which see for * more info. --- 1337,1345 * non-temp relation. Safe usage of this behavior requires that we arrange * that all new tuples go into new pages not containing any tuples from other * transactions, that the relation gets fsync'd before commit, and that the ! * transaction emits at least one WAL record or must mark ! * MyXactMustRecordCommitIfAny=true to ensure RecordTransactionCommit will ! * decide to WAL-log the commit. (see heap_sync() comments also) * * use_fsm is passed directly to RelationGetBufferForTuple, which see for * more info. *** *** 1413,1419 if (relation-rd_istemp) { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMadeTempRelUpdate = true; } else if (use_wal) { --- 1416,1422 if (relation-rd_istemp) { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMustRecordCommitIfAny = true; } else if (use_wal) { *** *** 1731,1737 else { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMadeTempRelUpdate = true; } END_CRIT_SECTION(); --- 1734,1740 else { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMustRecordCommitIfAny = true; } END_CRIT_SECTION(); *** *** 2172,2178 else { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMadeTempRelUpdate = true; } END_CRIT_SECTION(); --- 2175,2181 else { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMustRecordCommitIfAny = true; } END_CRIT_SECTION(); *** *** 2674,2680 else { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMadeTempRelUpdate = true; } END_CRIT_SECTION(); --- 2677,2683 else { /* No XLOG record, but still need to flag that XID exists on disk */ ! MyXactMustRecordCommitIfAny = true; } END_CRIT_SECTION(); *** *** 3456,3458 --- 3459,3490 else strcat(buf, UNKNOWN); } + + /* + * heap_sync - sync a heap, for use when no WAL has been written + * + * + */ + void + heap_sync(Relation rel, bool needToRecordCommit) + { + /* + * If our transaction has avoided writing WAL up to now, + * we need to force the changes to be written to disk then make + * commit record visible in clog, so other users can view these changes + */ + if (needToRecordCommit) + MyXactMustRecordCommitIfAny = true; + + if (!rel-rd_istemp) + { + /* + * If we skipped using WAL, and it's not a temp relation, + * we must force the relation down to disk before it's + *
Re: [PATCHES] Single-Transaction Utility options
On Sun, 2005-12-18 at 21:51 +0100, Peter Eisentraut wrote: Tom Lane wrote: I believe Peter's question was rhetorical: what he meant to point out is that the documentation needs to explain what is the reason for having this switch, ie, in what cases would you use it or not use it? Just saying what it does isn't really adequate docs. I once considered implementing this myself but found it infeasible for some reason I don't remember. Nevertheless I always thought that having an atomic restore ought to be a non-optional feature. Are there situations where one would not want to use it? (And if so, which one is the more normal case?) You're thinking is good. I guess if restores never failed, I'd be inclined to agree 100%, but I'm at about 80% right now. I'd say: if the patch is accepted technically, lets debate this point more widely on -hackers. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[PATCHES] Test, please ignore
Test. -- 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 ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Single-Transaction Utility options
Peter Eisentraut [EMAIL PROTECTED] writes: I once considered implementing this myself but found it infeasible for some reason I don't remember. Nevertheless I always thought that having an atomic restore ought to be a non-optional feature. Are there situations where one would not want to use it? Absolutely. As a nontrivial example, I *very* often load dumps sent to me by other people which are full of GRANT/REVOKE commands referencing users that don't exist in my installation. Since, most of the time, I don't particularly care about the ownership/privileges of the tables involved, having to create those users would just be a PITA. More generally, the pg_dump output has always been designed around the assumption that failed commands are non-fatal. Look at all those unportable SET commands that we don't give you an option to omit. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] COPY LOCK for WAL bypass
[EMAIL PROTECTED] (Simon Riggs) writes: On Sat, 2005-12-10 at 12:07 +, Simon Riggs wrote: Following patch implements COPY ... FROM ... LOCK Patch now updated so that it includes an additional optimization of COPY, so that WAL will not be written in the transaction that created the table. This now gives two fast paths for COPY: 1) COPY LOCK 2) COPY in same transaction (e.g. reloading a pg_dump) I presume that if this doesn't go into WAL, that means that this kind of update wouldn't play with PITR, right? That's presumably something that otta be pretty carefully documented :-). -- select 'cbbrowne' || '@' || 'cbbrowne.com'; http://cbbrowne.com/info/nonrdbms.html Microsoft Outlook: Deploying Viruses Has Never Been This Easy! ---(end of broadcast)--- TIP 6: explain analyze is your friend