Re: [HACKERS] [PATCH] "\ef " in psql
I wrote: > * the psql command seemed to have some ideas about supplying a blank > CREATE OR REPLACE FUNCTION command for a nonexistent function, but this > didn't actually work. In any case it seemed poorly thought out, because > you'd really need to pay some attention to *why* the regproc/regprocedure > lookup failed. I just ripped it out for the moment. I'm not averse to > the concept, if you can get it implemented properly. While I was out at dinner, the obvious solution presented itself: define \ef with no argument as being the command that presents an empty CREATE FUNCTION command template to fill in. This isn't any more or less typing than where I think you were going, and it eliminates all the ambiguity about whether you meant to type a nonexistent function name or just mistyped. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] "\ef " in psql
"Brendan Jurd" <[EMAIL PROTECTED]> writes: > On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane <[EMAIL PROTECTED]> wrote: >> ... I changed >> the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes >> the command to wait in the query buffer. > The principle of least astonishment suggests that \ef should behave in > the same way as \e. Quite. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Fwd: [HACKERS] [Patch Review] TRUNCATE Permission
Updated patch attached, based on comments from Ryan Bradetich and Tom Lane, and sync'd to latest CVS version. ...Robert On Mon, Sep 1, 2008 at 9:33 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > "Ryan Bradetich" <[EMAIL PROTECTED]> writes: >> On Mon, Sep 1, 2008 at 1:00 PM, Tom Lane <[EMAIL PROTECTED]> wrote: >>> [ something about "your patch" ] > >> This is Robert Haas's patch for the September 2008 commit-fest. >> I am just offering my review. > > Sorry about that, I got confused by the reply-to-a-reply. > >> Does my first suggestion still make sense for removing the TRUNCATE in >> pg_class_aclmask() when pg_Authid.rolcatupdate is not set? > > Probably. AFAICS it should be treated exactly like ACL_DELETE, so > anyplace that acl-whacking code is doing something for ACL_DELETE and > the patch doesn't add in ACL_TRUNCATE, I'd be suspicious ... > >regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > Index: doc/src/sgml/ddl.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ddl.sgml,v retrieving revision 1.82 diff -c -r1.82 ddl.sgml *** doc/src/sgml/ddl.sgml 9 May 2008 23:32:03 - 1.82 --- doc/src/sgml/ddl.sgml 6 Sep 2008 03:07:12 - *** *** 1356,1362 There are several different privileges: SELECT, INSERT, UPDATE, DELETE, !REFERENCES, TRIGGER, CREATE, CONNECT, TEMPORARY, EXECUTE, and USAGE. The privileges applicable to a particular --- 1356,1362 There are several different privileges: SELECT, INSERT, UPDATE, DELETE, !TRUNCATE, REFERENCES, TRIGGER, CREATE, CONNECT, TEMPORARY, EXECUTE, and USAGE. The privileges applicable to a particular Index: doc/src/sgml/user-manag.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/user-manag.sgml,v retrieving revision 1.39 diff -c -r1.39 user-manag.sgml *** doc/src/sgml/user-manag.sgml 1 Feb 2007 00:28:18 - 1.39 --- doc/src/sgml/user-manag.sgml 6 Sep 2008 03:07:13 - *** *** 293,299 granted. There are several different kinds of privilege: SELECT, INSERT, UPDATE, DELETE, !REFERENCES, TRIGGER, CREATE, CONNECT, TEMPORARY, EXECUTE, and USAGE. For more information on the different types of privileges supported by --- 293,299 granted. There are several different kinds of privilege: SELECT, INSERT, UPDATE, DELETE, !TRUNCATE, REFERENCES, TRIGGER, CREATE, CONNECT, TEMPORARY, EXECUTE, and USAGE. For more information on the different types of privileges supported by Index: doc/src/sgml/ref/grant.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/grant.sgml,v retrieving revision 1.70 diff -c -r1.70 grant.sgml *** doc/src/sgml/ref/grant.sgml 3 Jul 2008 15:59:55 - 1.70 --- doc/src/sgml/ref/grant.sgml 6 Sep 2008 03:07:14 - *** *** 20,26 ! GRANT { { SELECT | INSERT | UPDATE | DELETE | REFERENCES | TRIGGER } [,...] | ALL [ PRIVILEGES ] } ON [ TABLE ] tablename [, ...] TO { [ GROUP ] rolename | PUBLIC } [, ...] [ WITH GRANT OPTION ] --- 20,26 ! GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER } [,...] | ALL [ PRIVILEGES ] } ON [ TABLE ] tablename [, ...] TO { [ GROUP ] rolename | PUBLIC } [, ...] [ WITH GRANT OPTION ] *** *** 193,198 --- 193,208 + TRUNCATE + + +Allows on +the specified table. + + + + + REFERENCES *** *** 421,428 => \z mytable Access privileges Schema | Name | Type | Access privileges ! +-+---+-- ! public | mytable | table | miriam=arwdxt/miriam : =r/miriam : admin=arw/miriam (1 row) --- 431,438 => \z mytable Access privileges Schema | Name | Type | Access privileges ! +-+---+--- ! public | mytable | table | miriam=arwdDxt/miriam : =r/miriam : admin=arw/miriam (1 row) *** *** 436,441 --- 446,452 w -- UPDATE ("write") a -- INSERT ("append") d -- DELETE + D -- TRUNCATE x -- REFERENCES t -- TRIGGER X -- EXECUTE *** *** 443,449 C -- CREATE c -- CONNECT T -- TEMPORARY ! arwdxt
Re: [HACKERS] [PATCH] "\ef " in psql
On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > > * the way you had it set up, the CREATE OR REPLACE FUNCTION command > would be sent to the backend instantaneously upon return from the > editor, with no opportunity for the user to decide he didn't want his > changes applied. This seemed unacceptably dangerous to me. I changed > the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes > the command to wait in the query buffer. The principle of least astonishment suggests that \ef should behave in the same way as \e. With \e (which I use a lot), the command(s) are immediately executed by the backend as soon as you write and exit from the editor. I don't find that dangerous, and anyone who uses \e will already be very much accustomed to it. If \ef did something different, it would just be weird. If you're not sure you want to execute the contents of your \e editor session after all, you can always delete the semicolon, or everything in the file, before quitting. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New FSM allocation policy
Heikki Linnakangas wrote: > The way my rewritten FSM works at the moment is that pages are handed > out of the FSM in random order, to spread the load of multiple backends > to different pages. That's good for spreading the load, but it occurred > to me while observing a test case that inserts a lot of rows to an > almost empty table with plenty of free space, that that sucks for the > case of a single backend populating a table. Currently, FSM will hand > out pages in sequential order, so from OS point of view we're reading > and writing the pages sequentially. If the pages are handed out in > random order, we'll do random I/O instead. > > Fortunately there's an easy fix for that. If we optimize > RecordAndGetPageWithFreeSpace so that it will always return the next > page if it has enough space, we'll be doing sequential I/O again. That's > trivial as long as the next heap page is on the same FSM page, and > probably not too hard even if it's not. If we limit this optimization to > within the same FSM page, we'll effectively be filling fully a 32MB stripes > > Thoughts? > > I'm running more tests, and there's more issues that need discussion, > but I'll start separate threads for each. I'll also post an updated > patch separately. One other thing to keep in mind is that VACUUM can reduce a table's size if the trailing blocks are empty, so there is some gain if the earlier parts of the table are preferred for inserts. -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL SSL problem
Andriy Bakay escreveu: > I have problems to setup SSL for PostgreSQL server. I did all the steps > which described in the documentation (17.8. Secure TCP/IP Connections > with SSL), but when I try to start the PostgreSQL server the pg_ctl gave > me: "could not start server". And nothing in the logs (I enabled all of > them). I googled around but did not find much. > This is the wrong list to post it; try -general or -admin instead. Also it's not polite cc'ing developers as you did. > After I disable SSL option in postgresql.conf the server is starting > successfully. > There is something wrong with your setup. You don't post what steps you followed. Are you sure there is nothing at the logs? -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] reducing statistics write overhead
Euler Taveira de Oliveira <[EMAIL PROTECTED]> writes: > If you can't afford a 500 msec pgstat time, then you need to make it > tunable. Another ideas are (i) turn on/off pgstat per table or database > and (ii) make the pgstat time tunable per table or database. You can use > the reloptions column to store these info. These workarounds are much > simpler than that you proposed and they're almost for free. For normal usage on-demand dumping would be a really good thing; it'd cut the overhead of having stats on tremendously, especially for people who don't really use 'em. The particular signaling proposed here is bogus, but if Martin can make it work in a cleaner fashion I think it's likely a good idea. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] "\ef " in psql
Abhijit Menon-Sen <[EMAIL PROTECTED]> writes: > I have attached two patches: > - funcdef.diff implements pg_get_functiondef() > - edit.diff implements "\ef function" in psql based on (1). I've applied this with some corrections (mostly around proper quoting) and some outright editorialization: * the psql command seemed to have some ideas about supplying a blank CREATE OR REPLACE FUNCTION command for a nonexistent function, but this didn't actually work. In any case it seemed poorly thought out, because you'd really need to pay some attention to *why* the regproc/regprocedure lookup failed. I just ripped it out for the moment. I'm not averse to the concept, if you can get it implemented properly. * the way you had it set up, the CREATE OR REPLACE FUNCTION command would be sent to the backend instantaneously upon return from the editor, with no opportunity for the user to decide he didn't want his changes applied. This seemed unacceptably dangerous to me. I changed the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes the command to wait in the query buffer. Unfortunately there's no visual indication of that, other than a small change in the prompt status. It'd likely be better if we could get libreadline to redisplay the query buffer contents --- anyone have an idea how to do that? (I have some vague recollection that \e used to work that way, though it definitely fails to do so now.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] reducing statistics write overhead
Martin Pihlak escreveu: > I suspected that, but somehow managed to overlook it :( I guess it was > too tempting to use it. I'll start looking for alternatives. > If you can't afford a 500 msec pgstat time, then you need to make it tunable. Another ideas are (i) turn on/off pgstat per table or database and (ii) make the pgstat time tunable per table or database. You can use the reloptions column to store these info. These workarounds are much simpler than that you proposed and they're almost for free. -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Martijn van Oosterhout wrote: >> I suppose what happens is the original patch comes with design and >> later a newer version is posted with just changes. The commitfest page >> points to the latter, losing former in the archive somewhere. > Hmm, IMO this is a flaw in the commitfest entry for that patch -- it > should point to both. Yeah. What I think we should do here is that the main entry for a patch should point at the primary reference (first submission, or wherever it's best discussed), and then any later updates of the patch should be added as comments, instead of replacing the primary reference. It's been done this way for quite a few patches, but evidently not every one. Also, readers should remember to look through the whole thread in the archives, not just the single article linked to. (Dunno if that would have helped Martijn in this case, but there's often good material in the rest of the thread.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal: new border setting in psql
Gregory Stark wrote: > I wonder if it's worth keeping two variants at all really. Why not just make > psql's native table formatting exactly ReST? Is there any part of it that we > don't like as much as our existing tables? It doubles the number of display lines; a very obvious shortcoming. -- Bruce Momjian <[EMAIL PROTECTED]>http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
Martijn van Oosterhout wrote: > Just one thing though, I picked a random patch and started reading. > However, the commitfest page doesn't link to anywhere that actually > describes *what* the patch is trying to do. Many patches do have the > design and the patch in one page, but some don't. > > I suppose what happens is the original patch comes with design and > later a newer version is posted with just changes. The commitfest page > points to the latter, losing former in the archive somewhere. Hmm, IMO this is a flaw in the commitfest entry for that patch -- it should point to both. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On Thu, Sep 04, 2008 at 09:54:02PM +0100, Simon Riggs wrote: > * coding review - does it follow standard code guidelines? Are there > portability issues? Will it work on Windows/BSD etc? Are there > sufficient comments? > > * code review - does it do what it says, correctly? Just one thing though, I picked a random patch and started reading. However, the commitfest page doesn't link to anywhere that actually describes *what* the patch is trying to do. Many patches do have the design and the patch in one page, but some don't. I suppose what happens is the original patch comes with design and later a newer version is posted with just changes. The commitfest page points to the latter, losing former in the archive somewhere. Have a nice day, -- Martijn van Oosterhout <[EMAIL PROTECTED]> http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] libpq events patch (with sgml docs)
Andrew Chernow escribió: > Alvaro Herrera wrote: >> Andrew Chernow escribió: >>>* PQclear - >>>* free's the memory associated with a PGresult >>>*/ >> >> I'd add a comment here stating why the event name is not deallocated; >> otherwise it just looks like it's being leaked. > > The event name is being deallocated. Doh! Sorry, you're right. In that case, you're missing a NULL result check from strdup() in dupEvents() ;-) -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] reducing statistics write overhead
Tom Lane wrote: > Martin Pihlak <[EMAIL PROTECTED]> writes: >> So, as a simple optimization I am proposing that the file should be >> only written when some backend requests statistics. This would >> significantly reduce the undesired write traffic at the cost of >> slightly slower stats access. > > How necessary is this given the recent fixes to allow the stats file to > be kept on a ramdisk? > Ramdisk helps, but requires additional effort to set up. Also the stats file has a tendency to creep up on you -- as the database evolves the file size gradually increases and suddenly the DBA is left wondering what happened to performance. >> Attached is a WIP patch, which basically implements this: > > This patch breaks deadlock checking and statement_timeout, because > backends already use SIGALRM. You can't just take over that signal. > It's possible that you could get things to work by treating this as an > additional reason for SIGALRM, but that code is unreasonably complex > already. I'd suggest finding some other way. > I suspected that, but somehow managed to overlook it :( I guess it was too tempting to use it. I'll start looking for alternatives. regards, Martin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq events patch (with sgml docs)
Andrew Chernow escribió: > /* >* PQmakeEmptyPGresult >* returns a newly allocated, initialized PGresult with given status. >* If conn is not NULL and status indicates an error, the conn's >* errorMessage is copied. >* >* Note this is exported --- you wouldn't think an application would need >* to build its own PGresults, but this has proven useful in both libpgtcl >* and the Perl5 interface, so maybe it's not so unreasonable. > + * > + * Updated April 2008 - If conn is not NULL, event states will be copied > + * from the PGconn to the created PGresult. >*/ Don't do this either. We don't really need to know when the thing was changed; the comment should just state what the function does. I had folded the last paragraph into the introductory one, but I think you lost that part of my change. > + /* resultalloc the attribute names. The above memcpy has the attr > + * names pointing at the callers provided attDescs memory. > + */ "resultalloc"? Why not just "allocate"? >* PQclear - >*free's the memory associated with a PGresult >*/ I'd add a comment here stating why the event name is not deallocated; otherwise it just looks like it's being leaked. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] reducing statistics write overhead
On Fri, 05 Sep 2008 15:23:18 -0400 Tom Lane <[EMAIL PROTECTED]> wrote: > Martin Pihlak <[EMAIL PROTECTED]> writes: > > So, as a simple optimization I am proposing that the file should be > > only written when some backend requests statistics. This would > > significantly reduce the undesired write traffic at the cost of > > slightly slower stats access. > > How necessary is this given the recent fixes to allow the stats file > to be kept on a ramdisk? From an usability and integration perspective this patch is a nice touch. On demand is a nice feature when used correctly. Sincerely, Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 8.4devel out of memory
>>> Tom Lane <[EMAIL PROTECTED]> wrote: > Also, does the leak still occur if > you just run the query as-is rather than EXPLAIN ANALYZE it? The machine became unresponsive similar to the early symptoms of the apparent memory leak cited in this post: http://archives.postgresql.org/pgsql-bugs/2008-07/msg00105.php It was impossible to kill the offending process, since the connections were unresponsive; we didn't wait for the OOM killer, but rebooted the machine. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] reducing statistics write overhead
Martin Pihlak <[EMAIL PROTECTED]> writes: > So, as a simple optimization I am proposing that the file should be > only written when some backend requests statistics. This would > significantly reduce the undesired write traffic at the cost of > slightly slower stats access. How necessary is this given the recent fixes to allow the stats file to be kept on a ramdisk? > Attached is a WIP patch, which basically implements this: This patch breaks deadlock checking and statement_timeout, because backends already use SIGALRM. You can't just take over that signal. It's possible that you could get things to work by treating this as an additional reason for SIGALRM, but that code is unreasonably complex already. I'd suggest finding some other way. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] libpq events update
On Fri, Sep 5, 2008 at 9:54 AM, Andrew Chernow <[EMAIL PROTECTED]> wrote: > Andrew Chernow wrote: >> >> I think it got confused with the instanceData feature, which has nothing >> to do with the event system and requires public functions. libpqtypes >> happens to use the instanceData functions within its eventproc, but this is >> not a requirement. >> > > I forgot to mention that the instanceData functions should be moved from > libpq-events.h to libpq-fe.h because they are not part of the event system. > I plan on making this change as well, so let me know if you hate it. An updated patch with docs and the above change is on -patches. Should we have sent that here? merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] reducing statistics write overhead
Howdy, The statistics collector currently dumps the stats file at every 500ms. This is a major problem if the file becomes large -- occasionally we've been forced to disable stats collection to cope with it. Another issue is that while the file is frequently written, it is seldom read. Typically once a minute - autovacuum plus possible user initiated stats queries. So, as a simple optimization I am proposing that the file should be only written when some backend requests statistics. This would significantly reduce the undesired write traffic at the cost of slightly slower stats access. Attached is a WIP patch, which basically implements this: Disable periodic writing of the stats file. Introduce new stats message type - PGSTAT_MTYPE_INQUIRY. Backends send this to notify collector that stats is needed. Pid of the requestor is provided in the message. Backend then installs an alarm handler and starts a timer. Collector processes the messages and compiles a list of pids to be notified. If there are any, the stats file is written and SIGALRM is sent to the requestors. Backend then proceeds to read the stats file a usual. Thoughts, comments? regards, Martin Index: src/backend/postmaster/pgstat.c === RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/pgstat.c,v retrieving revision 1.181 diff -c -r1.181 pgstat.c *** src/backend/postmaster/pgstat.c 25 Aug 2008 18:55:43 - 1.181 --- src/backend/postmaster/pgstat.c 5 Sep 2008 18:36:24 - *** *** 85,90 --- 85,92 #define PGSTAT_SELECT_TIMEOUT 2 /* How often to check for postmaster * death; in seconds. */ + #define PGSTAT_WRITE_TIMEOUT 5 /* How long to wait for the collector + * to finish writing the file; in seconds. */ /* -- * The initial size hints for the hash tables used in the collector. *** *** 94,100 #define PGSTAT_TAB_HASH_SIZE 512 #define PGSTAT_FUNCTION_HASH_SIZE 512 - /* -- * GUC parameters * -- --- 96,101 *** *** 201,208 --- 202,213 */ static PgStat_GlobalStats globalStats; + /* Last time we wrote the stats file */ + static TimestampTz last_statwrite; + static volatile bool need_exit = false; static volatile bool need_statwrite = false; + static volatile bool stats_ready = false; static volatile bool got_SIGHUP = false; /* *** *** 213,218 --- 218,229 static instr_time total_func_time; + #define PGSTAT_MAX_INQUIRIES 16 /* Max number of outstanding stats inquiries */ + + /* List of backends that need notifications */ + static pid_t inquiring_backends[PGSTAT_MAX_INQUIRIES]; + static int num_inquiries = 0; + /* -- * Local function forward declarations * -- *** *** 223,229 NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]); static void pgstat_exit(SIGNAL_ARGS); ! static void force_statwrite(SIGNAL_ARGS); static void pgstat_beshutdown_hook(int code, Datum arg); static void pgstat_sighup_handler(SIGNAL_ARGS); --- 234,240 NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]); static void pgstat_exit(SIGNAL_ARGS); ! static void pgstat_alarm_handler(SIGNAL_ARGS); static void pgstat_beshutdown_hook(int code, Datum arg); static void pgstat_sighup_handler(SIGNAL_ARGS); *** *** 254,259 --- 265,271 static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len); static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len); static void pgstat_recv_funcpurge(PgStat_MsgFuncpurge *msg, int len); + static void pgstat_recv_inquiry(PgStat_MsgInquiry *msg, int len); /* *** *** 2463,2468 --- 2475,2496 hdr->m_type = mtype; } + /* -- + * pgstat_send_inquiry() - + * + * Notify collector that we need fresh data. + * -- + */ + static void + pgstat_send_inquiry(void) + { + PgStat_MsgInquiry msg; + + elog(DEBUG1, "pgstat_send_inquiry: I am %u", getpid()); + pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_INQUIRY); + msg.m_pid = getpid(); + pgstat_send(&msg, sizeof(msg)); + } /* -- * pgstat_send() - *** *** 2538,2545 NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]) { - struct itimerval write_timeout; - bool need_timer = false; int len; PgStat_Msg msg; --- 2566,2571 *** *** 2577,2583 pqsignal(SIGINT, SIG_IGN); pqsignal(SIGTERM, SIG_IGN); pqsignal(SIGQUIT, pgstat_exit); - pqsignal(SIGALRM, force_statwrite); pqsignal(SIGPIPE, SIG_IGN); pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, SIG_IGN); --- 2603,2608 *** *** 2598,2608 */ need_statwrite = true; - /* Preset the delay between status file writes */ - MemSet(&write_ti
Re: [HACKERS] CVS head has broken make
On Fri, Sep 5, 2008 at 2:52 PM, Alvaro Herrera <[EMAIL PROTECTED]> wrote: > Tom Lane wrote: >> Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: >> > yeah the "code coverage" changes broke it - the buildfarm dashboard is >> > pretty telling: >> >> Yes --- it looks like the configure.in patch is designed to look for >> gcov AND lcov (do we really need both?) AND genhtml, and error out >> if they're not present, even if you didn't say --enable-coverage. >> Please fix. > > gcov and lcov do different things; they are both needed. lcov is a GNU > extension of gcov, which generates the HTML pages. just confirmed that this is fixed. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Planner question
Tom Raney <[EMAIL PROTECTED]> writes: > Why does the planner consider both input variations of each symmetric merge > join? The README says "there is not a lot of difference" between the two > options. When are there any differences? The righthand side needs to support mark/restore, the left doesn't; so depending on plan types one way might need a helper Materialize node that the other way doesn't. Also, duplicated values are a bit cheaper to process on the left than the right. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CVS head has broken make
Tom Lane wrote: > Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: > > yeah the "code coverage" changes broke it - the buildfarm dashboard is > > pretty telling: > > Yes --- it looks like the configure.in patch is designed to look for > gcov AND lcov (do we really need both?) AND genhtml, and error out > if they're not present, even if you didn't say --enable-coverage. > Please fix. gcov and lcov do different things; they are both needed. lcov is a GNU extension of gcov, which generates the HTML pages. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Verbosity of Function Return Type Checks
Volkan YAZICI wrote: > BTW, Alvaro fixed my string concatenations which yielded in lines > exceeding 80 characters width, but I'd want to ask twice if you're sure > with this. Because, IMHO, PostgreSQL is also famous with the quality and > readability of its source code -- that I'm quite proud of as a user, > kudos to developers -- and I think it'd be better to stick with 80 > characters width convention as much as one can. Yeah, I'm quite anal about that. What will happen is that pgindent will "push back" the strings so that they start earlier in the line to keep the 80 char limit. IMHO that's actually clearer than truncating the string. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Planner question
On Fri, 2008-09-05 at 11:21 -0700, Tom Raney wrote: > Why does the planner consider both input variations of each symmetric merge > join? The README says "there is not a lot of difference" between the two > options. When are there any differences? > > -Tom Raney > http://archives.postgresql.org/pgsql-general/2008-08/msg00967.php My understanding from that thread is that if one table has high ndistinct and the other has low ndistinct, one plan may require more re-scanning than the other. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [Review] Tests citext casts by David Wheeler.
On Sep 5, 2008, at 11:30, Tom Lane wrote: Thanks for reviewing. I've committed this with your suggestions and one additional non-cosmetic change: schema-qualify names in the bodies of the SQL functions so that they are not search_path dependent. Thanks, I'll check that out. One thing that didn't make a lot of sense to me was the last new function: CREATE OR REPLACE FUNCTION translate( citext, citext, text ) RETURNS TEXT AS $$ SELECT pg_catalog.translate( pg_catalog.translate( $1::pg_catalog.text, pg_catalog.lower($2::pg_catalog.text), $3), pg_catalog.upper($2::pg_catalog.text), $3); $$ LANGUAGE SQL IMMUTABLE STRICT; Why is it using upper()? To make translate() work case-insensitively, it does two translates: One lowercase and one uppercase. This allows the translated value to be returned with its original casing in tact. No, this isn't ideal, but it was simple to do. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prototype: In-place upgrade v02
Zdenek Kotala wrote: Heikki Linnakangas napsal(a): The patch seems to be missing the new htup.c file. I'm sorry. I attached new version which is synchronized with current head. I would like to say more comments as well. 1) The patch contains also changes which was discussed during July commit fest. - PageGetTempPage modification suggested by Tom - another hash.h backward compatible cleanup It might be a good idea to split that into a separate patch. The sheer size of this patch is quite daunting, even though the bulk of it is straightforward search&replace. 2) I add tuplimits.h header file which contains tuple limits for different access method. It is not finished yet, but idea is to keep all limits in one file and easily add limits for different page layout version - for example replace static computing with dynamic based on relation (maxtuplesize could be store in pg_class for each relation). I need this header also because I fallen in a cycle in header dependency. 3) I already sent Page API performance result in http://archives.postgresql.org/pgsql-hackers/2008-08/msg00398.php I replaced call sequence PagetGetItemId, PageGetItemId with PageGetIndexTuple and PageGetHeapTuple function. It is main difference in this patch. PAgeGetHeap Tuple fills t_ver in HeapTuple to identify correct tupleheader version. It would be good to mention that PageAPI (and tuple API) implementation is only prototype without any performance optimization. You mentioned 5% performance degradation in that thread. What test case was that? What would be a worst-case scanario, and how bad is it? 5% is a pretty hefty price, especially when it's paid by not only upgraded installations, but also freshly initialized clusters. I think you'll need to pursue those performance optimizations. 4) This patch contains more topics for decision. First is general if this approach is acceptable. I don't like the invasiveness of this approach. It's pretty invasive already, and ISTM you'll need similar switch-case handling of all data types that have changed the internal representation as well. We've talked about this before, so you'll remember that I favor teh approach is to convert the page format, page at a time, when the pages are read in. I grant you that there's non-trivial issues with that as well, like if the converted data takes more space and don't fit in the page anymore. I wonder if we could go with some sort of a hybrid approach? Convert the whole page when it's read in, but if it doesn't fit, fall back to tricks like loosening the alignment requirements on platforms that can handle non-aligned data, or support a special truncated page header, without pd_tli and pd_prune_xid fields. Just a thought, not sure how feasible those particular tricks are, but something along those lines.. All in all, though. I find it a bit hard to see the big picture. For upgrade-in-place, what are all the pieces that we need? To keep this concrete, let's focus on PG 8.2 -> PG 8.3 (or are you focusing on PG 8.3 -> 8.4? That's fine with me as well, but let's pick one) and forget about hypothetical changes that might occur in a future version. I can see: 1. Handling page layout changes (pd_prune_xid, pd_flags) 2. Handling tuple header changes (infomask2, HOT bits, combocid) 3. Handling changes in data type representation (packed varlens) 4. Toast chunk size 5. Catalogs After putting all those together, how large a patch are we talking about, and what's the performance penalty then? How much of all that needs to be in core, and how much can live in a pgfoundry project or an extra binary in src/bin or contrib? I realize that none of us have a crystal ball, and one has to start somewhere, but I feel uneasy committing to an approach until we have a full plan. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [Review] Tests citext casts by David Wheeler.
"Ryan Bradetich" <[EMAIL PROTECTED]> writes: > Here is my review of the Test citext casts written by David Wheeler: Thanks for reviewing. I've committed this with your suggestions and one additional non-cosmetic change: schema-qualify names in the bodies of the SQL functions so that they are not search_path dependent. One thing that didn't make a lot of sense to me was the last new function: CREATE OR REPLACE FUNCTION translate( citext, citext, text ) RETURNS TEXT AS $$ SELECT pg_catalog.translate( pg_catalog.translate( $1::pg_catalog.text, pg_catalog.lower($2::pg_catalog.text), $3), pg_catalog.upper($2::pg_catalog.text), $3); $$ LANGUAGE SQL IMMUTABLE STRICT; Why is it using upper()? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Verbosity of Function Return Type Checks
On Fri, 05 Sep 2008, Tom Lane <[EMAIL PROTECTED]> writes: > I think the best way is to use > > subroutine(..., gettext_noop("special error message here")) > > at the call sites, and then > > errmsg("%s", _(msg)) > > when throwing the error. gettext_noop() is needed to have the string > be put into the message catalog, but it doesn't represent any run-time > effort. The _() macro is what actually makes the translation lookup > happen. We don't want to incur that cost in the normal no-error case, > which is why you shouldn't just do _() at the call site and pass an > already-translated string to the subroutine. Modified as you suggested. BTW, will there be a similar i18n scenario for "dropped column" you mentioned below? >> if (td1->attrs[i]->atttypid && >> td2->attrs[i]->atttypid && >> td1->attrs[i]->atttypid != td2->attrs[i]->atttypid) > >> expression to fix this? > > No, that's weakening the compatibility check. (There's a separate issue > here of teaching plpgsql to actually cope nicely with rowtypes > containing dropped columns, but that's well beyond the scope of this > patch.) > > What I'm on about is protecting format_type_be() from being passed > a zero and then failing. Perhaps it would be good enough to do > something like > > OidIsValid(td1->attrs[i]->atttypid) ? > format_type_with_typemod(td1->attrs[i]->atttypid, > td1->attrs[i]->atttypmod) : > "dropped column" > > while throwing the error. > > BTW, since what's actually being looked at is just the type OID and not > the typmod, it seems inappropriate to me to show the typmod in the > error. I'd go with just format_type_be(td1->attrs[i]->atttypid) here > I think. Done with format_type_be() usage. BTW, Alvaro fixed my string concatenations which yielded in lines exceeding 80 characters width, but I'd want to ask twice if you're sure with this. Because, IMHO, PostgreSQL is also famous with the quality and readability of its source code -- that I'm quite proud of as a user, kudos to developers -- and I think it'd be better to stick with 80 characters width convention as much as one can. Regards. Index: src/pl/plpgsql/src/pl_exec.c === RCS file: /projects/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v retrieving revision 1.219 diff -c -r1.219 pl_exec.c *** src/pl/plpgsql/src/pl_exec.c 1 Sep 2008 22:30:33 - 1.219 --- src/pl/plpgsql/src/pl_exec.c 5 Sep 2008 18:19:50 - *** *** 188,194 Oid reqtype, int32 reqtypmod, bool isnull); static void exec_init_tuple_store(PLpgSQL_execstate *estate); ! static bool compatible_tupdesc(TupleDesc td1, TupleDesc td2); static void exec_set_found(PLpgSQL_execstate *estate, bool state); static void plpgsql_create_econtext(PLpgSQL_execstate *estate); static void free_var(PLpgSQL_var *var); --- 188,195 Oid reqtype, int32 reqtypmod, bool isnull); static void exec_init_tuple_store(PLpgSQL_execstate *estate); ! static void validate_tupdesc_compat(TupleDesc expected, TupleDesc returned, ! const char *msg); static void exec_set_found(PLpgSQL_execstate *estate, bool state); static void plpgsql_create_econtext(PLpgSQL_execstate *estate); static void free_var(PLpgSQL_var *var); *** *** 384,394 { case TYPEFUNC_COMPOSITE: /* got the expected result rowtype, now check it */ ! if (estate.rettupdesc == NULL || ! !compatible_tupdesc(estate.rettupdesc, tupdesc)) ! ereport(ERROR, ! (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg("returned record type does not match expected record type"))); break; case TYPEFUNC_RECORD: --- 385,392 { case TYPEFUNC_COMPOSITE: /* got the expected result rowtype, now check it */ ! validate_tupdesc_compat(tupdesc, estate.rettupdesc, ! gettext_noop("returned record type does not match expected record type")); break; case TYPEFUNC_RECORD: *** *** 705,715 rettup = NULL; else { ! if (!compatible_tupdesc(estate.rettupdesc, ! trigdata->tg_relation->rd_att)) ! ereport(ERROR, ! (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg("returned tuple structure does not match table of trigger event"))); /* Copy tuple to upper executor memory */ rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval)); } --- 703,711 rettup = NULL; else { ! validate_tupdesc_compat(trigdata->tg_relation->rd_att, ! estate.rettupdesc, ! gettext_noop("returned tuple structure does not match table of trigger event")); /* Copy tuple to upper executor memory */ rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval)); } *** *** 2199,2209 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
[HACKERS] Planner question
Why does the planner consider both input variations of each symmetric merge join? The README says "there is not a lot of difference" between the two options. When are there any differences? -Tom Raney -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Withdraw PL/Proxy from commitfest
On Fri, Sep 5, 2008 at 7:37 PM, Heikki Linnakangas < [EMAIL PROTECTED]> wrote: > So, you'll implement the part of SQL-MED that deals with specifying remote > connections, e.g something like "CREATE CONNECTION" (no, I haven't looked at > what the syntax actually is)? > > Yeah, that sounds like a good idea. We should get that into core, and > modify contrib/dblink to use it as well. It's just a small part of SQL-MED, > but it's a start, and it's useful for these other projects. > Yes that's the plan. > > > Marko Kreen wrote: > >> In the previous discussion there was mentioned that Postgres should >> move to the SQL-MED direction in remote connection handling. >> >> SQL-MED specifies that connections should have names and referenced >> everywhere using names. PL/Proxy currently does not conform to that >> standard - it uses connection strings directly. Although it could >> made work with SQL-MED backend, it would look ugly. >> >> So I'd like to withdraw PL/Proxy from commitfest and rework it's >> connection handling scheme to be also name->connstr based. Idea will >> be that it will have user-definable connection handling backend, >> which operates on named connections. And in the future we can >> plug in a backend that reuses connection info from builtin SQL-MED store. >> >> Although the current connection handling works and is secure it has >> a deficiency that it's bit hard to hide the password that is used >> for connecting. User can either play with table/function permissions >> and SECURITY DEFINER functions but that's complex. Or he can put >> passwords into .pgpass - this is easy and secure but has the problem >> that the file is not manageable from inside database. >> >> So PL/Proxy needs new SQL-MED based scheme that fixes it. When this >> is ready we can re-discuss the builtin vs. PL-based remote functions. >> As I don't plan to work on it near-term there is no point polluting >> the commitfest page with it. >> >> [ There was a attempt to paint the .pgpass based password handling >> insecure because dblink makes the file world-readable. I still >> fail to see how this any way points to flaws of the scheme... ] >> >> > > -- > Heikki Linnakangas > EnterpriseDB http://www.enterprisedb.com > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] 8.4devel out of memory
"Kevin Grittner" <[EMAIL PROTECTED]> writes: > The tables and views aren't that hard; finding a way to generate > enough fake data may be a challenge. (I assume that since it took > over a half hour to run out of memory, the volume of data needs to be > sufficient to get there.) We don't really need 2GB of leakage to find the problem ... a query that generates a couple hundred meg of bloat would be plenty. Since we don't know how much space the query would have needed to run to completion, it's likely that something involving much less than a tenth as much data would still be enough to make the leak obvious. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CVS head has broken make
Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: > yeah the "code coverage" changes broke it - the buildfarm dashboard is > pretty telling: Yes --- it looks like the configure.in patch is designed to look for gcov AND lcov (do we really need both?) AND genhtml, and error out if they're not present, even if you didn't say --enable-coverage. Please fix. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 8.4devel out of memory
>>> Tom Lane <[EMAIL PROTECTED]> wrote: > "Kevin Grittner" <[EMAIL PROTECTED]> writes: >> PortalHeapMemory: 1620992 total in 200 blocks; 5856 free (8 chunks); > 1615136 used >> ExecutorState: 2787288448 total in 364 blocks; 328 free (5 chunks); > 2787288120 used > > Ouch. We have created a memory leak someplace, but it's hard to tell > where from this info. Can you boil this down into a self-contained test > case? I doubt it depends at all on the specific data, so the table & > view definitions plus some dummy data would probably be enough to > reproduce it. The tables and views aren't that hard; finding a way to generate enough fake data may be a challenge. (I assume that since it took over a half hour to run out of memory, the volume of data needs to be sufficient to get there.) > Is this a 32-bit or 64-bit build? 32-bit PostgreSQL on 32-bit Linux. > Also, does the leak still occur if > you just run the query as-is rather than EXPLAIN ANALYZE it? I will find out. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CVS head has broken make
Andrew Chernow wrote: Getting this, my cvs copy is as of 1:00PM EST. [EMAIL PROTECTED] pgsql]# ./configure --prefix=/usr checking build system type... i686-pc-linux-gnu checking host system type... i686-pc-linux-gnu checking which template to use... linux checking whether to build with 64-bit integer date/time support... yes checking whether NLS is wanted... no checking for default port number... 5432 checking for gcov... gcov checking for lcov... no configure: error: lcov not found I get the same error trying to make libpq. Another box with a copy from yesterday afternoon builds fine. Things updated that may be related. yeah the "code coverage" changes broke it - the buildfarm dashboard is pretty telling: http://buildfarm.postgresql.org/cgi-bin/show_status.pl Stefan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql is not translate-aware
Alvaro Herrera <[EMAIL PROTECTED]> writes: > The vices in the error message are not the translator's fault: missing > quotes and "plpgsql" instead of "PL/pgSQL": It's been message style policy for quite some time to not quote the output of format_type. I think this is because format_type sometimes puts quotes into its output, and it'd look weird. > I'd even go a bit further and say that the original should not include > the language name in the string, so that (say) plpython and plperl can > use the same translation: That'd only be useful if they all share a common message catalog, which does not seem like a good design direction to me. How would a non-core PL hope to get localized if it can't have its own catalog? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] CVS head has broken make
Getting this, my cvs copy is as of 1:00PM EST. [EMAIL PROTECTED] pgsql]# ./configure --prefix=/usr checking build system type... i686-pc-linux-gnu checking host system type... i686-pc-linux-gnu checking which template to use... linux checking whether to build with 64-bit integer date/time support... yes checking whether NLS is wanted... no checking for default port number... 5432 checking for gcov... gcov checking for lcov... no configure: error: lcov not found I get the same error trying to make libpq. Another box with a copy from yesterday afternoon builds fine. Things updated that may be related. P GNUmakefile.in P configure P configure.in P src/Makefile.global.in P src/backend/common.mk -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 8.4devel out of memory
"Kevin Grittner" <[EMAIL PROTECTED]> writes: > PortalHeapMemory: 1620992 total in 200 blocks; 5856 free (8 chunks); > 1615136 used > ExecutorState: 2787288448 total in 364 blocks; 328 free (5 chunks); > 2787288120 used Ouch. We have created a memory leak someplace, but it's hard to tell where from this info. Can you boil this down into a self-contained test case? I doubt it depends at all on the specific data, so the table & view definitions plus some dummy data would probably be enough to reproduce it. Is this a 32-bit or 64-bit build? Also, does the leak still occur if you just run the query as-is rather than EXPLAIN ANALYZE it? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql is not translate-aware
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > Actually this is wrong -- since the library is going to run with > > "postgres" text domain, we need to add the files to the backend's > > nls.mk: > > Can't we give it its own text domain? It seems fundamentally wrong > for a plug-in language to require core support for its messages. > (Now that I think about it, that may have been the reason we don't have > localization for it already.) I suppose this must be possible, > since e.g. glibc manages to have its own messages separate from > whatever app it's linked with. I'm not sure how this'd work. I think this would require plpgsql using dgettext (passing a domain) instead of plain gettext(), but since it uses ereport() just like the backend, I don't see a good way to make that work. Maybe another idea would be to call textdomain() just before calling anything that would raise an error, and reset it on exit. But since backend errors can happen at any time too, this doesn't seem possible. Not sure how glibc does it. Maybe they just use dgettext(). -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Withdraw PL/Proxy from commitfest
So, you'll implement the part of SQL-MED that deals with specifying remote connections, e.g something like "CREATE CONNECTION" (no, I haven't looked at what the syntax actually is)? Yeah, that sounds like a good idea. We should get that into core, and modify contrib/dblink to use it as well. It's just a small part of SQL-MED, but it's a start, and it's useful for these other projects. Marko Kreen wrote: In the previous discussion there was mentioned that Postgres should move to the SQL-MED direction in remote connection handling. SQL-MED specifies that connections should have names and referenced everywhere using names. PL/Proxy currently does not conform to that standard - it uses connection strings directly. Although it could made work with SQL-MED backend, it would look ugly. So I'd like to withdraw PL/Proxy from commitfest and rework it's connection handling scheme to be also name->connstr based. Idea will be that it will have user-definable connection handling backend, which operates on named connections. And in the future we can plug in a backend that reuses connection info from builtin SQL-MED store. Although the current connection handling works and is secure it has a deficiency that it's bit hard to hide the password that is used for connecting. User can either play with table/function permissions and SECURITY DEFINER functions but that's complex. Or he can put passwords into .pgpass - this is easy and secure but has the problem that the file is not manageable from inside database. So PL/Proxy needs new SQL-MED based scheme that fixes it. When this is ready we can re-discuss the builtin vs. PL-based remote functions. As I don't plan to work on it near-term there is no point polluting the commitfest page with it. [ There was a attempt to paint the .pgpass based password handling insecure because dblink makes the file world-readable. I still fail to see how this any way points to flaws of the scheme... ] -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On Fri, 2008-09-05 at 09:19 -0400, Andrew Dunstan wrote: > > Simon Riggs wrote: > > On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote: > > > > > >> If you are a postgresql hacker at all, or even want to be one, we need > >> your > >> help reviewing patches! There are several "easy" patches in the list, so > >> I can assign them to beginners. > >> > > > > It would be a reasonable rule that all patch submitters also have to do > > patch reviews. If we made it a strict rule, then sponsoring companies > > would know that they *must* provide money/time for that aspect also. > > Otherwise it is almost impossible to get formal approval to do that. > All this would do is to deter people from submitting patches. Hard rules > like this don't work in FOSS communities. I know it's like herding cats, > but persuasion is really our only tool. I don't *want* the rule, I just think we *need* the rule because otherwise sponsors/managers/etc make business decisions to exclude that aspect of the software dev process. Otherwise we have a patch-and-dump culture that is unsustainable because a few people's benevolence as reviewers turns everything into a bottleneck. It doesn't need to mean loss of control for core and committers. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql is not translate-aware
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Actually this is wrong -- since the library is going to run with > "postgres" text domain, we need to add the files to the backend's > nls.mk: Can't we give it its own text domain? It seems fundamentally wrong for a plug-in language to require core support for its messages. (Now that I think about it, that may have been the reason we don't have localization for it already.) I suppose this must be possible, since e.g. glibc manages to have its own messages separate from whatever app it's linked with. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql is not translate-aware
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > In reviewing Volkan Yazici's (sorry for the dots) patch to improve > > plpgsql's error messages, I noticed that we have no PO files for plpgsql > > at all! > > Ugh. Yeah, we should fix that. Does it actually just work, seeing > that plpgsql is a loadable library? Well, it didn't, but I just tested what I posted in the followup and it does work: alvherre=# create function aa (internal) returns int language plpgsql as $$ begin; select 1; end; $$; ERROR: las funciones plpgsql no pueden tener el tipo internal como argumento The vices in the error message are not the translator's fault: missing quotes and "plpgsql" instead of "PL/pgSQL": alvherre=# set lc_messages to 'C'; SET alvherre=# create function aa (internal) returns int language plpgsql as $$ begin; select 1; end; $$; ERROR: plpgsql functions cannot take type internal I'd even go a bit further and say that the original should not include the language name in the string, so that (say) plpython and plperl can use the same translation: "%s functions cannot take type \"%s\"", "PL/pgSQL", type_name -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On 9/5/08, Marko Kreen <[EMAIL PROTECTED]> wrote: > The list is correct but too verbose. And it does not attack the core > of the problem. I think the problem is not: > > What can/should I do? > > but instead: > > Can I take the responsibility? To clarify it - that was the problem I faced last commitfest. Basically, I'm not a newbie, but I'm not deeply familiar with most of the components in Postgres. I'm not afraid to patch any part of code, because I know somebody who *is* familiar with the part will later review it. But if I'm supposed to answer "Is this patch commitable?" then this is too much for me. But when I convinced myself that my only task is to decrease the amount problems a patch has, so that committers job will be easier, then I felt at ease to take stab on several of them. So I suppose this works for other too and maybe this is worth accepting as official policy - the reviewers job is not to guarantee some level of quality, but just to report any problems they are able to find, so that committer's job will be easier and he can concentrate on the in-depth review. All this assumes you want relatively nobodies doing the reviews. If you want guaranteed quality, then this will scare away lightweights or make them look only one aspect of the patch. This leaves the heavyweights, but as you know, they are busy.. > Lets say reviewer would like look on coding style or performance. > ATM it seems to him he well be now fully responsible for that aspect. > > I think we have better results and more relaxed atmospere if we > use following task description for reviewers: > > The committer will do in-depth review. You task as a reviewer > is to take off load from committers by catching simple problems. > Your task is done if you think the patch is ready for in-depth > review from committer. > > Note1 - Yes, the trick is to emphasize that all responsibility > lies on committer. > > Note2 - detailed lists of areas to look at and reviewer types are not > useful as each patch is different and each revier is different. > Long lists just confuse people. The simpler the better. > > The main thing is to make easy for reviewer to take the first look. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Verbosity of Function Return Type Checks
Volkan YAZICI <[EMAIL PROTECTED]> writes: > On Thu, 04 Sep 2008, Tom Lane <[EMAIL PROTECTED]> writes: >> This is not ready to go: you've lost the ability to localize most of >> the error message strings. > How can I make this available? What's your suggestion? I think the best way is to use subroutine(..., gettext_noop("special error message here")) at the call sites, and then errmsg("%s", _(msg)) when throwing the error. gettext_noop() is needed to have the string be put into the message catalog, but it doesn't represent any run-time effort. The _() macro is what actually makes the translation lookup happen. We don't want to incur that cost in the normal no-error case, which is why you shouldn't just do _() at the call site and pass an already-translated string to the subroutine. >> Another problem with it is it's likely going to fail completely on >> dropped columns (which will have atttypid = 0). > Is it ok if I'd replace > if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid) > line in validate_tupdesc_compat with > if (td1->attrs[i]->atttypid && > td2->attrs[i]->atttypid && > td1->attrs[i]->atttypid != td2->attrs[i]->atttypid) > expression to fix this? No, that's weakening the compatibility check. (There's a separate issue here of teaching plpgsql to actually cope nicely with rowtypes containing dropped columns, but that's well beyond the scope of this patch.) What I'm on about is protecting format_type_be() from being passed a zero and then failing. Perhaps it would be good enough to do something like OidIsValid(td1->attrs[i]->atttypid) ? format_type_with_typemod(td1->attrs[i]->atttypid, td1->attrs[i]->atttypmod) : "dropped column" while throwing the error. BTW, since what's actually being looked at is just the type OID and not the typmod, it seems inappropriate to me to show the typmod in the error. I'd go with just format_type_be(td1->attrs[i]->atttypid) here I think. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql is not translate-aware
Alvaro Herrera <[EMAIL PROTECTED]> writes: > In reviewing Volkan Yazici's (sorry for the dots) patch to improve > plpgsql's error messages, I noticed that we have no PO files for plpgsql > at all! Ugh. Yeah, we should fix that. Does it actually just work, seeing that plpgsql is a loadable library? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql is not translate-aware
Alvaro Herrera wrote: > It doesn't seem hard to add; I just had to create a nls.mk file and > things seem ready to go. Obviously, we'll need to add plpgsql to the > pgtranslation files in pgfoundry. Actually this is wrong -- since the library is going to run with "postgres" text domain, we need to add the files to the backend's nls.mk: Index: nls.mk === RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/nls.mk,v retrieving revision 1.22 diff -c -p -u -r1.22 nls.mk --- nls.mk 24 Mar 2008 18:08:47 - 1.22 +++ nls.mk 5 Sep 2008 16:00:18 - @@ -7,7 +7,7 @@ GETTEXT_FILES := + gettext-files GETTEXT_TRIGGERS:= _ errmsg errdetail errdetail_log errhint errcontext write_stderr yyerror gettext-files: distprep - find $(srcdir)/ $(srcdir)/../port/ -name '*.c' -print >$@ + find $(srcdir)/ $(srcdir)/../port/ $(srcdir)/../pl/ -name '*.c' -print >$@ my-maintainer-clean: rm -f gettext-files -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [Review] pgbench duration option
Hello again, I received the following email from a helpful fellow off-list, pointing out an error in my review: On Fri, Sep 5, 2008 at 7:03 PM, Ragnar <[EMAIL PROTECTED]> wrote: > On fös, 2008-09-05 at 15:07 +1000, Brendan Jurd wrote: >> Wouldn't this be better written as: >> >> if ((duration > 0 && timer_exceeded) || st->cnt >= >> nxacts) >> { >> >> } > > sorry, but these do not lok as the same thing to me. > > in the first variant there will not be a stop if > (duration > 0) and NOT (timer_exceeded) and (st->cnt >= nxacts) > but in the second variant there will. > > admittedly, i have no idea if that situation can occur. > > gnari > gnari is right. Looking closer I see that nxacts defaults to 10 in the absence of a -t option, so my version of the code would end up stopping when the run reaches 10 transactions, even if the user has specified a -T option. Sorry for the error. The (duration > 0) test does in fact need to be separate. Thanks for the catch, gnari. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plpgsql is not translate-aware
Hi, In reviewing Volkan Yazici's (sorry for the dots) patch to improve plpgsql's error messages, I noticed that we have no PO files for plpgsql at all! It doesn't seem hard to add; I just had to create a nls.mk file and things seem ready to go. Obviously, we'll need to add plpgsql to the pgtranslation files in pgfoundry. There are 141 new strings to translate, and from spanish I get 71 fuzzies, so it seems an easy project. Should I go ahead and commit the initial files? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Verbosity of Function Return Type Checks
On Fri, 5 Sep 2008, Alvaro Herrera <[EMAIL PROTECTED]> writes: > Please use the patch I posted yesterday, as it had all the issues I > found fixed. There were other changes in that patch too. My bad. Patch is modified with respect to suggestions[1][2] from Tom. (All 115 tests passed in cvs tip.) Regards. [1] "char *msg" is replaced with "const char *msg". [2] "errmsg(msg)" is replaced with 'errmsg("%s", msg)'. Index: src/pl/plpgsql/src/pl_exec.c === RCS file: /projects/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v retrieving revision 1.219 diff -c -r1.219 pl_exec.c *** src/pl/plpgsql/src/pl_exec.c 1 Sep 2008 22:30:33 - 1.219 --- src/pl/plpgsql/src/pl_exec.c 5 Sep 2008 13:47:07 - *** *** 188,194 Oid reqtype, int32 reqtypmod, bool isnull); static void exec_init_tuple_store(PLpgSQL_execstate *estate); ! static bool compatible_tupdesc(TupleDesc td1, TupleDesc td2); static void exec_set_found(PLpgSQL_execstate *estate, bool state); static void plpgsql_create_econtext(PLpgSQL_execstate *estate); static void free_var(PLpgSQL_var *var); --- 188,195 Oid reqtype, int32 reqtypmod, bool isnull); static void exec_init_tuple_store(PLpgSQL_execstate *estate); ! static void validate_tupdesc_compat(TupleDesc expected, TupleDesc returned, ! const char *msg); static void exec_set_found(PLpgSQL_execstate *estate, bool state); static void plpgsql_create_econtext(PLpgSQL_execstate *estate); static void free_var(PLpgSQL_var *var); *** *** 384,394 { case TYPEFUNC_COMPOSITE: /* got the expected result rowtype, now check it */ ! if (estate.rettupdesc == NULL || ! !compatible_tupdesc(estate.rettupdesc, tupdesc)) ! ereport(ERROR, ! (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg("returned record type does not match expected record type"))); break; case TYPEFUNC_RECORD: --- 385,392 { case TYPEFUNC_COMPOSITE: /* got the expected result rowtype, now check it */ ! validate_tupdesc_compat(tupdesc, estate.rettupdesc, ! "returned record type does not match expected record type"); break; case TYPEFUNC_RECORD: *** *** 705,715 rettup = NULL; else { ! if (!compatible_tupdesc(estate.rettupdesc, ! trigdata->tg_relation->rd_att)) ! ereport(ERROR, ! (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg("returned tuple structure does not match table of trigger event"))); /* Copy tuple to upper executor memory */ rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval)); } --- 703,711 rettup = NULL; else { ! validate_tupdesc_compat(trigdata->tg_relation->rd_att, ! estate.rettupdesc, ! "returned tuple structure does not match table of trigger event"); /* Copy tuple to upper executor memory */ rettup = SPI_copytuple((HeapTuple) DatumGetPointer(estate.retval)); } *** *** 2199,2209 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("record \"%s\" is not assigned yet", rec->refname), ! errdetail("The tuple structure of a not-yet-assigned record is indeterminate."))); ! if (!compatible_tupdesc(tupdesc, rec->tupdesc)) ! ereport(ERROR, ! (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg("wrong record type supplied in RETURN NEXT"))); tuple = rec->tup; } break; --- 2195,2204 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("record \"%s\" is not assigned yet", rec->refname), ! errdetail("The tuple structure of a not-yet-assigned" ! " record is indeterminate."))); ! validate_tupdesc_compat(tupdesc, rec->tupdesc, ! "wrong record type supplied in RETURN NEXT"); tuple = rec->tup; } break; *** *** 2309,2318 stmt->params); } ! if (!compatible_tupdesc(estate->rettupdesc, portal->tupDesc)) ! ereport(ERROR, ! (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg("structure of query does not match function result type"))); while (true) { --- 2304,2311 stmt->params); } ! validate_tupdesc_compat(estate->rettupdesc, portal->tupDesc, ! "structure of query does not match function result type"); while (true) { *** *** 5145,5167 } /* ! * Check two tupledescs have matching number and types of attributes */ ! static bool ! compatible_tupdesc(TupleDesc td1, TupleDesc td2) { ! int i; ! if (td1->natts != td2->natts) ! return false; ! for (i = 0; i < td1->natts; i++) ! { ! if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid) ! return false; ! } ! return true; } /* -- --- 5138,5174 } /* !
Re: [HACKERS] Need more reviewers!
On Fri, 2008-09-05 at 17:19 +0300, Marko Kreen wrote: > > > > I think this should be organised with different kinds of reviewer: > > The list is correct but too verbose. And it does not attack the core > of the problem. I think the problem is not: > > What can/should I do? > > but instead: > > Can I take the responsibility? Completely agree. The list was really an example of the different styles of review that are possible, not a rigid categorisation that must be followed. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On 9/5/08, Simon Riggs <[EMAIL PROTECTED]> wrote: > On Fri, 2008-09-05 at 16:03 +0200, Markus Wanner wrote: > > > I don't *want* the rule, I just think we *need* the rule because > > > otherwise sponsors/managers/etc make business decisions to exclude that > > > aspect of the software dev process. > > > > I agree that making sponsors/managers/etc aware of that aspect of the > > dev process is necessary and worthwhile. However, I don't think a rule > > for *patch submitters* helps with that. There must be other ways to > > convince managers to encourage reviewers. > > Such as? You might think those arguments exist and work, but I would say > they manifestly do not. Almost all people doing reviews are people that > have considerable control over their own time, or are directed by people > that understand the Postgres review process and wish to contribute to it > for commercial reasons. Well, the number of companies who are *interested* their patches getting in is rather small... I think it's more common for companies to think they are already donating to Postgres by encouraging their staff to write patches and publish them. So such applying such strict policy for everyone seems bad idea. Although I quite agree on strongly encouraging patch submitters to review. And those 3-4 companies who have direct commercial interests in Postgres development should probably internally rethink their time allocation. Note also we are only on our 2nd commitfest so its quite normal that people are not used to the process . We need to work few political aspects: * Making reviewers to more at ease. * Encouraging patch submitters to review. And technical aspects: * The (hopefully short and relaxed) rules for reviewers should be more visible. Best would be on (every) Commitfest page. * Wiki editing rules should be visible. Well, and then: * Although the wiki looks nice it's pain to operate. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] 8.4devel out of memory
I was testing a very complex statistical query, with (among other things) many EXISTS and NOT EXISTS tests against a build of the source snapshot from 3 September. (The query looks pretty innocent, but those aren't tables, they're complicated views.) Under 8.3.3 this query runs successfully, but takes a few hours. I started it last night before leaving, on the same machine where 8.3.3 has been running, and in the morning found this: olr=# explain analyze SELECT "MS"."sMatterNo", CAST(COUNT(*) AS int) AS "count" FROM "MatterSearch" "MS" JOIN "MatterDateStat" "S" ON ( "S"."matterNo" = "MS"."sMatterNo" AND "S"."isOnHold" = FALSE ) WHERE ( "MS"."matterStatusCode" IN ('OP', 'RO') ) GROUP BY "MS"."sMatterNo" ; ERROR: out of memory DETAIL: Failed on request of size 8. It was running for about half an hour before I left, and I didn't notice the error, so I'm pretty sure it took longer than that for this error to appear. [EMAIL PROTECTED]:~> df -h FilesystemSize Used Avail Use% Mounted on /dev/sda2 20G 8.0G 11G 43% / tmpfs 2.0G 16K 2.0G 1% /dev/shm /dev/sda3 253G 7.9G 245G 4% /var/pgsql/data [EMAIL PROTECTED]:~> free -m total used free sharedbuffers cached Mem: 4049 2239 1809 0 94 1083 -/+ buffers/cache: 1061 2987 Swap: 1027561466 There are several development databases on this machine, all fairly small, but enough that there's usually no significant free memory -- it gets used as cache. The 1.8 GB free this morning suggests that something allocated and free a lot of memory. [EMAIL PROTECTED]:~/postgresql-snapshot> uname -a Linux OLR-DEV-PG 2.6.5-7.286-bigsmp #1 SMP Thu May 31 10:12:58 UTC 2007 i686 i686 i386 GNU/Linux [EMAIL PROTECTED]:~/postgresql-snapshot> cat /proc/version Linux version 2.6.5-7.286-bigsmp ([EMAIL PROTECTED]) (gcc version 3.3.3 (SuSE Linux)) #1 SMP Thu May 31 10:12:58 UTC 2007 [EMAIL PROTECTED]:~/postgresql-snapshot> cat /etc/SuSE-release SUSE LINUX Enterprise Server 9 (i586) VERSION = 9 PATCHLEVEL = 3 Attached are the plans from 8.3.3 and 8.4devel. Also attached are the non-default 8.3.3 postgresql.conf settings; the file is the same for 8.4devel except for the port number. I don't know if the specifics of the views and tables would be useful here, or just noise, so I'll omit them unless someone asks for them. What would be the reasonable next step here? -Kevin [EMAIL PROTECTED]:~> /usr/local/pgsql-8.4dev/bin/pg_config BINDIR = /usr/local/pgsql-8.4dev/bin DOCDIR = /usr/local/pgsql-8.4dev/share/doc HTMLDIR = /usr/local/pgsql-8.4dev/share/doc INCLUDEDIR = /usr/local/pgsql-8.4dev/include PKGINCLUDEDIR = /usr/local/pgsql-8.4dev/include INCLUDEDIR-SERVER = /usr/local/pgsql-8.4dev/include/server LIBDIR = /usr/local/pgsql-8.4dev/lib PKGLIBDIR = /usr/local/pgsql-8.4dev/lib LOCALEDIR = /usr/local/pgsql-8.4dev/share/locale MANDIR = /usr/local/pgsql-8.4dev/share/man SHAREDIR = /usr/local/pgsql-8.4dev/share SYSCONFDIR = /usr/local/pgsql-8.4dev/etc PGXS = /usr/local/pgsql-8.4dev/lib/pgxs/src/makefiles/pgxs.mk CONFIGURE = '--prefix=/usr/local/pgsql-8.4dev' '--enable-integer-datetimes' '--enable-debug' '--disable-nls' CC = gcc CPPFLAGS = -D_GNU_SOURCE CFLAGS = -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wendif-labels -fno-strict-aliasing -g CFLAGS_SL = -fpic LDFLAGS = -Wl,-rpath,'/usr/local/pgsql-8.4dev/lib' LDFLAGS_SL = LIBS = -lpgport -lz -lreadline -lcrypt -ldl -lm VERSION = PostgreSQL 8.4devel [EMAIL PROTECTED]:~> /usr/local/pgsql-8.4dev/bin/pg_controldata /var/pgsql/data/kgrittn pg_control version number:842 Catalog version number: 200808311 Database system identifier: 5242286260647024629 Database cluster state: in production pg_control last modified: Thu 04 Sep 2008 05:17:28 PM CDT Latest checkpoint location: 0/26E7A718 Prior checkpoint location:0/26E7A6D4 Latest checkpoint's REDO location:0/26E7A718 Latest checkpoint's TimeLineID: 1 Latest checkpoint's NextXID: 0/3561 Latest checkpoint's NextOID: 49152 Latest checkpoint's NextMultiXactId: 1 Latest checkpoint's NextMultiOffset: 0 Time of latest checkpoint:Thu 04 Sep 2008 05:17:28 PM CDT Minimum recovery ending location: 0/0 Maximum data alignment: 4 Database block size: 8192 Blocks per segment of large relation: 131072 WAL block size: 8192 Bytes per WAL segment:16777216 Maximum length of identifiers:64 Maximum columns in an index: 32 Maximum size of a TOAST chunk:2000 Date/time type storage: 64-bit integers Float4 argument passing: by value Float8 argument passing: by reference Maximum length of locale name:128 LC_COLLATE:
Re: [HACKERS] code coverage patch
Gregory Stark wrote: > Peter Eisentraut <[EMAIL PROTECTED]> writes: > > > I have uploaded an example run here: > > http://developer.postgresql.org/~petere/coverage/ > > > > Current test coverage is about 66% overall. > > With some pretty glaring gaps: 0% coverage of geqo, 0% coverage of logtape > which implies no tuplesorts are spilling to disk, no coverage of mark/restore > on index scans... Yah, that kinda shocked me too. Clearly we should spend some effort to expand the regression tests a bit. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
Hi, Simon Riggs wrote: Such as? Dunno. Rules for sponsors? It would probably make sense to not only pay a single developer to create and submit a patch, but instead plan for paying others to review the code as well. You might think those arguments exist and work, but I would say they manifestly do not. Most managers - especially within software companies I'd say - are pretty much aware of how costly quality assurance (or the lack thereof) can be, no? What do you respond to potential sponsors who request that a new feature must be accepted into Postgres itself? Let's tell *them* that review is costly. Encourage them to pay others to review your work, for example. Let's coopete ;-) (or whatever the verb for coopetition is) Maybe we can do more WRT organizing this reviewing process, including payment. Some sort of bounty system or something. Dunno, this is just some brainstorming. Almost all people doing reviews are people that have considerable control over their own time, or are directed by people that understand the Postgres review process and wish to contribute to it for commercial reasons. Sure. I don't quite get where you are going with this argument, sorry. Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Synchronous Log Shipping Replication
Hi, In PGCon 2008, I proposed synchronous log shipping replication. Sorry for late posting, but I'd like to start the discussion about its implementation from now. http://www.pgcon.org/2008/schedule/track/Horizontal%20Scaling/76.en.html First of all, I'm not planning to put the prototype which I demoed in PGCon into core directly. - Portability issues (using message queue, multi-threaded ...) - Have too much dependency on Heartbeat Yes, since the prototype is useful reference of implementation, I plan to open it ASAP. But, I'm sorry - it still takes a month to open it. Pavan re-designed the sync replication based on the prototype and I posted that design doc on wiki. Please check it if you are interested in it. http://wiki.postgresql.org/wiki/NTT%27s_Development_Projects This design is too huge. In order to enhance the extensibility of postgres, I'd like to divide the sync replication into minimum hooks and some plugins and to develop it, respectively. Plugins for the sync replication plan to be available at the time of 8.4 release. In my design, WAL sending is achieved as follow by WALSender. WALSender is a new process which I introduce. 1) On COMMIT, backend requests WALSender to send WAL. 2) WALSender reads WAL from walbuffers and send it to slave. 3) WALSender waits for the response from slave and replies backend. I propose two hooks for WAL sending. WAL-writing hook This hook is for backend to communicate with WALSender. WAL-writing hook intercepts write system call in XLogWrite. That is, backend requests WAL sending whenever write is called. WAL-writing hook is available also for other uses e.g. Software RAID (writes WAL into two files for durability). Hook for WALSender -- This hook is for introducing WALSender. There are the following three ideas of how to introduce WALSender. A required hook differs by which idea is adopted. a) Use WALWriter as WALSender This idea needs WALWriter hook which intercepts WALWriter literally. WALWriter stops the local WAL write and focuses on WAL sending. This idea is very simple, but I don't think of the use of WALWriter hook other than WAL sending. b) Use new background process as WALSender This idea needs background-process hook which enables users to define new background processes. I think the design of this hook resembles that of rmgr hook proposed by Simon. I define the table like RmgrTable. It's for registering some functions (e.g. main function and exit...) for operating a background process. Postmaster calls the function from the table suitably, and manages a start and end of background process. ISTM that there are many uses in this hook, e.g. performance monitoring process like statspack. c) Use one backend as WALSender In this idea, slave calls the user-defined function which takes charge of WAL sending via SQL e.g. "SELECT pg_walsender()". Compared with other ideas, it's easy to implement WALSender because postmater handles the establishment and authentication of connection. But, this SQL causes a long transaction which prevents vacuum. So, this idea needs idle-state hook which executes plugin before transaction starts. I don't think of the use of this hook other than WAL sending either. Which idea should we adopt? Comments welcome. -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On 9/4/08, Simon Riggs <[EMAIL PROTECTED]> wrote: > On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote: > > We currently have 38 patches pending, and only nine people reviewing them. > > At this rate, the September commitfest will take three months. > > > > If you are a postgresql hacker at all, or even want to be one, we need your > > help reviewing patches! There are several "easy" patches in the list, so > > I can assign them to beginners. > > > > Please volunteer now! > > > Everybody is stuck in "I'm not good enough to do a full review". They're > right (myself included), so that just means we're organising it wrongly. > We can't expect to grow more supermen, but we probably can do more > teamwork and delegation. > > I think this should be organised with different kinds of reviewer: The list is correct but too verbose. And it does not attack the core of the problem. I think the problem is not: What can/should I do? but instead: Can I take the responsibility? Lets say reviewer would like look on coding style or performance. ATM it seems to him he well be now fully responsible for that aspect. I think we have better results and more relaxed atmospere if we use following task description for reviewers: The committer will do in-depth review. You task as a reviewer is to take off load from committers by catching simple problems. Your task is done if you think the patch is ready for in-depth review from committer. Note1 - Yes, the trick is to emphasize that all responsibility lies on committer. Note2 - detailed lists of areas to look at and reviewer types are not useful as each patch is different and each revier is different. Long lists just confuse people. The simpler the better. The main thing is to make easy for reviewer to take the first look. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On Fri, 2008-09-05 at 16:03 +0200, Markus Wanner wrote: > > I don't *want* the rule, I just think we *need* the rule because > > otherwise sponsors/managers/etc make business decisions to exclude that > > aspect of the software dev process. > > I agree that making sponsors/managers/etc aware of that aspect of the > dev process is necessary and worthwhile. However, I don't think a rule > for *patch submitters* helps with that. There must be other ways to > convince managers to encourage reviewers. Such as? You might think those arguments exist and work, but I would say they manifestly do not. Almost all people doing reviews are people that have considerable control over their own time, or are directed by people that understand the Postgres review process and wish to contribute to it for commercial reasons. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] code coverage patch
Peter Eisentraut <[EMAIL PROTECTED]> writes: > I have uploaded an example run here: > http://developer.postgresql.org/~petere/coverage/ > > Current test coverage is about 66% overall. With some pretty glaring gaps: 0% coverage of geqo, 0% coverage of logtape which implies no tuplesorts are spilling to disk, no coverage of mark/restore on index scans... -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PostgreSQL SSL problem
Hi Bruce and Team, I have problems to setup SSL for PostgreSQL server. I did all the steps which described in the documentation (17.8. Secure TCP/IP Connections with SSL), but when I try to start the PostgreSQL server the pg_ctl gave me: "could not start server". And nothing in the logs (I enabled all of them). I googled around but did not find much. After I disable SSL option in postgresql.conf the server is starting successfully. I have all certificates with proper CA signature, rest of applications (Postfix, Apache, etc.) work with this certificates very well. I am using OpenSSL from ports. Please, advise. My spec: FreeBSD 7.0-RELEASE-p3 amd64 PostgreSQL 8.3.3 (installed from ports): WITH_NLS=true WITHOUT_PAM=true WITHOUT_LDAP=true WITHOUT_MIT_KRB5=true WITHOUT_HEIMDAL_KRB5=true WITHOUT_OPTIMIZED_CFLAGS=true WITH_XML=true WITHOUT_TZDATA=true WITHOUT_DEBUG=true WITH_ICU=true WITH_INTDATE=true $ pg_config BINDIR = /usr/local/bin DOCDIR = /usr/local/share/doc/postgresql INCLUDEDIR = /usr/local/include PKGINCLUDEDIR = /usr/local/include/postgresql INCLUDEDIR-SERVER = /usr/local/include/postgresql/server LIBDIR = /usr/local/lib PKGLIBDIR = /usr/local/lib/postgresql LOCALEDIR = /usr/local/share/locale MANDIR = /usr/local/man SHAREDIR = /usr/local/share/postgresql SYSCONFDIR = /usr/local/etc/postgresql PGXS = /usr/local/lib/postgresql/pgxs/src/makefiles/pgxs.mk CONFIGURE = '--with-libraries=/usr/local/lib' '--with-includes=/usr/local/include' '--enable-thread-safety' '--with-docdir=/usr/local/share/doc/postgresql' '--with-openssl' '--with-system-tzdata=/usr/share/zoneinfo' '--enable-integer-datetimes' '--enable-nls' '--prefix=/usr/local' '--mandir=/usr/local/man' '--infodir=/usr/local/info/' '--build=amd64-portbld-freebsd7.0' 'CC=cc' 'CFLAGS=-O2 -fno-strict-aliasing -pipe ' 'LDFLAGS= -pthread -rpath=/usr/local/lib' 'build_alias=amd64-portbld-freebsd7.0' CC = cc CPPFLAGS = -I/usr/local/include CFLAGS = -O2 -fno-strict-aliasing -pipe -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv CFLAGS_SL = -fPIC -DPIC LDFLAGS = -pthread -rpath=/usr/local/lib -L/usr/local/lib -Wl,-R'/usr/local/lib' LDFLAGS_SL = LIBS = -lpgport -lintl -lssl -lcrypto -lz -lreadline -lcrypt -lm VERSION = PostgreSQL 8.3.3 Thanks, Andriy -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
Hi, Simon Riggs wrote: On Fri, 2008-09-05 at 09:19 -0400, Andrew Dunstan wrote: All this would do is to deter people from submitting patches. Hard rules like this don't work in FOSS communities. I know it's like herding cats, but persuasion is really our only tool. +1 I don't *want* the rule, I just think we *need* the rule because otherwise sponsors/managers/etc make business decisions to exclude that aspect of the software dev process. I agree that making sponsors/managers/etc aware of that aspect of the dev process is necessary and worthwhile. However, I don't think a rule for *patch submitters* helps with that. There must be other ways to convince managers to encourage reviewers. Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 > I don't *want* the rule, I just think we *need* the rule because > otherwise sponsors/managers/etc make business decisions to exclude that > aspect of the software dev process. How exactly would you even begin to enforce such a rule? Retroactively pull otherwise vali patches from the queue? Ban people from sending email to the -patches list? > Otherwise we have a patch-and-dump culture that is unsustainable because > a few people's benevolence as reviewers turns everything into a > bottleneck. It doesn't need to mean loss of control for core and > committers. That problem needs a solution, but not the one you proposed. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200809050953 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkjBOdIACgkQvJuQZxSWSsiFoACgoqOgumuuZq6z2HBPSAPZUWHd kS0An2TgFmOLTgdFWuLkpazFbECY4nnz =ZrYl -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] libpq events update
Andrew Chernow wrote: I think it got confused with the instanceData feature, which has nothing to do with the event system and requires public functions. libpqtypes happens to use the instanceData functions within its eventproc, but this is not a requirement. I forgot to mention that the instanceData functions should be moved from libpq-events.h to libpq-fe.h because they are not part of the event system. I plan on making this change as well, so let me know if you hate it. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] libpq events update
I would like to remove the PQpassThroughData and PQresultPassThroughData functions.The passThrough pointer should be added as a 3rd argument to the PGEventProc: typedef int (*PGEventProc)(PGEventId evtId, void *evtInfo, void *passThrough); Having a public accessor function for the passThrough. doesn't seem helpful. Its purpose is to be available to the eventproc, which doesn't require a public function. I think it got confused with the instanceData feature, which has nothing to do with the event system and requires public functions. libpqtypes happens to use the instanceData functions within its eventproc, but this is not a requirement. All those who oppose any of the above, speak now or forever hold your peace. An updated patch with full sgml documentation is coming. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_regress inputdir
Jorgen Austvik - Sun Norway wrote: Alvaro Herrera wrote: In my opinion, the need for running tests outside the test dir is not very strong (or we would have heard complaints before), and thus the solution is to remove --inputdir and --outputdir. Attached is a patch that removes --inputdir and --outputdir. I still prefere the first patch (that fixed my problem), but removing them is probably better than having them when they don't work. There is interest among packagers to run the regression tests or other tests after the build. The Red Hat RPMs have shipped a postgresql-test package for years with a hacked-up makefile that will probably overwrite random files that it shouldn't in /usr/lib. So I would rather be in favor of coming up with a solution that would make this work rather than removing the options. The solution would probably be adding another option to place the generated files, but the exact behavior would need to be worked out. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
Simon Riggs wrote: On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote: If you are a postgresql hacker at all, or even want to be one, we need your help reviewing patches! There are several "easy" patches in the list, so I can assign them to beginners. It would be a reasonable rule that all patch submitters also have to do patch reviews. If we made it a strict rule, then sponsoring companies would know that they *must* provide money/time for that aspect also. Otherwise it is almost impossible to get formal approval to do that. All this would do is to deter people from submitting patches. Hard rules like this don't work in FOSS communities. I know it's like herding cats, but persuasion is really our only tool. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] code coverage patch
Michelle Caisse wrote: Thanks, I'll take a look at these issues. I have committed your patch with some rework that mainly addresses the concerns also found by Alvaro with regard to cleaning and dependency handling. I have renamed the out target to coverage-html, to be more in line with our other targets. So the workflow is now ./configure --enable-coverage make make check make coverage-html $BROWSER coverage/index.html There are a couple of platform-specific problems that I have come across: * Linux (Debian) works OK * FreeBSD build fails, apparently because libgcov.a was not compiled with -fPIC * Mac OS X builds and runs OK but the server does not shut down in finite time at the end of the regression tests; it has to be killed manually. I have uploaded an example run here: http://developer.postgresql.org/~petere/coverage/ Current test coverage is about 66% overall. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Verbosity of Function Return Type Checks
Volkan YAZICI wrote: > On Thu, 4 Sep 2008, Alvaro Herrera <[EMAIL PROTECTED]> writes: > > Cool, thanks. I had a look and you had some of the expected vs. > > returned reversed. > > I'll happy to fix the reversed ones if you can report them in more > details. Please use the patch I posted yesterday, as it had all the issues I found fixed. There were other changes in that patch too. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: propose to include 3 new functions into intarray and intagg
Hi, Gregory Stark wrote: I definitely like the int_array_append_aggregate function but I don't see anything int[] specific about it. We should be able to have a generic array_union() aggregate which uses the same IsA(fcinfo->context, AggState) trick to scribble on its state variable. It don't even see any reason it couldn't work for arrays of varlenas, though it would take a bit of restructuring. I've just noticed that this already is a todo item: "Add array_accum() and array_to_set() functions for arrays The standards specify array_agg() and UNNEST." See also: http://archives.postgresql.org/pgsql-hackers/2007-08/msg00464.php Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Withdraw PL/Proxy from commitfest
In the previous discussion there was mentioned that Postgres should move to the SQL-MED direction in remote connection handling. SQL-MED specifies that connections should have names and referenced everywhere using names. PL/Proxy currently does not conform to that standard - it uses connection strings directly. Although it could made work with SQL-MED backend, it would look ugly. So I'd like to withdraw PL/Proxy from commitfest and rework it's connection handling scheme to be also name->connstr based. Idea will be that it will have user-definable connection handling backend, which operates on named connections. And in the future we can plug in a backend that reuses connection info from builtin SQL-MED store. Although the current connection handling works and is secure it has a deficiency that it's bit hard to hide the password that is used for connecting. User can either play with table/function permissions and SECURITY DEFINER functions but that's complex. Or he can put passwords into .pgpass - this is easy and secure but has the problem that the file is not manageable from inside database. So PL/Proxy needs new SQL-MED based scheme that fixes it. When this is ready we can re-discuss the builtin vs. PL-based remote functions. As I don't plan to work on it near-term there is no point polluting the commitfest page with it. [ There was a attempt to paint the .pgpass based password handling insecure because dblink makes the file world-readable. I still fail to see how this any way points to flaws of the scheme... ] -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: propose to include 3 new functions into intarray and intagg
Hi, Gregory Stark wrote: The naming 'bidx' seems a bit weired to me, but otherwise I'm also optimistic about it. If we wanted to put that in core Uh.. ATM it's just a patch against contrib. I don't think 'bidx' needs to go into core. Otherwise we'd have to move the whole intarr contrib module as well, no? it would make more sense to have a flag on the array indicating whether it's sorted or not which is maintained whenever we construct or alter an array. Then just have the regular _int_contains() (which is an operator @>) take advantage of it if it's set and the data type is fixed-size. Yeah, that sounds like a good thing. Currently, the intarr module doesn't provide the optimized functions for the outside world (_int_inter_inner() and such.. the _inner appendix really means "inside intarr module only). I've ended up copy'n'pasting that code into my own module, where I take care about ordering myself. But still want to maintain the optimization. However, that's probably not within the scope of this patch. Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: propose to include 3 new functions into intarray and intagg
Markus Wanner <[EMAIL PROTECTED]> writes: > Hi, > > Gregory Stark wrote: >> Regarding the patch listed on the commitfest "3 new functions into intarray >> and intagg" (which I just noticed has a reviewer listed -- doh): > > ..well, just add your name as well, no? Yeah, people should feel free to comment on items even if other people have or are as well. It would have just been more useful for me to pick one that didn't already have someone reading it is all. >> As far as detailed code commentary the only thing which jumps out at me is >> that it's using MemoryContextAlloc to grow the array instead of repalloc >> which >> seems like a waste. This isn't a new thing though, it was how intagg was >> written and this patch just didn't change it. > > Oh, good catch. Actually on further thought what's going on is that it's resizing the array by doubling its size when necessary. palloc/repalloc does that as well, so you're getting two layers of extra space to handle reallocations. I think it's simpler and cleaner to just repalloc just enough space at each growth and let repalloc handle allocating extra space to handle future growth. I think that would be necessary for handling varlenas also. > The naming 'bidx' seems a bit weired to me, but otherwise I'm also optimistic > about it. If we wanted to put that in core it would make more sense to have a flag on the array indicating whether it's sorted or not which is maintained whenever we construct or alter an array. Then just have the regular _int_contains() (which is an operator @>) take advantage of it if it's set and the data type is fixed-size. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
Josh Berkus wrote: Hackers, We currently have 38 patches pending, and only nine people reviewing them. At this rate, the September commitfest will take three months. If you are a postgresql hacker at all, or even want to be one, we need your help reviewing patches! There are several "easy" patches in the list, so I can assign them to beginners. Please volunteer now! Please assign me one; any of the "easy" ones. -- Ibrar Ahmed EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On Fri, 2008-09-05 at 12:18 +0100, Simon Riggs wrote: > > It would be a reasonable rule that all patch submitters also have to > do patch reviews. This is almost the only way to be accepted as a contributor to Fedora -- and I like it. -- Devrim GÜNDÜZ, RHCE devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org signature.asc Description: This is a digitally signed message part
Re: [HACKERS] Need more reviewers!
> That way, instead of just an appeal to the masses to volunteer for > $NEBULOUS_TASK, we can say something like "Please volunteer to review > patches. Doing an initial patch review is easy, please see our guide > to learn more." +1. I'll review a patch if you like, but the patch I have in this 'fest is only one of two I've ever submitted, and my understanding of PG guts is very weak, so I confidently predict that me looking at it has maybe 5% of the value of a committer looking at it, so I'm not sure that really gets us anywhere. Even if I think the patch is great, my opinion doesn't and shouldn't carry much weight compared to someone like Simon or Zdenek who are probably perceived by the committers as much more likely to have a clue, so the committer is just going to end up reviewing it all over again anyway. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
Josh Berkus wrote: Hackers, We currently have 38 patches pending, and only nine people reviewing them. At this rate, the September commitfest will take three months. If you are a postgresql hacker at all, or even want to be one, we need your help reviewing patches! There are several "easy" patches in the list, so I can assign them to beginners. Please volunteer now! Please assign me one; any of the "easy" ones. -- Ibrar Ahmed EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: propose to include 3 new functions into intarray and intagg
Hi, Gregory Stark wrote: Regarding the patch listed on the commitfest "3 new functions into intarray and intagg" (which I just noticed has a reviewer listed -- doh): ..well, just add your name as well, no? I definitely like the int_array_append_aggregate function but I don't see anything int[] specific about it. We should be able to have a generic array_union() aggregate which uses the same IsA(fcinfo->context, AggState) trick to scribble on its state variable. It don't even see any reason it couldn't work for arrays of varlenas, though it would take a bit of restructuring. Yeah, the same idea was bugging me. Doesn't such code already exist? So I would be definitely for a adding this to core if it were rewritten to work with generic arrays which, unless there are problems I'm not seeing, I don't think would be very hard. As far as detailed code commentary the only thing which jumps out at me is that it's using MemoryContextAlloc to grow the array instead of repalloc which seems like a waste. This isn't a new thing though, it was how intagg was written and this patch just didn't change it. Oh, good catch. I'm not against putting more functions into intagg and intarray and bidx and the grouping/counting thing seem like they might be useful functionality. but I have a feeling others might feel differently. The naming 'bidx' seems a bit weired to me, but otherwise I'm also optimistic about it. Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Need more reviewers!
On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote: > If you are a postgresql hacker at all, or even want to be one, we need your > help reviewing patches! There are several "easy" patches in the list, so > I can assign them to beginners. It would be a reasonable rule that all patch submitters also have to do patch reviews. If we made it a strict rule, then sponsoring companies would know that they *must* provide money/time for that aspect also. Otherwise it is almost impossible to get formal approval to do that. I count more than 20 patch authors that are not reviewing, including myself. Of that, there are at least 5 people on their second or subsequent patch (figure probably wildly inaccurate, since don't keep track of that). -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: propose to include 3 new functions into intarray and intagg
Regarding the patch listed on the commitfest "3 new functions into intarray and intagg" (which I just noticed has a reviewer listed -- doh): http://archives.postgresql.org/message-id/[EMAIL PROTECTED] I definitely like the int_array_append_aggregate function but I don't see anything int[] specific about it. We should be able to have a generic array_union() aggregate which uses the same IsA(fcinfo->context, AggState) trick to scribble on its state variable. It don't even see any reason it couldn't work for arrays of varlenas, though it would take a bit of restructuring. So I would be definitely for a adding this to core if it were rewritten to work with generic arrays which, unless there are problems I'm not seeing, I don't think would be very hard. As far as detailed code commentary the only thing which jumps out at me is that it's using MemoryContextAlloc to grow the array instead of repalloc which seems like a waste. This isn't a new thing though, it was how intagg was written and this patch just didn't change it. I'm not against putting more functions into intagg and intarray and bidx and the grouping/counting thing seem like they might be useful functionality. but I have a feeling others might feel differently. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Column-level Privileges
Hi, Stephen Frost wrote: I would suggest you review the updated patch (linked off the wiki page) here: http://archives.postgresql.org/message-id/[EMAIL PROTECTED] That's the patch I've been talking about: file colprivs_wip.20080902.diff.gz, dated Sept, 2nd. It includes my comments about what's done and what's not. I reiterated much of it here as well: http://archives.postgresql.org/pgsql-hackers/2008-09/msg00263.php Uh.. I've read that message as well, but didn't find it overly clear on what you were referring to. As mentioned in above, regression tests, documentation updates, dependency handling, and actually implementing the permission checks all remain. What I'm looking for feedback on are the changes to the grammer, parser, catalog changes, psql output, etc. Aha, good. So I'm going to (try to) check these things and comment. Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Column-level Privileges
Hi Markus, * Markus Wanner ([EMAIL PROTECTED]) wrote: > Stephen Frost wrote: >> Comments welcome, apologies for it not being ready by 9/1. I'm >> planning to continue working on it tomorrow, and throughout September >> as opportunity allows (ie: when Josh isn't keeping me busy). > > I'm trying to review this patch. I could at least compile it > successfully for now. That's a start at least. :) > There have already been some comments from Tom [1]. You've mostly > answered that you are going to fix these issues, but I'm pretty unclear > on what the current status is (of patch 09/02). Can you please elaborate > on what's done and what not? I would suggest you review the updated patch (linked off the wiki page) here: http://archives.postgresql.org/message-id/[EMAIL PROTECTED] It includes my comments about what's done and what's not. I reiterated much of it here as well: http://archives.postgresql.org/pgsql-hackers/2008-09/msg00263.php > As this is still marked as WIP on the wiki page: what needs to be done > until you consider it done? As mentioned in above, regression tests, documentation updates, dependency handling, and actually implementing the permission checks all remain. What I'm looking for feedback on are the changes to the grammer, parser, catalog changes, psql output, etc. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Page layout footprint
Zdenek Kotala wrote: OK. You convinced me that information could be collected from other sources. Great, I'm glad we're in agreement. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Page layout footprint
Heikki Linnakangas napsal(a): Zdenek Kotala wrote: Heikki Linnakangas napsal(a): AFAICS you can get all the same information from pg_controldata. We have a pretty good idea of the alignments of all the usual platforms anyway. If someone says in a bug report that they're running on x86_64 or 32-bit Sparc, we know what the alignments on those platforms are. And if you're working at such a low level, it's not that hard to calculate the offsets within the struct manually. I'm not sure if it is so easy. Are you able do it for SPARC, PPC or other non x86 CPUs? Not off the top of my head. But I am able to do it on x86 and x86_64 which are the platforms I work and debug on. OK. You convinced me that information could be collected from other sources. It could also report changes in alignment. Huh? I would be pretty surprised if the alignment changed randomly on some platform. I thought if somebody change compiler switches - for example 32/64 compilation. But it is probably rare case. Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] For what should pg_stop_backup wait?
On Fri, 2008-08-08 at 11:47 +0900, Fujii Masao wrote: > On Thu, Aug 7, 2008 at 11:34 PM, Simon Riggs <[EMAIL PROTECTED]> wrote: > > > > On Thu, 2008-08-07 at 14:59 +0100, Simon Riggs wrote: > > > >> I'll do a patch. Thanks for your input. > > > > Please review attached patch. > > Thank you for your patch! Would you mind if I asked you to be the official reviewer of this patch for the latest CommitFest? It would help greatly, since you understand the issue and clearly the code also. Thanks, if possible. http://wiki.postgresql.org/wiki/CommitFest:2008-09 -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Page layout footprint
Zdenek Kotala wrote: Heikki Linnakangas napsal(a): I'm afraid I still fail to see the usefulness of this. gdb knows how to deal with structs, If I correct that GDB knows structure only if you have debug version. But usually you don't have debug version on production system. Using gdb without debug systems is pretty much a lost cause anyway. And another small advantage is that --footprint switch is easy to use. It is easier that work with gdb and you can easy get info from users who are not familiar with gdb. AFAICS you can get all the same information from pg_controldata. We have a pretty good idea of the alignments of all the usual platforms anyway. If someone says in a bug report that they're running on x86_64 or 32-bit Sparc, we know what the alignments on those platforms are. And if you're working at such a low level, it's not that hard to calculate the offsets within the struct manually. I'm not sure if it is so easy. Are you able do it for SPARC, PPC or other non x86 CPUs? Not off the top of my head. But I am able to do it on x86 and x86_64 which are the platforms I work and debug on. If we needed more information about the architectures, we could just collect the output of pg_controldata. But I think the configure logs already contains all the useful information. It seems to be good idea. Only what we need is extend buildfarm to parse config.log and shows this data for each build machine. Well, the information is already there. I'm not convinced it's such an important issue that it's worth the effort to add special handling to extract that information from the log. Of course, if someone feels otherwise and does it, I won't object. It could also report changes in alignment. Huh? I would be pretty surprised if the alignment changed randomly on some platform. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Page layout footprint
Gregory Stark napsal(a): Zdenek Kotala <[EMAIL PROTECTED]> writes: Hmm, good question. For example ZFS is platform independent, you can take disk from SPARC machine and plug it into x86 and ZFS works perfectly. FWIW as far as I know *all* filesystems are platform independent. (Of course now someone is surely going to find some counter-example) Doesn't really change the argument though. Yeah, of course. I selected bad word. ZFS write data structures in native endian format. It does not need convert data to correct byte order. Only if you switch harddisk from SPARC to x86 you need convert bytes order. Other systems has a penalty for endian conversion on oposit platform or conversion is not supported. However, I'm not sure if ext3 filesystem which is created on x86 machine is readable under linux on SPARC machine? FAT32 works fine :-) everywhere. Maybe optimized platform independence is better term. Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Page layout footprint
Zdenek Kotala <[EMAIL PROTECTED]> writes: > Hmm, good question. For example ZFS is platform independent, you can take > disk > from SPARC machine and plug it into x86 and ZFS works perfectly. FWIW as far as I know *all* filesystems are platform independent. (Of course now someone is surely going to find some counter-example) Doesn't really change the argument though. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prototype: In-place upgrade
Heikki Linnakangas napsal(a): The patch seems to be missing the new htup.c file. Upps, I'm sorry I'm going to fix it and I will send new version asap. Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Page layout footprint
Heikki Linnakangas napsal(a): Zdenek Kotala wrote: The original proposal (http://archives.postgresql.org/message-id/[EMAIL PROTECTED]) contains two parts. First part is implementation of --footprint cmd line switch which shows you page layout structures footprint. It is useful for development (mostly for in-place upgrade) and also for manual data recovery when you need to know exact structures. I'm afraid I still fail to see the usefulness of this. gdb knows how to deal with structs, If I correct that GDB knows structure only if you have debug version. But usually you don't have debug version on production system. And another small advantage is that --footprint switch is easy to use. It is easier that work with gdb and you can easy get info from users who are not familiar with gdb. and for manual data recovery you need so much more than the page header structure. Yeah, I know, but I didn't want to spend several days with full coding without idea approval. There are special data, meta pages and so on which have to be added. And if you're working at such a low level, it's not that hard to calculate the offsets within the struct manually. I'm not sure if it is so easy. Are you able do it for SPARC, PPC or other non x86 CPUs? BTW, this makes me wonder if it would be possible to use the upgrade-in-place machinery to convert a data directory from one architecture to another? Just a thought.. Hmm, good question. For example ZFS is platform independent, you can take disk from SPARC machine and plug it into x86 and ZFS works perfectly. ZFS converts its own data during a read and any new block is written in a new format. You are able read all binary platform independent data like MP3, JPEG and so on. But PostgreSQL will not work, because PostgreSQL fails during pg_control file reading, because endianes are different. Convert data structures like PageHeader and so on could be possible but you don't have control over user data types. I think in this case is better to develop platform independent replication and use this mechanism for data transfer. However, there is still --footprint switch which is useful and it is reason why I put it on wiki for review and feedback. The switch could also use in build farm for collecting footprints from build farm members. If we needed more information about the architectures, we could just collect the output of pg_controldata. But I think the configure logs already contains all the useful information. It seems to be good idea. Only what we need is extend buildfarm to parse config.log and shows this data for each build machine. It could also report changes in alignment. Zdenek -- Zdenek Kotala Sun Microsystems Prague, Czech Republic http://sun.com/postgresql -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New FSM patch
Simon Riggs wrote: On Thu, 2008-09-04 at 11:07 +0300, Heikki Linnakangas wrote: Scenario: The binary tree on a page is corrupt, so that the value of an upper node is > Max(leftchild, rightchild). Consequence: Searchers will notice the corruption while trying to traverse down that path, and throw an elog(WARNING) in search_avail(). fsm_search will retry the search, and will in worst case go into an infinite loop. That's obviously not good. We could automatically fix the upper nodes of the tree, but that would wipe evidence that would be useful in debugging. We probably need to break out of infinite loops, especially ones that output warning messages on each loop. :-) Yep. I turned that warning into an error in the latest patch I just posted elsewhere in this thread. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Column-level Privileges
Hello Stephen, Stephen Frost wrote: Comments welcome, apologies for it not being ready by 9/1. I'm planning to continue working on it tomorrow, and throughout September as opportunity allows (ie: when Josh isn't keeping me busy). I'm trying to review this patch. I could at least compile it successfully for now. There have already been some comments from Tom [1]. You've mostly answered that you are going to fix these issues, but I'm pretty unclear on what the current status is (of patch 09/02). Can you please elaborate on what's done and what not? As this is still marked as WIP on the wiki page: what needs to be done until you consider it done? Regards Markus Wanner [1]: Comments from Tom: http://archives.postgresql.org/pgsql-patches/2008-05/msg00111.php -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers