Re: [PATCHES] patch: garbage error strings in libpq
Tom Lane wrote: > [EMAIL PROTECTED] writes: >> Another approach would have been to make libpq_gettext() preserve errno. > > That seems like a far easier, cleaner, and more robust fix than this. Provided that either: (a) the C standard has added a sequence point between the arguments in a function call, which AFAIK wasn't there before, or the sequence point was there all along (and the compiler implements it); (b) the compiler is sufficiently naive; (c) you get lucky with instruction scheduling on your particular architecture. This is why I called this approach was "tempting," but didn't go for it. I felt it was better to really fix the instances I found first, then see what patterns emerge and refactor. Like maybe a wrapper for printfPQExpBuffer() that takes a PGconn *, an untranslated format string, and varargs; this in turn can do the libpq_gettext(). That would cover all uses of printfPQExpBuffer() in libpq--except for one of the out-of-memory errors where no translation is done, which may have been unintentional (and this bug is again duplicated in the code). > Moreover I don't believe that this approach works either, as the result > of strerror() is not guaranteed still usable after another strerror call > (ie, it can use a static buffer repeatedly), so you'd still have the > problem if libpq_gettext invokes strerror. I suppose that a really > robust solution would involve libpq_gettext saving errno, restoring > errno, and invoking strerror() again ... Check again. The calls to strerror() are routed through pqStrerror() which copies the error message to the buffer, or in the case of GNU strerror_r(), at least ensures it is in some reusable location. Jeroen ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Satoshi Nagayasu wrote: > Bruce Momjian wrote: > > I am not sure what to do with this patch. It is missing dump > > capability, there is no clause to disable all triggers on a table, and > > it uses a table owner check when a super user check is required (because > > of referential integrity). > > > > Satoshi, are you still working on it? If not does someone else want to > > complete it? I realized you just started on it but the deadline is > > soon. > > I've already implemented 'ENABLE/DISABLE TRIGGER ALL', > and the superuser check has been added. Please check it. > > And, I'm going to have a business trip to Sydney this weekend, > so I'll complete pg_dump stuffs while my flight. OK, but once our patch queue is empty, we will need to process your patch. My guess is that we are several days away from 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 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Quick little \h enhancement for psql
Greg Sabino Mullane wrote: > > http://archives.postgresql.org/pgsql-patches/2005-05/msg00197.php Applied. --- Attached is a patch that enhances the "\h" capability in psql. I often find myself typing a command and then wanting to get the syntax for it. So I do a ctrl-a and add a \h: but psql does not recognize the command, because I have stuff attached to it (e.g. "alter table foobar"), so I have to scroll over and delete everything except the name of the command itself. This patch gives \h three chances to match: if nothing matches the complete string (current behavior), it tries to match the first two words (e.g. "ALTER TABLE"). If that fails, it tries to match the first word (e.g. "DELETE"). -- 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/bin/psql/help.c === RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v retrieving revision 1.102 diff -c -c -r1.102 help.c *** src/bin/psql/help.c 14 Jun 2005 02:57:41 - 1.102 --- src/bin/psql/help.c 6 Jul 2005 02:18:59 - *** *** 305,356 } else { ! int i; boolhelp_found = false; FILE *output; ! size_t len; int nl_count = 0; char *ch; ! /* don't care about trailing spaces or semicolons */ len = strlen(topic); while (topic[len - 1] == ' ' || topic[len - 1] == ';') ! len--; ! /* Count newlines for pager */ ! for (i = 0; QL_HELP[i].cmd; i++) { ! if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 || ! strcmp(topic, "*") == 0) ! { ! nl_count += 5; ! for (ch = QL_HELP[i].syntax; *ch != '\0'; ch++) ! if (*ch == '\n') ! nl_count++; ! /* If we have an exact match, exit. Fixes \h SELECT */ ! if (pg_strcasecmp(topic, QL_HELP[i].cmd) == 0) ! break; ! } ! } ! ! output = PageOutput(nl_count, pager); ! ! for (i = 0; QL_HELP[i].cmd; i++) ! { ! if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 || ! strcmp(topic, "*") == 0) ! { ! help_found = true; ! fprintf(output, _("Command: %s\n" ! "Description: %s\n" ! "Syntax:\n%s\n\n"), ! QL_HELP[i].cmd, ! _(QL_HELP[i].help), ! _(QL_HELP[i].syntax)); ! /* If we have an exact match, exit. Fixes \h SELECT */ ! if (pg_strcasecmp(topic, QL_HELP[i].cmd) == 0) ! break; ! } } if (!help_found) --- 305,382 } else { ! int i,j,x=0; boolhelp_found = false; FILE *output; ! size_t len, wordlen; int nl_count = 0; char *ch; ! /* User gets two chances: exact match, then the first word */ ! ! /* First pass : strip trailing spaces and semicolons */ len = strlen(topic); while (topic[len - 1] == ' ' || topic[len - 1] == ';') ! len--; ! for (x=1; x<=3; x++) /* Three chances to guess that word... */ { ! if (x>1) /* Nothing on first pass - try the opening words */ ! { ! wordlen=j=1; ! while (topic[j] != ' ' && j++= len) /* Don't try again if the same word */ ! { ! output = P
Re: [HACKERS] [PATCHES] Dbsize backend integration
Tom Lane wrote: > Bruce Momjian writes: > > Tom Lane wrote: > >> I could live with that. Or "pg_total_relation_size". > > > The problem with "total", to me, is that it already is the total size of > > the heap/index/toast. Complete has the idea of adding additional > > pieces, which I think fits best. > > [ shrug ] I don't care --- if you do, then do that. > > I finally realized exactly what was bugging me about "dbfile_size": it > seems to imply that we are measuring the size of one *file*, which is > under no circumstance the definition of any of these functions (see > file splitting behavior for relations exceeding 1GB). Yes, that is an issue I considered. I was more relying on the _idea_ that people thought it was a single file, but that is an implementation detail that shouldn't be promoted. > pg_relation_size plus pg_complete_relation_size is fine. Ship it... OK. -- 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 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] [PATCHES] Dbsize backend integration
Bruce Momjian writes: > Tom Lane wrote: >> I could live with that. Or "pg_total_relation_size". > The problem with "total", to me, is that it already is the total size of > the heap/index/toast. Complete has the idea of adding additional > pieces, which I think fits best. [ shrug ] I don't care --- if you do, then do that. I finally realized exactly what was bugging me about "dbfile_size": it seems to imply that we are measuring the size of one *file*, which is under no circumstance the definition of any of these functions (see file splitting behavior for relations exceeding 1GB). pg_relation_size plus pg_complete_relation_size is fine. Ship it... regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] [PATCHES] Dbsize backend integration
Tom Lane wrote: > Bruce Momjian writes: > > If we go pg_table_size() and pg_relation_size(), which is object-only > > and which is heap + index + toast? I think ideally we want > > pg_relation_size to be the combined one, but then we have pg_table_size > > that works on indexes and toast too, and that is confusing, and we don't > > want to add index and toast versions. Or is an index a relation? And > > TOAST? > > All the backend code thinks so --- anything that has an entry in > pg_class is a relation. So personally I don't find "table" and > "relation" confusing in this context. But I can see it might be > confusing to people not familiar with PG jargon. > > > OK, how about pg_relation_size for heap/index/toast, and > > pg_complete_relation_size for the combined total. > > I could live with that. Or "pg_total_relation_size". The problem with "total", to me, is that it already is the total size of the heap/index/toast. Complete has the idea of adding additional pieces, which I think fits best. -- 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 4: Don't 'kill -9' the postmaster
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Bruce Momjian wrote: > I am not sure what to do with this patch. It is missing dump > capability, there is no clause to disable all triggers on a table, and > it uses a table owner check when a super user check is required (because > of referential integrity). > > Satoshi, are you still working on it? If not does someone else want to > complete it? I realized you just started on it but the deadline is > soon. I've already implemented 'ENABLE/DISABLE TRIGGER ALL', and the superuser check has been added. Please check it. And, I'm going to have a business trip to Sydney this weekend, so I'll complete pg_dump stuffs while my flight. Thank you. -- NAGAYASU Satoshi <[EMAIL PROTECTED]> diff -cr pgsql.orig/src/backend/commands/tablecmds.c pgsql/src/backend/commands/tablecmds.c *** pgsql.orig/src/backend/commands/tablecmds.c 2005-06-28 14:08:54.0 +0900 --- pgsql/src/backend/commands/tablecmds.c 2005-07-01 15:50:27.0 +0900 *** *** 236,241 --- 236,243 Oid newOwnerId); static void ATExecClusterOn(Relation rel, const char *indexName); static void ATExecDropCluster(Relation rel); + static void ATExecEnableDisableTrigger(Relation rel, char *trigname, + bool enable); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, char *tablespacename); static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace); *** *** 1993,1998 --- 1995,2005 } pass = AT_PASS_DROP; break; + case AT_EnableTrig: /* ENABLE TRIGGER */ + case AT_DisableTrig:/* DISABLE TRIGGER */ + ATSimplePermissions(rel, false); + pass = AT_PASS_MISC; + break; case AT_SetTableSpace: /* SET TABLESPACE */ /* This command never recurses */ ATPrepSetTableSpace(tab, rel, cmd->name); *** *** 2155,2160 --- 2162,2173 * Nothing to do here; Phase 3 does the work */ break; + case AT_EnableTrig: /* ENABLE TRIGGER */ + ATExecEnableDisableTrigger(rel, cmd->name, true); + break; + case AT_DisableTrig:/* DISABLE TRIGGER */ + ATExecEnableDisableTrigger(rel, cmd->name, false); + break; default:/* oops */ elog(ERROR, "unrecognized alter table type: %d", (int) cmd->subtype); *** *** 5465,5470 --- 5478,5492 } /* + * ALTER TABLE ENABLE/DISABLE TRIGGER + */ + static void + ATExecEnableDisableTrigger(Relation rel, char *trigname, bool enable) + { + EnableDisableTrigger(rel, trigname, enable); + } + + /* * ALTER TABLE SET TABLESPACE */ static void diff -cr pgsql.orig/src/backend/commands/trigger.c pgsql/src/backend/commands/trigger.c *** pgsql.orig/src/backend/commands/trigger.c 2005-05-30 16:20:58.0 +0900 --- pgsql/src/backend/commands/trigger.c2005-07-04 10:40:27.0 +0900 *** *** 3063,3065 --- 3063,3158 afterTriggerAddEvent(new_event); } } + + /* -- + * EnableDisableTrigger() + * + *Called by ALTER TABLE ENABLE/DISABLE TRIGGER + * to change 'tgenabled' flag in the pg_trigger. + * -- + */ + void + EnableDisableTrigger(Relation rel, const char *tgname, bool enable) + { + Relation tgrel; + SysScanDesc tgscan; + ScanKeyData keys[2]; + HeapTuple tuple; + int nkeys; + int changed; + + /* Permissions checks */ + if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, + RelationGetRelationName(rel)); + + if (!allowSystemTableMods && IsSystemRelation(rel)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), +errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(rel; + + tgrel = heap_open(TriggerRelationId, RowExclusiveLock); + + nkeys = 2; + changed = 0; + if ( strcmp(tgname, "*")==0 ) + { + if ( !superuser() ) + elog(ERROR, "ENABLE/DISABLE TRIGGER ALL is allowed superuser only."); + + nkeys = 1; + } + + ScanKeyInit(&keys[0], +
Re: [HACKERS] [PATCHES] Dbsize backend integration
Bruce Momjian writes: > If we go pg_table_size() and pg_relation_size(), which is object-only > and which is heap + index + toast? I think ideally we want > pg_relation_size to be the combined one, but then we have pg_table_size > that works on indexes and toast too, and that is confusing, and we don't > want to add index and toast versions. Or is an index a relation? And > TOAST? All the backend code thinks so --- anything that has an entry in pg_class is a relation. So personally I don't find "table" and "relation" confusing in this context. But I can see it might be confusing to people not familiar with PG jargon. > OK, how about pg_relation_size for heap/index/toast, and > pg_complete_relation_size for the combined total. I could live with that. Or "pg_total_relation_size". regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Disable page writes when fsync off, add GUC
Bruce Momjian wrote: > This also adds a full_page_writes GUC to turn off page writes to WAL. > Some people might not want full_page_writes. Fsync linkage removed, patch attached and applied. -- 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: doc/src/sgml/runtime.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v retrieving revision 1.335 diff -c -c -r1.335 runtime.sgml *** doc/src/sgml/runtime.sgml 2 Jul 2005 19:16:36 - 1.335 --- doc/src/sgml/runtime.sgml 5 Jul 2005 23:15:33 - *** *** 1660,1666 This option can only be set at server start or in the ! postgresql.conf file. --- 1660,1668 This option can only be set at server start or in the ! postgresql.conf file. If this option ! is off, consider also turning off ! guc-full-page-writes. *** *** 1687,1692 --- 1689,1725 + + +full_page_writes configuration parameter + + full_page_writes (boolean) + + + A page write in process during an operating system crash might + be only partially written to disk, leading to an on-disk page + that contains a mix of old and new data. During recovery, the + row changes stored in WAL are not enough to completely restore + the page. + + + + When this option is on, the PostgreSQL server + writes full pages to WAL when they first modified after a checkpoint + so full recovery is possible. Turning this option off might lead + to a corrupt system after an operating system crash because + uncorrected partial pages might contain inconsistent or corrupt + data. The risks are less but similar to fsync. + + + + This option can only be set at server start or in the + postgresql.conf file. The default is + on. + + + + wal_buffers (integer) Index: src/backend/access/transam/xlog.c === RCS file: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v retrieving revision 1.206 diff -c -c -r1.206 xlog.c *** src/backend/access/transam/xlog.c 4 Jul 2005 04:51:44 - 1.206 --- src/backend/access/transam/xlog.c 5 Jul 2005 23:15:36 - *** *** 103,108 --- 103,109 char *XLogArchiveCommand = NULL; char *XLOG_sync_method = NULL; const charXLOG_sync_method_default[] = DEFAULT_SYNC_METHOD_STR; + bool fullPageWrites = true; #ifdef WAL_DEBUG bool XLOG_DEBUG = false; *** *** 594,600 { /* OK, put it in this slot */ dtbuf[i] = rdt->buffer; ! if (XLogCheckBuffer(rdt, &(dtbuf_lsn[i]), &(dtbuf_xlg[i]))) { dtbuf_bkp[i] = true; rdt->data = NULL; --- 595,603 { /* OK, put it in this slot */ dtbuf[i] = rdt->buffer; ! /* If fsync is off, no need to backup pages. */ ! if (fullPageWrites && ! XLogCheckBuffer(rdt, &(dtbuf_lsn[i]), &(dtbuf_xlg[i]))) { dtbuf_bkp[i] = true; rdt->data = NULL; Index: src/backend/utils/misc/guc.c === RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v retrieving revision 1.272 diff -c -c -r1.272 guc.c *** src/backend/utils/misc/guc.c4 Jul 2005 04:51:51 - 1.272 --- src/backend/utils/misc/guc.c5 Jul 2005 23:15:39 - *** *** 83,88 --- 83,89 extern intCommitDelay; extern intCommitSiblings; extern char *default_tablespace; + extern bool fullPageWrites; static const char *assign_log_destination(const char *value, bool doit, GucSource source); *** *** 483,488 --- 484,501 false, NULL, NULL }, { + {"full_page_writes", PGC_SIGHUP, WAL_SETTINGS, +
Re: [HACKERS] [PATCHES] Dbsize backend integration
Dave Page wrote: > > >>You are into the cycle we were in. We discussed pg_object size (too > > >>vague) and pg_index_size (needs pg_toast_size too, and maybe toast > > >>indexes; too many functions). > > > > > > Yeah, I read those discussions, and think you were better > > off then than you > > > are now, which is why I went back to it somewhat. > > > > To be honest, the amount of effort being expended on this naming > > discussion far outweighs the benefits. Maybe it's time for a core > > member to step in and just resolve it - one way or the other? > > Agreed. The current names were discussed (at some length!) by Bruce & I > before I reworked the latest version of the patch. Can we just settle on > that? If we go pg_table_size() and pg_relation_size(), which is object-only and which is heap + index + toast? I think ideally we want pg_relation_size to be the combined one, but then we have pg_table_size that works on indexes and toast too, and that is confusing, and we don't want to add index and toast versions. Or is an index a relation? And TOAST? OK, how about pg_relation_size for heap/index/toast, and pg_complete_relation_size for the combined total. -- 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 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Dependencies on shared objects
On Tue, Jul 05, 2005 at 04:32:22PM -0400, Tom Lane wrote: > Another question about this: why bother with dependencies on > tablespaces? That seems to me to be isomorphic with dependencies on > databases --- we don't need those either, because in both cases we > count on the filesystem to provide ground truth about which objects > live inside a database/tablespace. Because it appeared ugly to me to require information from the filesystem for something that should be known internally in the database (system catalogs). No other reason that I remember. -- Alvaro Herrera () "You knock on that door or the sun will be shining on places inside you that the sun doesn't usually shine" (en Death: "The High Cost of Living") ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Dependencies on shared objects
Another question about this: why bother with dependencies on tablespaces? That seems to me to be isomorphic with dependencies on databases --- we don't need those either, because in both cases we count on the filesystem to provide ground truth about which objects live inside a database/tablespace. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Dependencies on shared objects
Alvaro Herrera <[EMAIL PROTECTED]> writes: > On Tue, Jul 05, 2005 at 02:47:15PM -0400, Tom Lane wrote: >> Although I don't have any particular objection to the OWNER/NORMAL >> distinction, your explanation doesn't seem to make sense. Don't you >> have to poke the Acl anyway, if it's non-null? Else the grantor values >> will be wrong. > Hum, we don't register a dependency on the owner when registering > dependencies from the Acl -- we actively skip that (because we know the > owner has an entry of the other type already). Is this still an issue? Not sure. ISTM the distinction we want to capture is more along the lines of DEPENDENCY_NORMAL vs DEPENDENCY_AUTO --- that is, we should be willing to auto-drop grants to a user when dropping the user, but not be willing to auto-drop objects unless the drop is with CASCADE. Also, grants from the user to someone else (using grant options) probably shouldn't go away without CASCADE either, though this is maybe debatable. If you believe the latter then the OWNER/ACL division is clearly the wrong way to think about it. So I'd be inclined to use the NORMAL/AUTO terminology instead. On the other hand: we might need to be able to express that "this object's ACL depends on that user" as opposed to "this object depends on that user" --- if you're really using the dependency type as a flag of this kind, then we might have to keep it. I've not gotten that far in reading the patch yet. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Autovacuum integration patch
While we are at it (assuming the autovacuum patch gets included in 8.1) we have a few autovacuum related todo items. These are the ones I can think of right now: * XID Wraparound improvement, moving to per-table vacuuming rather than per database. (8.2) * Alter table commands to set per table autovacuum threshold settings. (8.2) * Incorporate FSM data to improve vacuum decision making (8.2) * Deal with stats reset better, possibly force stats reset to false if autovacuum is enabled. (8.2) * Add the concept of a maintenance window to autovacuum. (maybe 8.1?) * Have the VACUUM and ANALYZE commands update the pg_autovacuum table itself. This will allow autovacuum to work in harmony with manually issued VACUUM's. (I would like to see this done for 8.1, but I will understand if people demand that it wait for 8.2) * Add some regression tests? Not sure what would be appropriate here. (8.1) * Improve autovacuum threshold defaults (8.1) Anyone have anything to add to the list? Matthew Bruce Momjian wrote: TODO item? ---(end of broadcast)--- TIP 3: 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] Dependencies on shared objects
On Tue, Jul 05, 2005 at 02:47:15PM -0400, Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > I attach a patch to implement dependencies on shared objects. > > As some of you may remember, the purpose of this patch is to record > > dependencies on shared objects, such as roles and tablespaces, from > > regular database objects. This is done on a new shared system catalog > > called pg_shdepend, so that when a backend wants to drop any shared > > object, it can easily verify whether it is referenced in other database. > > Will work on applying this next. Cool. > > - added a dependency type. There are three types: PIN, same as normal > > dependencies; OWNER, for roles that own objects; NORMAL, all the rest > > (roles in the Acl and tablespaces). > > I needed to separate the OWNER entries to support changing ownership > > of objects without having to poke the whole Acl for the object. > > Although I don't have any particular objection to the OWNER/NORMAL > distinction, your explanation doesn't seem to make sense. Don't you > have to poke the Acl anyway, if it's non-null? Else the grantor values > will be wrong. Hum, we don't register a dependency on the owner when registering dependencies from the Acl -- we actively skip that (because we know the owner has an entry of the other type already). Is this still an issue? -- Alvaro Herrera () "La verdad no siempre es bonita, pero el hambre de ella sí" ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Autovacuum integration patch
Matthew T. O'Connor wrote: > I think so. Something like: Improve autovacuum xid wraparound detection > by moving to a pertable solution rather than per database. Thanks, added. -- 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 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Autovacuum integration patch
I think so. Something like: Improve autovacuum xid wraparound detection by moving to a pertable solution rather than per database. Matt Bruce Momjian wrote: TODO item? ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Dependencies on shared objects
Alvaro Herrera <[EMAIL PROTECTED]> writes: > I attach a patch to implement dependencies on shared objects. > As some of you may remember, the purpose of this patch is to record > dependencies on shared objects, such as roles and tablespaces, from > regular database objects. This is done on a new shared system catalog > called pg_shdepend, so that when a backend wants to drop any shared > object, it can easily verify whether it is referenced in other database. Will work on applying this next. > - added a dependency type. There are three types: PIN, same as normal > dependencies; OWNER, for roles that own objects; NORMAL, all the rest > (roles in the Acl and tablespaces). > I needed to separate the OWNER entries to support changing ownership > of objects without having to poke the whole Acl for the object. Although I don't have any particular objection to the OWNER/NORMAL distinction, your explanation doesn't seem to make sense. Don't you have to poke the Acl anyway, if it's non-null? Else the grantor values will be wrong. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Autovacuum integration patch
TODO item? --- Alvaro Herrera wrote: > On Tue, Jul 05, 2005 at 01:00:50PM -0400, Tom Lane wrote: > > "Matthew T. O'Connor" writes: > > > Tom Lane wrote: > > >> No, you're wrong. VACUUMing of individual tables is perfectly good > > >> enough as far as XID wrap protection goes, it's just that we chose to > > >> track whether it had been done at the database level. If we tracked it > > >> in, say, a new pg_class column then in principle you could protect > > >> against XID wrap with only table-at-a-time VACUUMs. > > > > > Good, I'm glad I'm wrong on this. This will be another nice advantage > > > of autovacuum then and should be fairly easy to do. Any thoughts on > > > this being a change we can get in for 8.1? > > > > I'd say this is probably a tad too late --- there's a fair amount of > > code change that would be needed, none of which has been written, and > > we are past the feature-freeze deadline for new code. > > Right. I've written a small, non-intrusive patch that handles the Xid > wraparound just as pg_autovacuum used to, checking the Xid from > pg_database. > > -- > Alvaro Herrera () > "Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans) > > ---(end of broadcast)--- > TIP 4: Don't 'kill -9' the postmaster > -- 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 7: don't forget to increase your free space map settings
Re: [PATCHES] Python setof patch
OK, patch backed out, new and regression file removed. --- Andrew Dunstan wrote: > > > Michael Fuhr wrote: > > >On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote: > > > > > >>Aside from minor problems like being broken and undocumented, there is a > >>more serious question here: is this even the functionality we want? > >> > >> > > > >I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's > >return_next. > > > > > > > > Agreed. My rudimentary testing shows that plperl's shiny new return_next > functionality not only avoids memory blowup but has a 40% speed > improvement over the old 'return the lot at once' API. > > cheers > > andrew > > ---(end of broadcast)--- > TIP 7: don't forget to increase your free space map settings > -- 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/pl/plpython/plpython.c === RCS file: /cvsroot/pgsql/src/pl/plpython/plpython.c,v retrieving revision 1.62 retrieving revision 1.63 diff -c -r1.62 -r1.63 *** src/pl/plpython/plpython.c 6 May 2005 17:24:55 - 1.62 --- src/pl/plpython/plpython.c 4 Jul 2005 18:59:42 - 1.63 *** *** 29,35 * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. * * IDENTIFICATION ! *$PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.62 2005/05/06 17:24:55 tgl Exp $ * * */ --- 29,35 * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. * * IDENTIFICATION ! *$PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.63 2005/07/04 18:59:42 momjian Exp $ * * */ *** *** 286,291 --- 286,294 static PyObject *PLy_exc_fatal = NULL; static PyObject *PLy_exc_spi_error = NULL; + /* End-of-set Indication */ + static PyObject *PLy_endofset = NULL; + /* some globals for the python module */ static char PLy_plan_doc[] = { *** *** 770,775 --- 773,788 fcinfo->isnull = true; rv = (Datum) NULL; } + /* test for end-of-set condition */ + else if (fcinfo->flinfo->fn_retset && plrv == PLy_endofset) + { + ReturnSetInfo *rsi; + + fcinfo->isnull = true; + rv = (Datum)NULL; + rsi = (ReturnSetInfo *)fcinfo->resultinfo; + rsi->isDone = ExprEndResult; + } else { fcinfo->isnull = false; *** *** 2317,2325 --- 2330,2340 PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL); PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL); PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL); + PLy_endofset = PyErr_NewException("plpy.EndOfSet",NULL,NULL); PyDict_SetItemString(plpy_dict, "Error", PLy_exc_error); PyDict_SetItemString(plpy_dict, "Fatal", PLy_exc_fatal); PyDict_SetItemString(plpy_dict, "SPIError", PLy_exc_spi_error); + PyDict_SetItemString(plpy_dict, "EndOfSet", PLy_endofset); /* * initialize main module, and add plpy ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Python setof patch
Michael Fuhr wrote: On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote: Aside from minor problems like being broken and undocumented, there is a more serious question here: is this even the functionality we want? I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's return_next. Agreed. My rudimentary testing shows that plperl's shiny new return_next functionality not only avoids memory blowup but has a 40% speed improvement over the old 'return the lot at once' API. cheers andrew ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Python setof patch
On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote: > > Aside from minor problems like being broken and undocumented, there is a > more serious question here: is this even the functionality we want? I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's return_next. -- Michael Fuhr http://www.fuhr.org/~mfuhr/ ---(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] Autovacuum integration patch
On Tue, Jul 05, 2005 at 01:00:50PM -0400, Tom Lane wrote: > "Matthew T. O'Connor" writes: > > Tom Lane wrote: > >> No, you're wrong. VACUUMing of individual tables is perfectly good > >> enough as far as XID wrap protection goes, it's just that we chose to > >> track whether it had been done at the database level. If we tracked it > >> in, say, a new pg_class column then in principle you could protect > >> against XID wrap with only table-at-a-time VACUUMs. > > > Good, I'm glad I'm wrong on this. This will be another nice advantage > > of autovacuum then and should be fairly easy to do. Any thoughts on > > this being a change we can get in for 8.1? > > I'd say this is probably a tad too late --- there's a fair amount of > code change that would be needed, none of which has been written, and > we are past the feature-freeze deadline for new code. Right. I've written a small, non-intrusive patch that handles the Xid wraparound just as pg_autovacuum used to, checking the Xid from pg_database. -- Alvaro Herrera () "Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans) ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Python setof patch
Michael Fuhr <[EMAIL PROTECTED]> writes: >> This patch allows the PL/Python module to do (SRF) functions. > Does this patch work? Aside from minor problems like being broken and undocumented, there is a more serious question here: is this even the functionality we want? The proposed test case is: CREATE or replace FUNCTION test_setof() returns setof text AS 'if GD.has_key("calls"): GD["calls"] = GD["calls"] + 1 if GD["calls"] > 2: del GD["calls"] return plpy.EndOfSet else: GD["calls"] = 1 return str(GD["calls"])' LANGUAGE plpythonu; which is essentially exposing the old value-per-call SRF implementation to the Python programmer. Do we really want to do that? plperl and plpgsql have not taken that tack. One reason this doesn't seem a particularly great idea is that the above example would likely fail if invoked twice in one query, due to it using only one global state variable for both invocations. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Autovacuum integration patch
"Matthew T. O'Connor" writes: > Tom Lane wrote: >> No, you're wrong. VACUUMing of individual tables is perfectly good >> enough as far as XID wrap protection goes, it's just that we chose to >> track whether it had been done at the database level. If we tracked it >> in, say, a new pg_class column then in principle you could protect >> against XID wrap with only table-at-a-time VACUUMs. > Good, I'm glad I'm wrong on this. This will be another nice advantage > of autovacuum then and should be fairly easy to do. Any thoughts on > this being a change we can get in for 8.1? I'd say this is probably a tad too late --- there's a fair amount of code change that would be needed, none of which has been written, and we are past the feature-freeze deadline for new code. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Autovacuum integration patch
Tom Lane wrote: "Matthew T. O'Connor" writes: The current implementation of XID wraparound requires that the vacuum command be run against the entire database, you can not run it on a per table basis and have it work. At least that is my understanding, No, you're wrong. VACUUMing of individual tables is perfectly good enough as far as XID wrap protection goes, it's just that we chose to track whether it had been done at the database level. If we tracked it in, say, a new pg_class column then in principle you could protect against XID wrap with only table-at-a-time VACUUMs. (I think you'd still want the pg_database column, but you'd update it to be the minimum of the per-table values at the completion of any VACUUM.) At the time this didn't seem particularly worth the complication since no one would be likely to try to do that manually --- but with autovacuum handling the work, it starts to sound more realistic. Good, I'm glad I'm wrong on this. This will be another nice advantage of autovacuum then and should be fairly easy to do. Any thoughts on this being a change we can get in for 8.1? Matt ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Autovacuum integration patch
"Matthew T. O'Connor" writes: >>> Hmm. Yes, this patch doesn't handle Xid wraparound. This should be >>> easy to add though. Anyway, I was thinking that we could add a "last >>> vacuum Xid" to pg_autovacuum, and handle Xid wraparound for each table >>> separately -- this means you don't have to issue huge whole-database >>> VACUUMs, because it will be handled nicely for each table. Storing the >>> last vacuum Xid in pg_database would have to be rethought. > The current implementation of XID wraparound requires that the vacuum > command be run against the entire database, you can not run it on a per > table basis and have it work. At least that is my understanding, No, you're wrong. VACUUMing of individual tables is perfectly good enough as far as XID wrap protection goes, it's just that we chose to track whether it had been done at the database level. If we tracked it in, say, a new pg_class column then in principle you could protect against XID wrap with only table-at-a-time VACUUMs. (I think you'd still want the pg_database column, but you'd update it to be the minimum of the per-table values at the completion of any VACUUM.) At the time this didn't seem particularly worth the complication since no one would be likely to try to do that manually --- but with autovacuum handling the work, it starts to sound more realistic. regards, tom lane ---(end of broadcast)--- TIP 3: 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] INSERT ... RETURNING
[EMAIL PROTECTED] writes: > Attached is a patch (by Gavin Sherry, fixed up to apply to 8.1 by me) that > implements INSERT ... RETURNING functionality. > It does work for the common case of RETURNING the value of a serial/sequence > column, but gets confused when returning results out-of-order (CREATE TABLE x > (a int, b int), INSERT ... RETURNING b, a) and doesn't let you specify the > same > column multiple times (INSERT ... RETURNING b, b). These will be addressed > soon. This is pretty considerably shy of what I thought had been agreed to anyway: - should allow expressions not only column names - should work for UPDATE and DELETE too regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Python setof patch
Michael Fuhr <[EMAIL PROTECTED]> writes: > I'm running HEAD (8.1devel), which is where the patch was committed. > Could somebody else test HEAD and see what they get? Same as you --- ie, this patch is utterly broken. > I don't see the setof functionality in the regression tests, > presumably because there's no expected/plpython_setof.sql file: That's because it wasn't added to the Makefile. However, a test that only defines a function and doesn't actually exercise it seems a bit useless anyway :-( regards, tom lane ---(end of broadcast)--- TIP 3: 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] Error handling fix in interfaces/libpq/fe-secure.c
BTW, I read at http://www.gnu.org/software/libc/manual/html_node/Translation-with-gettext.html The gettext function does not modify the value of the global errno variable. This is necessary to make it possible to write something like printf (gettext ("Operation failed: %m\n")); which is pretty much what I expected to find. Ergo, this entire discussion is wrong, and whatever bug you are concerned about will not be solved this way. What you may actually be running into is the problem that there are two different definitions of strerror_r() out there, the SUS spec and the GNU spec, and pre-8.0 we failed to distinguish these. The 7.4 coding will yield garbage messages in some cases when GNU strerror_r is in use. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Python setof patch
On Tue, Jul 05, 2005 at 04:23:42PM +0200, Gerrit van Dyk wrote: > > Ok, I just looked at the code again and the results I am getting, is > what you expect. > > 2 rows every time returning 1 and 2. > > What version of postgres are you running, I am running 8.0.3 I'm running HEAD (8.1devel), which is where the patch was committed. Could somebody else test HEAD and see what they get? -- Michael Fuhr http://www.fuhr.org/~mfuhr/ ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Error handling fix in interfaces/libpq/fe-secure.c
[EMAIL PROTECTED] writes: > Here's another one similar to what I described in my previous message. More or less proves my point, no? Even if you manage to fix every occurence of this issue now, it'll keep popping up as people change the code. This approach is just not maintainable. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] patch: garbage error strings in libpq
[EMAIL PROTECTED] writes: > Another approach would have been to make libpq_gettext() preserve errno. That seems like a far easier, cleaner, and more robust fix than this. Moreover I don't believe that this approach works either, as the result of strerror() is not guaranteed still usable after another strerror call (ie, it can use a static buffer repeatedly), so you'd still have the problem if libpq_gettext invokes strerror. I suppose that a really robust solution would involve libpq_gettext saving errno, restoring errno, and invoking strerror() again ... regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] pgcrypto volatility and strictness changes
This patch updates the DDL for contrib/pgcrypto to create all functions as STRICT, and all functions except gen_salt() as IMMUTABLE. gen_salt() is VOLATILE. Although the functions are now STRICT, I left their PG_ARGISNULL() checks in place as a protective measure for users who install the new code but use old (non-STRICT) catalog entries (e.g., restored from a dump). Per recent discussion in pgsql-hackers. -- Michael Fuhr http://www.fuhr.org/~mfuhr/ Index: contrib/pgcrypto/pgcrypto.sql.in === RCS file: /projects/cvsroot/pgsql/contrib/pgcrypto/pgcrypto.sql.in,v retrieving revision 1.9 diff -c -r1.9 pgcrypto.sql.in *** contrib/pgcrypto/pgcrypto.sql.in14 May 2003 03:25:56 - 1.9 --- contrib/pgcrypto/pgcrypto.sql.in5 Jul 2005 14:11:45 - *** *** 4,74 CREATE OR REPLACE FUNCTION digest(text, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_digest' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION digest(bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_digest' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION digest_exists(text) RETURNS bool AS 'MODULE_PATHNAME', 'pg_digest_exists' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION hmac(text, text, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_hmac' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION hmac(bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_hmac' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION hmac_exists(text) RETURNS bool AS 'MODULE_PATHNAME', 'pg_hmac_exists' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION crypt(text, text) RETURNS text AS 'MODULE_PATHNAME', 'pg_crypt' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION gen_salt(text) RETURNS text AS 'MODULE_PATHNAME', 'pg_gen_salt' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION gen_salt(text, int4) RETURNS text AS 'MODULE_PATHNAME', 'pg_gen_salt_rounds' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION encrypt(bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_encrypt' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION decrypt(bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_decrypt' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION encrypt_iv(bytea, bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_encrypt_iv' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION decrypt_iv(bytea, bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_decrypt_iv' ! LANGUAGE 'C'; CREATE OR REPLACE FUNCTION cipher_exists(text) RETURNS bool AS 'MODULE_PATHNAME', 'pg_cipher_exists' ! LANGUAGE 'C'; --- 4,74 CREATE OR REPLACE FUNCTION digest(text, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_digest' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION digest(bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_digest' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION digest_exists(text) RETURNS bool AS 'MODULE_PATHNAME', 'pg_digest_exists' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION hmac(text, text, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_hmac' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION hmac(bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_hmac' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION hmac_exists(text) RETURNS bool AS 'MODULE_PATHNAME', 'pg_hmac_exists' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION crypt(text, text) RETURNS text AS 'MODULE_PATHNAME', 'pg_crypt' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION gen_salt(text) RETURNS text AS 'MODULE_PATHNAME', 'pg_gen_salt' ! LANGUAGE 'C' VOLATILE STRICT; CREATE OR REPLACE FUNCTION gen_salt(text, int4) RETURNS text AS 'MODULE_PATHNAME', 'pg_gen_salt_rounds' ! LANGUAGE 'C' VOLATILE STRICT; CREATE OR REPLACE FUNCTION encrypt(bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_encrypt' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION decrypt(bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_decrypt' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION encrypt_iv(bytea, bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_encrypt_iv' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION decrypt_iv(bytea, bytea, bytea, text) RETURNS bytea AS 'MODULE_PATHNAME', 'pg_decrypt_iv' ! LANGUAGE 'C' IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION cipher_exists(text) RETURNS bool AS 'MODULE_PATHNAME', 'pg_cipher_exists' ! LANGUAGE 'C' IMMUTABLE STRICT; ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Python setof patch
On Mon, Jul 04, 2005 at 03:04:51PM -0400, Bruce Momjian wrote: > > Patch applied. Thanks. > > Gerrit van Dyk wrote: > > > > This patch allows the PL/Python module to do (SRF) functions. Does this patch work? The test_setof() function in sql/plpython_setof.sql gives me the following: SELECT * FROM test_setof(); test_setof 1 (1 row) If I call the function again I get this: SELECT * FROM test_setof(); test_setof 2 (1 row) Calling the function a third time gives this: SELECT * FROM test_setof(); test_setof (0 rows) Am I misreading the code, or shouldn't the function return two rows on each invocation? I don't see the setof functionality in the regression tests, presumably because there's no expected/plpython_setof.sql file: == running regression test queries== test plpython_schema ... ok test plpython_populate... ok test plpython_function... ok test plpython_test... ok test plpython_error ... ok test plpython_drop... ok = All 6 tests passed. = What about documentation updates? Still in the works? -- Michael Fuhr http://www.fuhr.org/~mfuhr/ ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] Error handling fix in interfaces/libpq/fe-secure.c
Here's another one similar to what I described in my previous message. In libpq's pqsecure_read(), if SSL_read() returns -1 and sets an error of SSL_ERROR_SYSCALL, errno may be polluted by libpq_gettext() before a human-readable string is derived from it. Also, pqReadData() will see the wrong errno value after the call. The attached patch fixes both by introducing a named variable to hold the significant value of errno. Jeroen --- fe-secure.c.org 2005-07-05 19:45:19.0 +0700 +++ fe-secure.c 2005-07-05 19:55:26.0 +0700 @@ -340,9 +340,13 @@ char sebuf[256]; if (n == -1) + { + const int errcode = SOCK_ERRNO; printfPQExpBuffer(&conn->errorMessage, libpq_gettext("SSL SYSCALL error: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + SOCK_STRERROR(errcode, sebuf, sizeof(sebuf))); + SOCK_ERRNO_SET(errcode); + } else { printfPQExpBuffer(&conn->errorMessage, ---(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
[PATCHES] patch: garbage error strings in libpq
Several libpqxx users have been reporting odd problems with certain error messages generated by libpq. One of them was the inclusion of garbage data. As it turns out, src/interfaces/libpq/fe-misc.c contains several instances of this construct: printfPQExpBuffer(&conn->ErrorMessage, libpq_gettext("error: %s"), SOCK_STRERROR(SOCK_ERRNO, buffer, sizeof(buffer))); This may occur in other source files as well. On Unix-like systems, SOCK_ERRNO defines to plain errno--which is likely to be overwritten by the libpq_gettext(). I'm attaching a patch that fixes these instances by introducing a named pointer to the SOCK_STRERROR message, initialized before either of the other function calls. Another approach would have been to make libpq_gettext() preserve errno. It's tempting, but I'm not sure it would be valid from a language-lawyer point of view. There is no sequence point between the evaluations of libpq_gettext() and SOCK_STRERROR(). From what I vaguely remember hearing somewhere in the distant past, that means that theoretically they may be evaluated not just in any order but even in parallel. I guess it may actually happen if both inlining and scheduling are sufficiently aggressive. Even if libpq_gettext() is made to restore errno, it will still have to pollute errno at some points during its execution. Jeroen --- fe-misc.c.org 2005-07-05 17:48:25.0 +0700 +++ fe-misc.c 2005-07-05 18:13:03.0 +0700 @@ -23,7 +23,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.113 2005/02/22 04:42:20 momjian Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-misc.c,v 1.114 2005/06/12 00:00:21 neilc Exp $ * *- */ @@ -175,7 +175,8 @@ conn->inCursor += len; if (conn->Pfdebug) - fprintf(conn->Pfdebug, libpq_gettext("From backend (%lu)> %.*s\n"), (unsigned long) len, (int) len, s); + fprintf(conn->Pfdebug, libpq_gettext("From backend (%lu)> %.*s\n"), +(unsigned long) len, (int) len, s); return 0; } @@ -590,6 +591,7 @@ conn->inBufSize - conn->inEnd); if (nread < 0) { + const char *errstr; if (SOCK_ERRNO == EINTR) goto retry3; /* Some systems return EAGAIN/EWOULDBLOCK for no data */ @@ -606,9 +608,10 @@ if (SOCK_ERRNO == ECONNRESET) goto definitelyFailed; #endif + errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not receive data from server: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + errstr); return -1; } if (nread > 0) @@ -681,6 +684,7 @@ conn->inBufSize - conn->inEnd); if (nread < 0) { + const char *errstr; if (SOCK_ERRNO == EINTR) goto retry4; /* Some systems return EAGAIN/EWOULDBLOCK for no data */ @@ -697,9 +701,10 @@ if (SOCK_ERRNO == ECONNRESET) goto definitelyFailed; #endif + errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not receive data from server: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + errstr); return -1; } if (nread > 0) @@ -759,6 +764,7 @@ if (sent < 0) { + const char *errstr; /* * Anything except EAGAIN/EWOULDBLOCK/EINTR is trouble. If * it's EPIPE or ECONNRESET, assume we've lost the backend @@ -799,9 +805,10 @@ return -1; default: + errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not send data to server: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + errstr); /* We don't assume it's a fatal error... */ conn->outCount = 0; return -1; @@ -986,10 +993,12 @@ if (result < 0) { char sebuf[256]; + const char *errstr; + errstr = SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("select() failed: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); + errstr); } return result; ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] [PATCH] pgcrypto: pgp_encrypt v3
On Tue, Jul 05, 2005 at 10:20:17AM +1000, Neil Conway wrote: > Bruce Momjian wrote: > >Your patch has been added to the PostgreSQL unapplied patches list > > That is not the latest version of Marko's patch. Bruce got v3, thats indeed the latest. Also, http://momjian.postgresql.org/cgi-bin/pgpatches shows v3. > But in any case, the > patch is not yet ready for application: > > http://archives.postgresql.org/pgsql-patches/2005-07/msg00077.php Now I did fresh rebuild of CVS and played with it. Result is that it is only partly my error. It just happens that I did initdb with option '-E unicode'. Now, can anybody explain the following difference: $ psql -c '\l' template1; psql -c "select 'a\nxx'::text as x;" template1 List of databases Name| Owner | Encoding +---+--- contrib_regression | marko | SQL_ASCII postgres | marko | SQL_ASCII template0 | marko | SQL_ASCII template1 | marko | SQL_ASCII (4 rows) x -- a xx (1 row) $ psql -c '\l' template1; psql -c "select 'a\nxx'::text as x;" template1 List of databases Name| Owner | Encoding +---+-- contrib_regression | marko | UTF8 postgres | marko | UTF8 template0 | marko | UTF8 template1 | marko | UTF8 (4 rows) x --- a xx (1 row) = I can send new regression test for SQL_ASCII, but it still would not work for all users. -- marko ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] enable/disable trigger (Re: Fwd: [HACKERS] Open items)
Bruce Momjian wrote: I am not sure what to do with this patch. It is missing dump capability, there is no clause to disable all triggers on a table, and it uses a table owner check when a super user check is required (because of referential integrity). From a user's view, a trigger implementing RI isn't a trigger but an implementation detail he shouldn't need to care about. So for std triggers, owner check should be sufficient, requiring superuser for RI triggers only. This impacts EN/DISABLE TRIGGER ALL too. To touch RI triggers as well, an additional keyword is needed. Regards, Andreas ---(end of broadcast)--- TIP 3: 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: [HACKERS] [PATCHES] Dbsize backend integration
> -Original Message- > From: Christopher Kings-Lynne [mailto:[EMAIL PROTECTED] > Sent: 05 July 2005 02:39 > To: Robert Treat > Cc: Bruce Momjian; Dave Page; Tom Lane; Dawid Kuroczko; > Andreas Pflug; PostgreSQL-patches; PostgreSQL-development > Subject: Re: [HACKERS] [PATCHES] Dbsize backend integration > > >>You are into the cycle we were in. We discussed pg_object size (too > >>vague) and pg_index_size (needs pg_toast_size too, and maybe toast > >>indexes; too many functions). > > > > Yeah, I read those discussions, and think you were better > off then than you > > are now, which is why I went back to it somewhat. > > To be honest, the amount of effort being expended on this naming > discussion far outweighs the benefits. Maybe it's time for a core > member to step in and just resolve it - one way or the other? Agreed. The current names were discussed (at some length!) by Bruce & I before I reworked the latest version of the patch. Can we just settle on that? Regards, Dave. ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]