Re: [HACKERS] pg_terminate_backend for same-role
On Thu, Mar 15, 2012 at 10:45 PM, Tom Lane wrote: > Daniel Farina writes: >> On Thu, Mar 15, 2012 at 10:33 PM, Tom Lane wrote: >>> But actually I don't see what you hope to gain from such a change, >>> even if it can be made to work. Anyone who can do kill(SIGINT) can >>> do kill(SIGKILL), say --- so you have to be able to trust the signal >>> sender. What's the point of not trusting it to verify the client >>> identity? > >> No longer true with pg_cancel_backend not-by-superuser, no? > > No. That doesn't affect the above argument in the least. And in fact > if there's any question whatsoever as to whether unprivileged > cross-backend signals are secure, they are not going in in the first > place. Okay, well, I believe there is a race in handling common administrative signals that *might* possibly matter. In the past, pg_cancel_backend was superuser only, which is a lot like saying "only available to people who can be the postgres user and run kill." The cancellation packet is handled via first checking the other backend's BackendKey and then SIGINTing it, leaving only the most narrow possibility for a misdirected SIGINT. But it really is unfortunate that I, a user, run a query or have a problematic connection of my own role and just want the thing to stop, but I can't do anything about it without superuser. In recognition of that, pg_cancel_backend now can operate on backends owned by the same user (once again, checked before the signal is processed by the receiver, just like with the cancellation packet). There was some angst (but I'm not sure about how specific or uniform) to extend such signaling power to pg_terminate_backend, and the only objection I can think of is there is this race, or so it would seem to me. Maybe it's too small to care, in which case we can just extend the same policy to pg_terminate_backend, or maybe it's not, in which case we could get rid of any signaling race conditions. The only hypothetical person who would be happy with the current situation, if characterized correctly, would be one who thinks that pid-race on SIGINT from non-superusers (long has been true in the form of the cancellation packet) is okay but on SIGTERM such a race is not. -- fdr -- 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_terminate_backend for same-role
Daniel Farina writes: > On Thu, Mar 15, 2012 at 10:33 PM, Tom Lane wrote: >> But actually I don't see what you hope to gain from such a change, >> even if it can be made to work. Anyone who can do kill(SIGINT) can >> do kill(SIGKILL), say --- so you have to be able to trust the signal >> sender. What's the point of not trusting it to verify the client >> identity? > No longer true with pg_cancel_backend not-by-superuser, no? No. That doesn't affect the above argument in the least. And in fact if there's any question whatsoever as to whether unprivileged cross-backend signals are secure, they are not going in in the first place. 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] pg_terminate_backend for same-role
On Thu, Mar 15, 2012 at 10:33 PM, Tom Lane wrote: > Daniel Farina writes: >> The way MyCancelKey is checked now is backwards, in my mind. It seems >> like it would be better checked by the receiving PID (one can use a >> check/recheck also, if so inclined). Is there a large caveat to that? > > You mean, other than the fact that kill(2) can't transmit such a key? I was planning on using an out-of-line mechanism. Bad idea? > But actually I don't see what you hope to gain from such a change, > even if it can be made to work. Anyone who can do kill(SIGINT) can > do kill(SIGKILL), say --- so you have to be able to trust the signal > sender. What's the point of not trusting it to verify the client > identity? No longer true with pg_cancel_backend not-by-superuser, no? Now there are new people who can do kill(SIGINT) (modulus the already existing cancel requests). -- fdr -- 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_terminate_backend for same-role
Daniel Farina writes: > The way MyCancelKey is checked now is backwards, in my mind. It seems > like it would be better checked by the receiving PID (one can use a > check/recheck also, if so inclined). Is there a large caveat to that? You mean, other than the fact that kill(2) can't transmit such a key? But actually I don't see what you hope to gain from such a change, even if it can be made to work. Anyone who can do kill(SIGINT) can do kill(SIGKILL), say --- so you have to be able to trust the signal sender. What's the point of not trusting it to verify the client identity? 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] pg_terminate_backend for same-role
On Thu, Mar 15, 2012 at 9:39 PM, Fujii Masao wrote: >> Shall we just do everything using the >> MyCancelKey (which I think could just be called "SessionKey", >> "SessionSecret", or even just "Session") as to ensure we have no case >> of mistaken identity? Or does that end up being problematic? > > What if pid is unfortunately reused after passing the test of MyCancelKey > and before sending the signal? The way MyCancelKey is checked now is backwards, in my mind. It seems like it would be better checked by the receiving PID (one can use a check/recheck also, if so inclined). Is there a large caveat to that? I'm working on a small patch to do this I hope to post soon (modulus difficulties), but am fully aware that messing around PGPROC and signal handling can be a bit fiddly. -- fdr -- 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_terminate_backend for same-role
On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina wrote: > Parallel to pg_cancel_backend, it'd be nice to allow the user to just > outright kill a backend that they own (politely, with a SIGTERM), > aborting any transactions in progress, including the idle transaction, > and closing the socket. +1 > I imagine the problem is a race condition whereby a pid might be > reused by another process owned by another user (doesn't that also > affect pg_cancel_backend?). Yes, but I think it's too unlikely to happen. Not sure if we really should worry about that. > Shall we just do everything using the > MyCancelKey (which I think could just be called "SessionKey", > "SessionSecret", or even just "Session") as to ensure we have no case > of mistaken identity? Or does that end up being problematic? What if pid is unfortunately reused after passing the test of MyCancelKey and before sending the signal? Regards, -- 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] patch for parallel pg_dump
On Thu, Mar 15, 2012 at 12:56 AM, Joachim Wieland wrote: > On Wed, Mar 14, 2012 at 2:02 PM, Robert Haas wrote: >> I think we should get rid of die_horribly(), and instead have arrange >> to always clean up AH via an on_exit_nicely hook. > > Good. The only exit handler I've seen so far is > pgdump_cleanup_at_exit. If there's no other one, is it okay to remove > all of this stacking functionality (see on_exit_nicely_index / > MAX_ON_EXIT_NICELY) from dumputils.c and just define two global > variables, one for the function and one for the arg that this function > would operate on (or a struct of both)? No. That code is included by other things - like pg_dumpall - that don't know there's such a thing as an Archive. But I don't see that as a big problem; just on_exit_nicely whatever you want. We could also add on_exit_nicely_reset(), if needed, to clear the existing handlers. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] foreign key locks, 2nd attempt
On Tue, Mar 13, 2012 at 01:46:24PM -0400, Robert Haas wrote: > On Mon, Mar 12, 2012 at 3:28 PM, Simon Riggs wrote: > > I agree with you that some worst case performance tests should be > > done. Could you please say what you think the worst cases would be, so > > those can be tested? That would avoid wasting time or getting anything > > backwards. > > I've thought about this some and here's what I've come up with so far: I question whether we are in a position to do the testing necessary to commit this for 9.2. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 11:04:06PM -0400, Bruce Momjian wrote: > On Tue, Mar 13, 2012 at 02:35:02PM -0300, Alvaro Herrera wrote: > > > > Excerpts from Bruce Momjian's message of mar mar 13 14:00:52 -0300 2012: > > > > > > On Tue, Mar 06, 2012 at 04:39:32PM -0300, Alvaro Herrera wrote: > > > > > > When there is a single locker in a tuple, we can just store the locking > > > > info > > > > in the tuple itself. We do this by storing the locker's Xid in XMAX, > > > > and > > > > setting hint bits specifying the locking strength. There is one > > > > exception > > > > here: since hint bit space is limited, we do not provide a separate > > > > hint bit > > > > for SELECT FOR SHARE, so we have to use the extended info in a > > > > MultiXact in > > > > that case. (The other cases, SELECT FOR UPDATE and SELECT FOR KEY > > > > SHARE, are > > > > presumably more commonly used due to being the standards-mandated > > > > locking > > > > mechanism, or heavily used by the RI code, so we want to provide fast > > > > paths > > > > for those.) > > > > > > Are those tuple bits actually "hint" bits? They seem quite a bit more > > > powerful than a "hint". > > > > I'm not sure what's your point. We've had a "hint" bit for SELECT FOR > > UPDATE for ages. Even 8.2 had HEAP_XMAX_EXCL_LOCK and > > HEAP_XMAX_SHARED_LOCK. Maybe they are misnamed and aren't really > > "hints", but it's not the job of this patch to fix that problem. > > Now I am confused. Where do you see the word "hint" used by > HEAP_XMAX_EXCL_LOCK and HEAP_XMAX_SHARED_LOCK. These are tuple infomask > bits, not hints, meaning they are not optional or there just for > performance. Are you saying that the bit is only a guide and is there only for performance? If so, I understand why it is called "hint". -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] foreign key locks, 2nd attempt
On Tue, Mar 13, 2012 at 02:35:02PM -0300, Alvaro Herrera wrote: > > Excerpts from Bruce Momjian's message of mar mar 13 14:00:52 -0300 2012: > > > > On Tue, Mar 06, 2012 at 04:39:32PM -0300, Alvaro Herrera wrote: > > > > When there is a single locker in a tuple, we can just store the locking > > > info > > > in the tuple itself. We do this by storing the locker's Xid in XMAX, and > > > setting hint bits specifying the locking strength. There is one exception > > > here: since hint bit space is limited, we do not provide a separate hint > > > bit > > > for SELECT FOR SHARE, so we have to use the extended info in a MultiXact > > > in > > > that case. (The other cases, SELECT FOR UPDATE and SELECT FOR KEY SHARE, > > > are > > > presumably more commonly used due to being the standards-mandated locking > > > mechanism, or heavily used by the RI code, so we want to provide fast > > > paths > > > for those.) > > > > Are those tuple bits actually "hint" bits? They seem quite a bit more > > powerful than a "hint". > > I'm not sure what's your point. We've had a "hint" bit for SELECT FOR > UPDATE for ages. Even 8.2 had HEAP_XMAX_EXCL_LOCK and > HEAP_XMAX_SHARED_LOCK. Maybe they are misnamed and aren't really > "hints", but it's not the job of this patch to fix that problem. Now I am confused. Where do you see the word "hint" used by HEAP_XMAX_EXCL_LOCK and HEAP_XMAX_SHARED_LOCK. These are tuple infomask bits, not hints, meaning they are not optional or there just for performance. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Keystone auth in PostgreSQL
Daniel Farina writes: > On Thu, Mar 15, 2012 at 6:38 PM, Tom Lane wrote: >> Our standard answer when someone asks for $random-auth-method is to >> suggest that they find a PAM module for it and use PAM. I wouldn't >> want to claim that PAM is a particularly great interface for this >> sort of thing, but it's out there and I don't know of any serious >> competition. > I considered writing a PAM module to do some stuff at one time (to try > to solve the two-passwords-for-a-user problem), but the non-intrinsic > complexity to perform pretty simple tasks in the whole thing is pretty > terrible -- it ended up being more attractive to do fairly ugly role > mangling in Postgres's own authentication system. And, like you, I > don't know of any serious competition to PAM in performing simple > authentication delegations. Yeah, I've only had to touch our PAM interface a couple of times, but each time I came away thinking "my goodness, that's ugly and over- complicated". I'm not volunteering to build something better, though. 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] Keystone auth in PostgreSQL
On Thu, Mar 15, 2012 at 6:38 PM, Tom Lane wrote: > Our standard answer when someone asks for $random-auth-method is to > suggest that they find a PAM module for it and use PAM. I wouldn't > want to claim that PAM is a particularly great interface for this > sort of thing, but it's out there and I don't know of any serious > competition. I considered writing a PAM module to do some stuff at one time (to try to solve the two-passwords-for-a-user problem), but the non-intrinsic complexity to perform pretty simple tasks in the whole thing is pretty terrible -- it ended up being more attractive to do fairly ugly role mangling in Postgres's own authentication system. And, like you, I don't know of any serious competition to PAM in performing simple authentication delegations. -- fdr -- 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 08:37:36PM -0400, Robert Haas wrote: > On Thu, Mar 15, 2012 at 5:07 PM, Simon Riggs wrote: > > On Wed, Mar 14, 2012 at 5:23 PM, Robert Haas wrote: > >>> You still have HEAP_XMAX_{INVALID,COMMITTED} to reduce the pressure on > >>> mxid > >>> lookups, so I think something more sophisticated is needed to exercise > >>> that > >>> cost. ?Not sure what. > >> > >> I don't think HEAP_XMAX_COMMITTED is much help, because committed != > >> all-visible. > > > > So because committed does not equal all visible there will be > > additional lookups on mxids? That's complete rubbish. > > Noah seemed to be implying that once the updating transaction > committed, HEAP_XMAX_COMMITTED would get set and save the mxid lookup. > But I think that's not true, because anyone who looks at the tuple > afterward will still need to know the exact xmax, to test it against > their snapshot. Yeah, my comment above was wrong. I agree that we'll need to retrieve the mxid members during every MVCC scan until we either mark the page all-visible or have occasion to simplify the mxid xmax to the updater xid. -- 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] Keystone auth in PostgreSQL
Daniel Farina writes: > From my vantage point, a rehash of federated authentication of some > kind would be enormously useful, but it's not really clear if there > are any concrete implementations worth supporting directly: I only > wish it was much easier to delegate authentication so someone could > implement, say, Keystone without excessive contortion. (Or maybe > someone just needs to vend some advice on the "proper" way to > delegate). Our standard answer when someone asks for $random-auth-method is to suggest that they find a PAM module for it and use PAM. I wouldn't want to claim that PAM is a particularly great interface for this sort of thing, but it's out there and I don't know of any serious competition. The alternative of supporting $random-auth-method directly doesn't scale very nicely... 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] foreign key locks, 2nd attempt
Excerpts from Robert Haas's message of jue mar 15 21:37:36 -0300 2012: > > On Thu, Mar 15, 2012 at 5:07 PM, Simon Riggs wrote: > > On Wed, Mar 14, 2012 at 5:23 PM, Robert Haas wrote: > >>> You still have HEAP_XMAX_{INVALID,COMMITTED} to reduce the pressure on > >>> mxid > >>> lookups, so I think something more sophisticated is needed to exercise > >>> that > >>> cost. Not sure what. > >> > >> I don't think HEAP_XMAX_COMMITTED is much help, because committed != > >> all-visible. > > > > So because committed does not equal all visible there will be > > additional lookups on mxids? That's complete rubbish. > > Noah seemed to be implying that once the updating transaction > committed, HEAP_XMAX_COMMITTED would get set and save the mxid lookup. > But I think that's not true, because anyone who looks at the tuple > afterward will still need to know the exact xmax, to test it against > their snapshot. Yeah, we don't set HEAP_XMAX_COMMITTED on multis, even when there's an update in it and it committed. I think we could handle it, at least some of the cases, but that'd require careful re-examination of all the tqual.c code, which is not something I want to do right now. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 5:07 PM, Simon Riggs wrote: > On Wed, Mar 14, 2012 at 5:23 PM, Robert Haas wrote: >>> You still have HEAP_XMAX_{INVALID,COMMITTED} to reduce the pressure on mxid >>> lookups, so I think something more sophisticated is needed to exercise that >>> cost. Not sure what. >> >> I don't think HEAP_XMAX_COMMITTED is much help, because committed != >> all-visible. > > So because committed does not equal all visible there will be > additional lookups on mxids? That's complete rubbish. Noah seemed to be implying that once the updating transaction committed, HEAP_XMAX_COMMITTED would get set and save the mxid lookup. But I think that's not true, because anyone who looks at the tuple afterward will still need to know the exact xmax, to test it against their snapshot. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Storage Manager crash at mdwrite()
On Thu, 2012-03-15 at 19:36 -0400, Tareq Aljabban wrote: > When configuring postgreSQL, I'm adding the libraries needed to run > HDFS C API (libhdfs). > >From the information below, it looks like C++. > > ./configure --prefix=/diskless/myUser/Workspace/EclipseWS1/pgsql > --enable-depend --enable-cassert --enable-debug CFLAGS="$CFLAGS > -I/diskless/myUser/Workspace/HDFS_Append/hdfs/src/c++/libhdfs > -I/usr/lib/jvm/default-java/include" LDFLAGS="$LDFLAGS > -L/diskless/myUser/Workspace/HDFS_Append/hdfs/src/c++/libhdfs > -L/diskless/myUser/Workspace/HDFS_Append/build/c++/Linux-i386-32/lib > -L/usr/lib/jvm/default-java/jre/lib/i386/server -ljvm -lhdfs" > > > > > > I have done lots of changes so far on how the storage manager works. > In fact, I changed smgr.c so instead of calling regular md.c > functions, that it would call my functions . > For simplicity, you can say that whenever mdwrite() was supposed to be > called, another function is also called beside it. so now what is > being called is: > mdwrite(param1, param2, buffer, ); > hdfswrite(param1, param2, buffer, ); > > > where hdfswrite() is the function where I write the buffer to HDFS. > I changed hdfswrite() so it will always write only the same (dummy) > buffer down to HDFS storage. Let's say "dummyBufferFile". After > writing this file 3 times to HDFS, I'm getting the message that I've > shown in my first email. > The same hdfswrite() code works without any issues when I run it in a > separate application. > > > Hope it's clear now. Well, it's clear that there's a lot going on ;) In other words, is there a reason you think that these would all work together nicely? I don't know the specifics, but I understand there are numerous perils to linking complex C++ code into complex C code, particularly around exception handling. Look in the archives for previous discussions around C++. 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] Storage Manager crash at mdwrite()
When configuring postgreSQL, I'm adding the libraries needed to run HDFS C API (libhdfs). ./configure --prefix=/diskless/myUser/Workspace/EclipseWS1/pgsql --enable-depend --enable-cassert --enable-debug CFLAGS="$CFLAGS -I/diskless/myUser/Workspace/HDFS_Append/hdfs/src/c++/libhdfs -I/usr/lib/jvm/default-java/include" LDFLAGS="$LDFLAGS -L/diskless/myUser/Workspace/HDFS_Append/hdfs/src/c++/libhdfs -L/diskless/myUser/Workspace/HDFS_Append/build/c++/Linux-i386-32/lib -L/usr/lib/jvm/default-java/jre/lib/i386/server -ljvm -lhdfs" I have done lots of changes so far on how the storage manager works. In fact, I changed smgr.c so instead of calling regular md.c functions, that it would call my functions . For simplicity, you can say that whenever mdwrite() was supposed to be called, another function is also called beside it. so now what is being called is: mdwrite(param1, param2, buffer, ); hdfswrite(param1, param2, buffer, ); where hdfswrite() is the function where I write the buffer to HDFS. I changed hdfswrite() so it will always write only the same (dummy) buffer down to HDFS storage. Let's say "dummyBufferFile". After writing this file 3 times to HDFS, I'm getting the message that I've shown in my first email. The same hdfswrite() code works without any issues when I run it in a separate application. Hope it's clear now. On Thu, Mar 15, 2012 at 5:28 PM, Jeff Davis wrote: > On Thu, 2012-03-15 at 13:49 -0400, Tareq Aljabban wrote: > > I'm implementing an extention to mdwrite() at > > backend/storage/smgr/md.c > > When a block is written to the local storage using mdwrite(), I'm > > sending this block to an HDFS storage. > > So far I don't need to read back the values I'm writing to HDFS. This > > approach is working fine in the initDB phase. > > However, when I'm running postgres (bin/pg_ctl start), the first few > > write operations run successfully, and then suddenly (after writing > > exactly 3 files to HDFS), I get a 130 exit code with the following > > message showing the JVM thread dump of HDFS: > > > > > > LOG: background writer process (PID 29347) exited with exit code 130 > > LOG: terminating any other active server processes > > ... > > > This seems like an HDFS issue, but the same code worked properly in > > initDB(). I replaced this HDFS write code with a code that writes > > always the same block (empty one) to HDFS regardless from the value > > received by mdwrite().. Kept getting the same issue after writing 3 > > files. > > I copied this exact code to a separate C application and ran it there > > and it executed without any problems (I wrote/deleted 100 files). > > That's why I'm doubting that it's something related to postgreSQL. > > > > > > Any ideas on what's going wrong? > > What code, exactly, did you change in md.c, and anywhere else in > postgres? Are you linking in new libraries/code from somewhere into the > postgres backend? > > Regards, > Jeff Davis > >
[HACKERS] pg_terminate_backend for same-role
Parallel to pg_cancel_backend, it'd be nice to allow the user to just outright kill a backend that they own (politely, with a SIGTERM), aborting any transactions in progress, including the idle transaction, and closing the socket. I imagine the problem is a race condition whereby a pid might be reused by another process owned by another user (doesn't that also affect pg_cancel_backend?). Shall we just do everything using the MyCancelKey (which I think could just be called "SessionKey", "SessionSecret", or even just "Session") as to ensure we have no case of mistaken identity? Or does that end up being problematic? -- fdr -- 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] Command Triggers, v16
On 15 March 2012 22:06, Dimitri Fontaine wrote: > Dimitri Fontaine writes: >>> At this moment in time, CTAS is still outstanding. Is the plan to try >>> to get that in for this release, or as an enhancement in 9.3? >> >> The plan is to get CTAS as a utility command in 9.2 then update the >> command trigger patch to benefit from the new situation. We've been >> wondering about making its own commit fest entry for that patch, it's >> now clear in my mind, that needs to happen. > > https://commitfest.postgresql.org/action/patch_view?id=823 Looks like the ctas-on-command-triggers-01.patch patch needs re-basing. Thom -- 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] Faster compression, again
On Thu, Mar 15, 2012 at 10:14:12PM +, Simon Riggs wrote: > On Wed, Mar 14, 2012 at 6:06 PM, Daniel Farina wrote: > > > If we're curious how it affects replication > > traffic, I could probably gather statistics on LZO-compressed WAL > > traffic, of which we have a pretty huge amount captured. > > What's the compression like for shorter chunks of data? Is it worth > considering using this for the libpq copy protocol and therefore > streaming replication also? > > -- > Simon Riggs http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services Here is a pointer to some tests with Snappy+CouchDB: https://github.com/fdmanana/couchdb/blob/b8f806e41727ba18ed6143cee31a3242e024ab2c/snappy-couch-tests.txt They checked compression on smaller chunks of data. I have extracted the basic results. The first number is the original size in bytes, followed by the compressed size in bytes, the percent compressed and the compression ratio: 77 -> 60, 90% or 1.1:1 120 -> 104, 87% or 1.15:1 127 -> 80, 63% or 1.6:1 5942 -> 2930, 49% or 2:1 It looks like a good candidate for both the libpq copy protocol and streaming replication. My two cents. Regards, Ken -- 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] Faster compression, again
On Thu, Mar 15, 2012 at 3:14 PM, Simon Riggs wrote: > On Wed, Mar 14, 2012 at 6:06 PM, Daniel Farina wrote: > >> If we're curious how it affects replication >> traffic, I could probably gather statistics on LZO-compressed WAL >> traffic, of which we have a pretty huge amount captured. > > What's the compression like for shorter chunks of data? Is it worth > considering using this for the libpq copy protocol and therefore > streaming replication also? The overhead is between 1 and 5 bytes that reserve the high bit as a continuation bit (so one byte for small data), and then straight into data. So I think it could be applied for most payloads that are a few bytes wide. Presumably that could be lifted, but the format description only allows for 2**32 - 1 for the uncompressed size. I'd really like to find a way to layer both message-oblivious and message-aware transport under FEBE with both backend and frontend support without committing the project to new code for-ever-and-ever. I guess I could investigate it in brief now, unless you've already thought about/done some work in that area. -- fdr -- 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 10:13 PM, Alvaro Herrera wrote: > > Excerpts from Simon Riggs's message of jue mar 15 19:04:41 -0300 2012: >> >> On Thu, Mar 15, 2012 at 9:54 PM, Alvaro Herrera >> wrote: >> > >> > Excerpts from Simon Riggs's message of jue mar 15 18:38:53 -0300 2012: >> >> On Thu, Mar 15, 2012 at 2:26 AM, Robert Haas >> >> wrote: >> > >> >> > But that would only make sense if >> >> > we thought that getting rid of the fsyncs would be more valuable than >> >> > avoiding the blocking here, and I don't. >> >> >> >> You're right that the existing code could use some optimisation. >> >> >> >> I'm a little tired, but I can't see a reason to fsync this except at >> >> checkpoint. >> > >> > Hang on. What fsyncs are we talking about? I don't see that the >> > multixact code calls any fsync except that checkpoint and shutdown. >> >> If a dirty page is evicted it will fsync. > > Ah, right. > >> >> Also seeing that we issue 2 WAL records for each RI check. We issue >> >> one during MultiXactIdCreate/MultiXactIdExpand and then immediately >> >> afterwards issue a XLOG_HEAP_LOCK record. The comments on both show >> >> that each thinks it is doing it for the same reason and is the only >> >> place its being done. Alvaro, any ideas why that is. >> > >> > AFAIR the XLOG_HEAP_LOCK log entry only records the fact that the row is >> > being locked by a multixact -- it doesn't record the contents (member >> > xids) of said multixact, which is what the other log entry records. >> >> Agreed. But issuing two records when we could issue just one seems a >> little strange, especially when the two record types follow one >> another so closely - so we end up queuing for the lock twice while >> holding the lock on the data block. > > Hmm, that seems optimization that could be done separately. Oh yes, definitely not something for you to add to the main patch. Just some additional tuning to alleviate Robert's concerns. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & 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] logging in high performance systems.
On Fri, Mar 9, 2012 at 9:53 AM, Robert Haas wrote: > Tom recently committed something by another author that is along > similar lines to what you have here (I think). Can you comment on > whether you think more is still needed and what the differences are > between that approach and yours? Hearing no response, I am marking this Returned with Feedback. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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_stats_recovery view
On Fri, Mar 9, 2012 at 7:20 AM, Fujii Masao wrote: > On Tue, Feb 14, 2012 at 4:10 PM, Jaime Casanova wrote: >> On Thu, Feb 2, 2012 at 2:32 AM, Magnus Hagander wrote: >>> >>> I haven't looked through the code in detail, but one direct comment: >>> do we really need/want to send this through the stats collector? It >>> will only ever have one sender - perhaps we should just either store >>> it in shared memory or store it locally and only send it on demand? >>> >> >> fyi, i intend to send a reworked patch later today, it will store the >> info locally and send it on demand. > > Jaime, > are you planning to submit the updated version of the patch? Hearing no response, I have marked this patch Returned with Feedback. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Faster compression, again
On Wed, Mar 14, 2012 at 6:06 PM, Daniel Farina wrote: > If we're curious how it affects replication > traffic, I could probably gather statistics on LZO-compressed WAL > traffic, of which we have a pretty huge amount captured. What's the compression like for shorter chunks of data? Is it worth considering using this for the libpq copy protocol and therefore streaming replication also? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & 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] foreign key locks, 2nd attempt
Excerpts from Simon Riggs's message of jue mar 15 19:04:41 -0300 2012: > > On Thu, Mar 15, 2012 at 9:54 PM, Alvaro Herrera > wrote: > > > > Excerpts from Simon Riggs's message of jue mar 15 18:38:53 -0300 2012: > >> On Thu, Mar 15, 2012 at 2:26 AM, Robert Haas wrote: > > > >> > But that would only make sense if > >> > we thought that getting rid of the fsyncs would be more valuable than > >> > avoiding the blocking here, and I don't. > >> > >> You're right that the existing code could use some optimisation. > >> > >> I'm a little tired, but I can't see a reason to fsync this except at > >> checkpoint. > > > > Hang on. What fsyncs are we talking about? I don't see that the > > multixact code calls any fsync except that checkpoint and shutdown. > > If a dirty page is evicted it will fsync. Ah, right. > >> Also seeing that we issue 2 WAL records for each RI check. We issue > >> one during MultiXactIdCreate/MultiXactIdExpand and then immediately > >> afterwards issue a XLOG_HEAP_LOCK record. The comments on both show > >> that each thinks it is doing it for the same reason and is the only > >> place its being done. Alvaro, any ideas why that is. > > > > AFAIR the XLOG_HEAP_LOCK log entry only records the fact that the row is > > being locked by a multixact -- it doesn't record the contents (member > > xids) of said multixact, which is what the other log entry records. > > Agreed. But issuing two records when we could issue just one seems a > little strange, especially when the two record types follow one > another so closely - so we end up queuing for the lock twice while > holding the lock on the data block. Hmm, that seems optimization that could be done separately. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] Command Triggers, v16
Dimitri Fontaine writes: >> At this moment in time, CTAS is still outstanding. Is the plan to try >> to get that in for this release, or as an enhancement in 9.3? > > The plan is to get CTAS as a utility command in 9.2 then update the > command trigger patch to benefit from the new situation. We've been > wondering about making its own commit fest entry for that patch, it's > now clear in my mind, that needs to happen. https://commitfest.postgresql.org/action/patch_view?id=823 Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 9:54 PM, Alvaro Herrera wrote: > > Excerpts from Simon Riggs's message of jue mar 15 18:38:53 -0300 2012: >> On Thu, Mar 15, 2012 at 2:26 AM, Robert Haas wrote: > >> > But that would only make sense if >> > we thought that getting rid of the fsyncs would be more valuable than >> > avoiding the blocking here, and I don't. >> >> You're right that the existing code could use some optimisation. >> >> I'm a little tired, but I can't see a reason to fsync this except at >> checkpoint. > > Hang on. What fsyncs are we talking about? I don't see that the > multixact code calls any fsync except that checkpoint and shutdown. If a dirty page is evicted it will fsync. >> Also seeing that we issue 2 WAL records for each RI check. We issue >> one during MultiXactIdCreate/MultiXactIdExpand and then immediately >> afterwards issue a XLOG_HEAP_LOCK record. The comments on both show >> that each thinks it is doing it for the same reason and is the only >> place its being done. Alvaro, any ideas why that is. > > AFAIR the XLOG_HEAP_LOCK log entry only records the fact that the row is > being locked by a multixact -- it doesn't record the contents (member > xids) of said multixact, which is what the other log entry records. Agreed. But issuing two records when we could issue just one seems a little strange, especially when the two record types follow one another so closely - so we end up queuing for the lock twice while holding the lock on the data block. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & 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] Command Triggers, v16
Thanks for testing this new version (again). A quick answer now, I'll send another patch tomorrow. Thom Brown writes: > I don’t understand how functions can return a type of “command > trigger”. This certainly works, but I’ve never seen a type consisting > of more than one word. Could you explain this for me? This is also I tricked that in the grammar, the type is called cmdtrigger but I though it wouldn't be a good choice for the SQL statement. > at odds with the error message in src/backend/commands/cmdtrigger.c: > > errmsg("function \"%s\" must return type \"trigger\"", I realized I needed another trigger like data type after I had worked this message, I need to get back on it, thanks. > At this moment in time, CTAS is still outstanding. Is the plan to try > to get that in for this release, or as an enhancement in 9.3? The plan is to get CTAS as a utility command in 9.2 then update the command trigger patch to benefit from the new situation. We've been wondering about making its own commit fest entry for that patch, it's now clear in my mind, that needs to happen. Stay tuned, follow up email due tomorrow. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] foreign key locks, 2nd attempt
Excerpts from Simon Riggs's message of jue mar 15 18:46:44 -0300 2012: > > On Thu, Mar 15, 2012 at 1:17 AM, Alvaro Herrera > wrote: > > > As things stand today > > Can I confirm where we are now? Is there another version of the patch > coming out soon? Yes, another version is coming soon. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] foreign key locks, 2nd attempt
Excerpts from Simon Riggs's message of jue mar 15 18:38:53 -0300 2012: > On Thu, Mar 15, 2012 at 2:26 AM, Robert Haas wrote: > > But that would only make sense if > > we thought that getting rid of the fsyncs would be more valuable than > > avoiding the blocking here, and I don't. > > You're right that the existing code could use some optimisation. > > I'm a little tired, but I can't see a reason to fsync this except at > checkpoint. Hang on. What fsyncs are we talking about? I don't see that the multixact code calls any fsync except that checkpoint and shutdown. > Also seeing that we issue 2 WAL records for each RI check. We issue > one during MultiXactIdCreate/MultiXactIdExpand and then immediately > afterwards issue a XLOG_HEAP_LOCK record. The comments on both show > that each thinks it is doing it for the same reason and is the only > place its being done. Alvaro, any ideas why that is. AFAIR the XLOG_HEAP_LOCK log entry only records the fact that the row is being locked by a multixact -- it doesn't record the contents (member xids) of said multixact, which is what the other log entry records. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] Faster compression, again
On Wed, Mar 14, 2012 at 11:06 AM, Daniel Farina wrote: >...and it has been ported to C (recently, and with some > quirks, like no LICENSE file...yet, although it is linked from the > original Snappy project). I poked the author about the license and he fixed it in a jiffy. Now under BSD, with Intel's Copyright. He seems to be committing a few enhancements, but the snail's trail of the Internet suggests that this code has made its way into Linux as well, including btrfs. So now I guess we can have at it... https://github.com/andikleen/snappy-c/ -- fdr -- 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] CREATE FOREGIN TABLE LACUNA
On Thu, Mar 15, 2012 at 11:23:43AM -0300, Alvaro Herrera wrote: > Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012: > > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote: > > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: > > > >> I think that instead of inventing new grammar productions and a new > > > >> node type for this, you should just reuse the existing productions for > > > >> LIKE clauses and then reject invalid options during parse analysis. > > > > > > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and > > > > submit that as a separate patch? > > > > > > I don't see any reason to do that. I merely meant that you could > > > reuse TableLikeClause or maybe even TableElement in the grammer for > > > CreateForeignTableStmt. > > > > Next WIP patch attached implementing this via reusing TableLikeClause > > and refactoring transformTableLikeClause(). > > > > What say? > > Looks much better to me, but the use of strcmp() doesn't look good. > ISTM that stmtType is mostly used for error messages. Is it used for anything at all? > I think you should add some kind of identifier (such as the original > parser Node) into the CreateStmtContext so that you can do a IsA() > test instead -- a bit more invasive as a patch, but much cleaner. OK Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 1:17 AM, Alvaro Herrera wrote: > As things stand today Can I confirm where we are now? Is there another version of the patch coming out soon? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 2:26 AM, Robert Haas wrote: > On Wed, Mar 14, 2012 at 9:17 PM, Alvaro Herrera > wrote: >>> > Agreed. But speaking of that, why exactly do we fsync the multixact SLRU >>> > today? >>> >>> Good question. So far, I can't think of a reason. "nextMulti" is critical, >>> but we already fsync it with pg_control. We could delete the other >>> multixact >>> state data at every startup and set OldestVisibleMXactId accordingly. >> >> Hmm, yeah. > > In a way, the fact that we don't do that is kind of fortuitous in this > situation. I had just assumed that we were not fsyncing it because > there seems to be no reason to do so. But since we are, we already > know that the fsyncs resulting from frequent mxid allocation aren't a > huge pain point. If they were, somebody would have presumably > complained about it and fixed it before now. So that means that what > we're really worrying about here is the overhead of fsyncing a little > more often, which is a lot less scary than starting to do it when we > weren't previously. Good > Now, we could look at this as an opportunity to optimize the existing > implementation by removing the fsyncs, rather than adding the new > infrastructure Alvaro is proposing. This is not an exercise in tuning mxact code. There is a serious algorithmic problem that is causing real world problems. Removing the fsync will *not* provide a solution to the problem, so there is no "opportunity" here. > But that would only make sense if > we thought that getting rid of the fsyncs would be more valuable than > avoiding the blocking here, and I don't. You're right that the existing code could use some optimisation. I'm a little tired, but I can't see a reason to fsync this except at checkpoint. Also seeing that we issue 2 WAL records for each RI check. We issue one during MultiXactIdCreate/MultiXactIdExpand and then immediately afterwards issue a XLOG_HEAP_LOCK record. The comments on both show that each thinks it is doing it for the same reason and is the only place its being done. Alvaro, any ideas why that is. > I still think that someone needs to do some benchmarking here, because > this is a big complicated performance patch, and we can't predict the > impact of it on real-world scenarios without testing. There is > clearly some additional overhead, and it makes sense to measure it and > hopefully discover that it isn't excessive. Still, I'm a bit > relieved. Very much agreed. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & 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] Another review of URI for libpq, v7 submission
Daniel Farina writes: > >> Finally, attached is v8. Hopefully I didn't mess things up too much. > > I'll give it another look-over. Do you have these in git somewhere? It > will help me save time on some of the incremental changes. Yes, I've just pushed my dev branch to this fork of mine: https://github.com/a1exsh/postgres/commits/uri -- 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] Storage Manager crash at mdwrite()
On Thu, 2012-03-15 at 13:49 -0400, Tareq Aljabban wrote: > I'm implementing an extention to mdwrite() at > backend/storage/smgr/md.c > When a block is written to the local storage using mdwrite(), I'm > sending this block to an HDFS storage. > So far I don't need to read back the values I'm writing to HDFS. This > approach is working fine in the initDB phase. > However, when I'm running postgres (bin/pg_ctl start), the first few > write operations run successfully, and then suddenly (after writing > exactly 3 files to HDFS), I get a 130 exit code with the following > message showing the JVM thread dump of HDFS: > > > LOG: background writer process (PID 29347) exited with exit code 130 > LOG: terminating any other active server processes ... > This seems like an HDFS issue, but the same code worked properly in > initDB(). I replaced this HDFS write code with a code that writes > always the same block (empty one) to HDFS regardless from the value > received by mdwrite().. Kept getting the same issue after writing 3 > files. > I copied this exact code to a separate C application and ran it there > and it executed without any problems (I wrote/deleted 100 files). > That's why I'm doubting that it's something related to postgreSQL. > > > Any ideas on what's going wrong? What code, exactly, did you change in md.c, and anywhere else in postgres? Are you linking in new libraries/code from somewhere into the postgres backend? 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] foreign key locks, 2nd attempt
On Thu, Mar 15, 2012 at 2:15 AM, Robert Haas wrote: > On Wed, Mar 14, 2012 at 6:10 PM, Noah Misch wrote: >>> Well, post-release, the cat is out of the bag: we'll be stuck with >>> this whether the performance characteristics are acceptable or not. >>> That's why we'd better be as sure as possible before committing to >>> this implementation that there's nothing we can't live with. It's not >>> like there's any reasonable way to turn this off if you don't like it. >> >> I disagree; we're only carving in stone the FOR KEY SHARE and FOR KEY UPDATE >> syntax additions. We could even avoid doing that by not documenting them. A >> later major release could implement them using a completely different >> mechanism or even reduce them to aliases, KEY SHARE = SHARE and KEY UPDATE = >> UPDATE. To be sure, let's still do a good job the first time. > > What I mean is really that, once the release is out, we don't get to > take it back. Sure, the next release can fix things, but any > regressions will become obstacles to upgrading and pain points for new > users. This comment is completely superfluous. It's a complete waste of time to turn up on a thread and remind people that if they commit something and it doesn't actually work that it would be a bad thing. Why, we might ask do you think that thought needs to be expressed here? Please, don't answer, lets spend the time on actually reviewing the patch. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & 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] Command Triggers, v16
On 15 March 2012 18:13, Dimitri Fontaine wrote: > Hi, > > I guess it's time to start a new thread here. Please find attached > version 16 of the command trigger patch, with augmented documentation > and “magic variable” support (TG_WHEN, TG_OBJECTID and such). > > The current version of the patch only supports PLpgSQL, I need to add > support for the other languages in core but I though Thom would like to > be able to play with a new patch before I finish plpython, plperl and > pltcl support. > > This patch also includes edits following latest reviews from both Thom, > Andres and Robert, in particular ANY command triggers are now called > from the same place as specific command triggers and receive the same > parameters. Good to see that ANY COMMAND triggers contain everything the specific triggers have. I've completed a complete re-run of all my testing. Note: incremental patch attached for the following section... -START The docs have an excessive opening tag. The docs also list ALTER CAST as an option, which it isn't. There's an old version of a paragraph included, immediately followed by its revised version. It begins with "Triggers on ANY command...". The example given for the abort_any_command function has a typo. The RAISE statement should have a comma after the closing single quote instead of %. In doc/src/sgml/plpgsql.sgml: “The command trigger function return's value is not used.” should be “The command trigger function’s return value is not used.” “This example trigger just raise a...” should be “This example trigger just raises a...” The example procedure isn't called correctly in the create command trigger statement below it. It refers to it at "any_snitch", but the function is just named "snitch". Also the style is inconsistent with the other trigger functions further up the page, such as putting the function language last, showing the return type on the same line as the CREATE FUNCTION line and using upper-case lettering for keywords. I don’t understand how functions can return a type of “command trigger”. This certainly works, but I’ve never seen a type consisting of more than one word. Could you explain this for me? This is also at odds with the error message in src/backend/commands/cmdtrigger.c: errmsg("function \"%s\" must return type \"trigger\"", Should be “command trigger” as a regular trigger can’t be used on command triggers. END At this moment in time, CTAS is still outstanding. Is the plan to try to get that in for this release, or as an enhancement in 9.3? I don’t know if this was a problem before that I didn’t spot (probably), but triggers for both ANY COMMAND and ALTER FOREIGN TABLE show a command tag of ALTER TABLE for ALTER FOREIGN TABLE statements where the column is renamed: thom@test=# ALTER FOREIGN TABLE test.dict2 RENAME COLUMN word TO words; NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER TABLE' objectid=16569 schemaname='test' objectname='dict2' NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='ALTER TABLE' objectid=16569 schemaname='test' objectname='dict2' NOTICE: Command trigger: tg_when='AFTER' cmd_tag='ALTER TABLE' objectid=16569 schemaname='test' objectname='dict2' NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='ALTER TABLE' objectid=16569 schemaname='test' objectname='dict2' ALTER TABLE I don’t think this is the fault of the trigger code because it actually says ALTER TABLE at the bottom, suggesting it’s something already present. This isn’t the case when adding or dropping columns. Any comments? Altering the properties of a function (such as cost, security definer, whether it’s stable etc) doesn’t report the function’s OID: thom@test=# ALTER FUNCTION test.testfunc2() COST 77; NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER FUNCTION' objectid= schemaname='test' objectname='testfunc2' NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='ALTER FUNCTION' objectid= schemaname='test' objectname='testfunc2' NOTICE: Command trigger: tg_when='AFTER' cmd_tag='ALTER FUNCTION' objectid= schemaname='test' objectname='testfunc2' NOTICE: Command trigger on any: tg_when='AFTER' cmd_tag='ALTER FUNCTION' objectid= schemaname='test' objectname='testfunc2' ALTER FUNCTION I get a garbage objectname for AFTER ANY COMMAND triggers on ALTER TEXT SEARCH DICTIONARY when changing its options. It doesn’t show it in the below example because I can’t get it displaying in plain text, but where the objectname is blank is where I’m seeing unicode a square containing “0074” 63 times in a row: thom@test=# ALTER TEXT SEARCH DICTIONARY testnew.test_stem2 ( StopWords = dutch ); NOTICE: Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER TEXT SEARCH DICTIONARY' objectid=16617 schemaname='testnew' objectname='test_stem2' NOTICE: Command trigger: tg_when='BEFORE' cmd_tag='ALTER TEXT SEARCH DICTIONARY' objectid=16617 schemaname='testnew' objectname='test_stem2' NOTICE: Command trigger: tg_when
Re: [HACKERS] Keystone auth in PostgreSQL
On Thu, Mar 15, 2012 at 1:14 PM, Bruce Momjian wrote: > On Wed, Mar 14, 2012 at 11:38:19AM +0530, Vivek Singh Raghuwanshi wrote: >> Hi All, >> >> Can i use keystone auth with PostgreSQL, it is very helpful when i am >> using OpenStack as a cloud service and implement DBaaS. > > I don't think so. I have never heard of keystone auth: > > http://www.bitkoo.com/products-keystone-how-it-works.php Semantically overloaded, because I believe it refers to this: http://keystone.openstack.org/ From my vantage point, a rehash of federated authentication of some kind would be enormously useful, but it's not really clear if there are any concrete implementations worth supporting directly: I only wish it was much easier to delegate authentication so someone could implement, say, Keystone without excessive contortion. (Or maybe someone just needs to vend some advice on the "proper" way to delegate). -- fdr -- 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] Another review of URI for libpq, v7 submission
On Thu, Mar 15, 2012 at 7:36 AM, Alex Shulgin wrote: > I wonder if there's any evidence as to that mangling the email addresses > helps to reduce spam at all? I mean replacing "(at)" back to "@" and > "(dot)" to "." is piece of cake for a spam crawler. I suspect we're long past the point in Internet history where such simple obfuscation could possibly matter. > Finally, attached is v8. Hopefully I didn't mess things up too much. I'll give it another look-over. Do you have these in git somewhere? It will help me save time on some of the incremental changes. -- fdr -- 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] foreign key locks, 2nd attempt
On Wed, Mar 14, 2012 at 5:23 PM, Robert Haas wrote: >> You still have HEAP_XMAX_{INVALID,COMMITTED} to reduce the pressure on mxid >> lookups, so I think something more sophisticated is needed to exercise that >> cost. Not sure what. > > I don't think HEAP_XMAX_COMMITTED is much help, because committed != > all-visible. So because committed does not equal all visible there will be additional lookups on mxids? That's complete rubbish. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & 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] ECPG FETCH readahead
On Tue, Mar 6, 2012 at 6:06 AM, Noah Misch wrote: > On Tue, Mar 06, 2012 at 07:07:41AM +0100, Boszormenyi Zoltan wrote: >> 2012-03-05 19:56 keltez?ssel, Noah Misch ?rta: >> >> Or how about a new feature in the backend, so ECPG can do >> >> UPDATE/DELETE ... WHERE OFFSET N OF cursor >> >> and the offset of computed from the actual cursor position and the >> >> position known >> >> by the application? This way an app can do readahead and do work on rows >> >> collected >> >> by the cursor with WHERE CURRENT OF which gets converted to WHERE OFFSET >> >> OF >> >> behind the scenes. >> > That's a neat idea, but I would expect obstacles threatening our ability to >> > use it automatically for readahead. You would have to make the cursor a >> > SCROLL cursor. We'll often pass a negative offset, making the operation >> > fail >> > if the cursor query used FOR UPDATE. Volatile functions in the query will >> > get >> > more calls. That's assuming the operation will map internally to something >> > like MOVE N; UPDATE ... WHERE CURRENT OF; MOVE -N. You might come up with >> > innovations to mitigate those obstacles, but those innovations would >> > probably >> > also apply to MOVE/FETCH. In any event, this would constitute a >> > substantive >> > patch in its own right. >> >> I was thinking along the lines of a Portal keeping the ItemPointerData >> for each tuple in the last FETCH statement. The WHERE OFFSET N OF cursor >> would treat the offset value relative to the tuple order returned by FETCH. >> So, OFFSET 0 OF == CURRENT OF and other values of N are negative. >> This way, it doesn't matter if the cursor is SCROLL, NO SCROLL or have >> the default behaviour with "SCROLL in some cases". Then ECPGopen() >> doesn't have to play games with the DECLARE statement. Only ECPGfetch() >> needs to play with MOVE statements, passing different offsets to the backend, >> not what the application passed. > > That broad approach sounds promising. The main other consideration that comes > to mind is a plan to limit resource usage for a cursor that reads, say, 1B > rows. However, I think attempting to implement this now will significantly > decrease the chance of getting the core patch features committed now. > >> > One way out of trouble here is to make WHERE CURRENT OF imply READHEAD >> > 1/READHEAD 0 (incidentally, perhaps those two should be synonyms) on the >> > affected cursor. If the cursor has some other readahead quantity declared >> > explicitly, throw an error during preprocessing. >> >> I played with this idea a while ago, from a different point of view. >> If the ECPG code had the DECLARE mycur, DML ... WHERE CURRENT OF mycur >> and OPEN mycur in exactly this order, i.e. WHERE CURRENT OF appears in >> a standalone function between DECLARE and the first OPEN for the cursor, >> then ECPG disabled readahead automatically for that cursor and for that >> cursor only. But this requires effort on the user of ECPG and can be very >> fragile. Code cleanup with reordering functions can break previously >> working code. > > Don't the same challenges apply to accurately reporting an error when the user > specifies WHERE CURRENT OF for a readahead cursor? I think we need either an updated version of this patch that's ready for commit real soon now, or we need to postpone it to 9.3. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] [v9.2] Add GUC sepgsql.client_label
On Wed, Mar 14, 2012 at 11:10 AM, Kohei KaiGai wrote: > If it is ready to commit, please remember the credit to Yeb's volunteer > on this patch. Done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Keystone auth in PostgreSQL
On Wed, Mar 14, 2012 at 11:38:19AM +0530, Vivek Singh Raghuwanshi wrote: > Hi All, > > Can i use keystone auth with PostgreSQL, it is very helpful when i am > using OpenStack as a cloud service and implement DBaaS. I don't think so. I have never heard of keystone auth: http://www.bitkoo.com/products-keystone-how-it-works.php -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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: autocomplete for functions
Excerpts from Peter Eisentraut's message of jue mar 15 16:25:53 -0300 2012: > On sön, 2012-02-19 at 20:10 +0100, Pavel Stehule wrote: > > I found so this extremely simple patch should be useful. > > > > It helps for pattern SELECT fx(); > > Isn't that just a subset of what I had proposed? > > http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net So do you intend to commit your patch? -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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: autocomplete for functions
2012/3/15 Peter Eisentraut : > On sön, 2012-02-19 at 20:10 +0100, Pavel Stehule wrote: >> I found so this extremely simple patch should be useful. >> >> It helps for pattern SELECT fx(); > > Isn't that just a subset of what I had proposed? > > http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net > >> There was thread about it. > > Which thread was that? > > probably yours :) Regards Pavel -- 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: autocomplete for functions
On sön, 2012-02-19 at 20:10 +0100, Pavel Stehule wrote: > I found so this extremely simple patch should be useful. > > It helps for pattern SELECT fx(); Isn't that just a subset of what I had proposed? http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net > There was thread about it. Which thread was that? -- 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_upgrade and statistics
Excerpts from Greg Stark's message of jue mar 15 14:45:16 -0300 2012: > On Thu, Mar 15, 2012 at 3:15 PM, Andrew Dunstan wrote: > > > > You're not the only person who could do that. I don't think this is all down > > to you. It should just be understood that if the stats format is changed, > > adjusting pg_upgrade needs to be part of the change. When we modified how > > enums worked, we adjusted pg_upgrade at the same time. That sort of thing > > seems totally reasonable to me. > > > > I haven't looked at it, but I'm wondering how hard it is going to be in > > practice? > > Historically pg_statistic has been pretty static. But that seems like > a negative, not a positive -- a big part of my fear here is that it > really really needs a lot of work to improve the statistics. I *hope* > they get knocked around dramatically in each of the next few versions > to handle multi-column stats, something to replace correlation that's > more useful, custom stats functions for more data types, stats > specifically for partitioned tables, etc. I wouldn't want to see any > reason to hold back on these changes. What Peter proposed seems to me pretty reasonable, in the sense that it should be possible to come up with a function that creates some text representation of whatever is in pg_statistic, and another function to load that data into the new catalog(s). There's no need to keep pg_statistic binary-compatible, or even continue to have only pg_statistic (IIRC Zoltan/Hans-Jurgen patch for cross-column stats adds a new catalog, pg_statistic2 or similar). If the upgrade target release has room for more/improved stats, that's fine -- they'll be unused after loading the stats from the dump. And the old stats can be reacommodated, if necessary. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] pg_upgrade and statistics
Peter Eisentraut writes: > On tor, 2012-03-15 at 11:15 -0400, Andrew Dunstan wrote: >> I haven't looked at it, but I'm wondering how hard it is going to be >> in practice? > Take a look at the commit log of pg_statistic.h; it's not a lot. That says nothing as all about the cost of dealing with a change. And as Greg pointed out, there might be a lot more churn in the future than there has been in the past. We're getting to the point where stats are a primary limitation on what we can do, so I wouldn't be surprised if he's right and there's more activity in this area soon. 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] CREATE FOREGIN TABLE LACUNA
On ons, 2012-03-14 at 17:38 -0400, Tom Lane wrote: > Peter Eisentraut writes: > > On ons, 2012-03-14 at 17:16 -0400, Robert Haas wrote: > >> If a constraint is NOT ENFORCED, then the query planner presumably > >> won't rely on it for planning purposes > > > Why do you presume that? > > What does SQL:2011 say exactly about the semantics of NOT ENFORCED? > Is an implementation allowed to fail in undefined ways if a constraint > is marked NOT ENFORCED and is not actually true? It doesn't say anything about that. I might have to dig deeper into the change proposals to see if any rationale came with this change. But in any case, even if we spell it differently, I think there are use cases for a constraint mode that says, assume it's true for optimization purposes, but don't spend any cycles on verifying it. And that constraint mode could apply to regular tables and foreign tables alike. -- 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_upgrade and statistics
On tor, 2012-03-15 at 11:15 -0400, Andrew Dunstan wrote: > I haven't looked at it, but I'm wondering how hard it is going to be > in practice? Take a look at the commit log of pg_statistic.h; it's not a lot. -- 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] EquivalenceClasses and subqueries and PlaceHolderVars, oh my
I wrote: > Yeb Havinga writes: >> I'm having a hard time imagining that add_child_rel_equivalences is not >> just plain wrong. Even though it will only add child equivalence members >> to a parent eq class when certain conditions are met, isn't it the case >> that since a union (all) is addition of tuples and not joining, any kind >> of propagating restrictions on a append rel child member to other areas >> of the plan can cause unwanted results, like the ones currently seen? > None of the known problems are the fault of that, really. The child > entries don't cause merging of ECs, which would be the only way that > they'd affect the semantics of the query at large. So in that sense > they are not really members of the EC but just some auxiliary entries > that ease figuring out whether a child expression matches an EC. After further thought about that, I've concluded that indeed my patch 57664ed25e5dea117158a2e663c29e60b3546e1c was just plain wrong, and Teodor was more nearly on the right track than I was in the original discussion. If child EC members aren't full-fledged members then there's no a-priori reason why they need to be distinct from each other. There are only a few functions that actually match anything to child members (although there are some others that could use Asserts or tests to make it clearer that they aren't paying attention to child members). AFAICT, if we don't try to enforce uniqueness of child members, the only consequences will be: (1) It'll be order-dependent which EquivalenceClass a child index column is thought to match. As I explained earlier, this is not really the fault of this representational detail, but is a basic shortcoming of the whole current concept of ECs. Taking the first match is fine for now. (2) It'll be unclear which of several identical subplan output columns should be sorted by in prepare_sort_from_pathkeys. Now ordinarily that does not particularly matter --- if you have multiple identical nonvolatile expressions, you can take any one (and we already have a hack in there for the volatile case). I think it *only* matters for MergeAppend, where we need to be sure that the sort column locations match across all the children. However, we can fix that in some localized way instead of screwing up ECs generally. The idea I have in mind at the moment, since create_merge_append_plan already starts by determining the sort column locations for the MergeAppend itself, is to pass down that info to the calls for the child plans and insist that we match to the same column locations we found for the parent MergeAppend. So I now propose reverting the earlier two patches (but not their regression test cases of course) and instead hacking MergeAppend plan building as per (2). 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] Storage Manager crash at mdwrite()
I'm implementing an extention to mdwrite() at backend/storage/smgr/md.c When a block is written to the local storage using mdwrite(), I'm sending this block to an HDFS storage. So far I don't need to read back the values I'm writing to HDFS. This approach is working fine in the initDB phase. However, when I'm running postgres (bin/pg_ctl start), the first few write operations run successfully, and then suddenly (after writing exactly 3 files to HDFS), I get a 130 exit code with the following message showing the JVM thread dump of HDFS: LOG: background writer process (PID 29347) exited with exit code 130 LOG: terminating any other active server processes WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. HINT: In a moment you should be able to reconnect to the database and repeat your command. 2012-03-15 13:27:52 Full thread dump OpenJDK Server VM (16.0-b13 mixed mode): "IPC Client (47) connection to localhost/127.0.0.1:8020 from taljab1" daemon prio=10 tid=0x8994d400 nid=0x72e4 in Object.wait() [0x898ad000] java.lang.Thread.State: TIMED_WAITING (on object monitor) at java.lang.Object.wait(Native Method) - waiting on <0x8a3c3050> (a org.apache.hadoop.ipc.Client$Connection) at org.apache.hadoop.ipc.Client$Connection.waitForWork(Client.java:403) - locked <0x8a3c3050> (a org.apache.hadoop.ipc.Client$Connection) at org.apache.hadoop.ipc.Client$Connection.run(Client.java:445) "IPC Client (47) connection to localhost/127.0.0.1:8020 from taljab1" daemon prio=10 tid=0x89b87c00 nid=0x72e3 in Object.wait() [0x898fe000] java.lang.Thread.State: TIMED_WAITING (on object monitor) at java.lang.Object.wait(Native Method) - waiting on <0x8a2ff268> (a org.apache.hadoop.ipc.Client$Connection) at org.apache.hadoop.ipc.Client$Connection.waitForWork(Client.java:403) - locked <0x8a2ff268> (a org.apache.hadoop.ipc.Client$Connection) at org.apache.hadoop.ipc.Client$Connection.run(Client.java:445) "Low Memory Detector" daemon prio=10 tid=0x09daa400 nid=0x72cd runnable [0x] java.lang.Thread.State: RUNNABLE "CompilerThread1" daemon prio=10 tid=0x09da8400 nid=0x72cc waiting on condition [0x] java.lang.Thread.State: RUNNABLE "CompilerThread0" daemon prio=10 tid=0x09da6000 nid=0x72cb waiting on condition [0x] java.lang.Thread.State: RUNNABLE "Signal Dispatcher" daemon prio=10 tid=0x09da4800 nid=0x72ca waiting on condition [0x] java.lang.Thread.State: RUNNABLE "Finalizer" daemon prio=10 tid=0x09d94800 nid=0x72c9 in Object.wait() [0x89db4000] java.lang.Thread.State: WAITING (on object monitor) at java.lang.Object.wait(Native Method) - waiting on <0x8a7202b0> (a java.lang.ref.ReferenceQueue$Lock) at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:133) - locked <0x8a7202b0> (a java.lang.ref.ReferenceQueue$Lock) at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:149) at java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:177) This shows the JVM thread dump of HDFS. This shows the JVM thread dump of HDFS. "Reference Handler" daemon prio=10 tid=0x09d8fc00 nid=0x72c8 in Object.wait() [0x89e05000] java.lang.Thread.State: WAITING (on object monitor) at java.lang.Object.wait(Native Method) - waiting on <0x8a720338> (a java.lang.ref.Reference$Lock) at java.lang.Object.wait(Object.java:502) at java.lang.ref.Reference$ReferenceHandler.run(Reference.java:133) - locked <0x8a720338> (a java.lang.ref.Reference$Lock) "main" prio=10 tid=0x09d3bc00 nid=0x72c6 runnable [0x] java.lang.Thread.State: RUNNABLE "VM Thread" prio=10 tid=0x09d8d000 nid=0x72c7 runnable "VM Periodic Task Thread" prio=10 tid=0x09dac800 nid=0x72ce waiting on condition JNI global references: 763 Heap def new generation total 4800K, used 1844K [0x8a27, 0x8a7a, 0x9487) eden space 4288K, 34% used [0x8a27, 0x8a3e0418, 0x8a6a) from space 512K, 72% used [0x8a72, 0x8a77cdd8, 0x8a7a) to space 512K, 0% used [0x8a6a, 0x8a6a, 0x8a72) tenured generation total 10624K, used 0K [0x9487, 0x952d, 0xa947) the space 10624K, 0% used [0x9487, 0x9487, 0x94870200, 0x952d) compacting perm gen total 16384K, used 5765K [0xa947, 0xaa47, 0xb147) the space 16384K, 35% used [0xa947, 0xa9a11480, 0xa9a11600, 0xaa47) No shared spaces configured. This seems like an HDFS issue, but the same code worked properly in initDB(). I replaced this HDFS write code with a code that writes always the same block (empty one) to HDFS regardless from the value received by mdwrite().. Kept getting the same issue after writing 3 files. I copied this exact code to a separate C application and ran it there and it executed without any problems (I wrote/deleted 100 files). That's why I'm doubting that it'
Re: [HACKERS] pg_upgrade and statistics
On Thu, Mar 15, 2012 at 3:15 PM, Andrew Dunstan wrote: > > You're not the only person who could do that. I don't think this is all down > to you. It should just be understood that if the stats format is changed, > adjusting pg_upgrade needs to be part of the change. When we modified how > enums worked, we adjusted pg_upgrade at the same time. That sort of thing > seems totally reasonable to me. > > I haven't looked at it, but I'm wondering how hard it is going to be in > practice? Historically pg_statistic has been pretty static. But that seems like a negative, not a positive -- a big part of my fear here is that it really really needs a lot of work to improve the statistics. I *hope* they get knocked around dramatically in each of the next few versions to handle multi-column stats, something to replace correlation that's more useful, custom stats functions for more data types, stats specifically for partitioned tables, etc. I wouldn't want to see any reason to hold back on these changes. -- greg -- 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] CREATE FOREGIN TABLE LACUNA
On Thu, Mar 15, 2012 at 10:23 AM, Alvaro Herrera wrote: > Looks much better to me, but the use of strcmp() doesn't look good. > ISTM that stmtType is mostly used for error messages. I think you > should add some kind of identifier (such as the original parser Node) > into the CreateStmtContext so that you can do a IsA() test instead -- a > bit more invasive as a patch, but much cleaner. +1. Or maybe add a relkind to CreateStmt, if it isn't there already, and test that. > Also the error messages need more work. +1. I suggest something like "ERROR: foreign tables do not support LIKE INCLUDING INDEXES". -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] CREATE FOREGIN TABLE LACUNA
On Thu, Mar 15, 2012 at 11:23:43AM -0300, Alvaro Herrera wrote: > Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012: > > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote: > > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: > > > >> I think that instead of inventing new grammar productions and > > > >> a new node type for this, you should just reuse the existing > > > >> productions for LIKE clauses and then reject invalid options > > > >> during parse analysis. > > > > > > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE > > > > TABLE and submit that as a separate patch? > > > > > > I don't see any reason to do that. I merely meant that you > > > could reuse TableLikeClause or maybe even TableElement in the > > > grammer for CreateForeignTableStmt. > > > > Next WIP patch attached implementing this via reusing > > TableLikeClause and refactoring transformTableLikeClause(). > > > > What say? > > Looks much better to me, but the use of strcmp() doesn't look good. > ISTM that stmtType is mostly used for error messages. I think you > should add some kind of identifier (such as the original parser > Node) into the CreateStmtContext so that you can do a IsA() test > instead -- a bit more invasive as a patch, but much cleaner. OK > Also the error messages need more work. What sort? The more I look at this, the more I think that CREATE TABLE and CREATE FOREIGN TABLE should be merged, but that's the subject of a later patch. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: 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] pg_upgrade and statistics
Andrew Dunstan writes: > On 03/15/2012 11:03 AM, Bruce Momjian wrote: >> On Thu, Mar 15, 2012 at 08:22:24AM +0200, Peter Eisentraut wrote: >>> I think this could be budgeted under keeping pg_dump backward >>> compatible. You have to do that anyway for each catalog change, and so >>> doing something extra for a pg_statistic change should be too shocking. >> Well, the big question is whether the community wants to buy into that >> workload. It isn't going to be possible for me to adjust the statistics >> dump/restore code based on the changes someone makes unless I can fully >> understand the changes by looking at the patch. > You're not the only person who could do that. I don't think this is all > down to you. It should just be understood that if the stats format is > changed, adjusting pg_upgrade needs to be part of the change. When we > modified how enums worked, we adjusted pg_upgrade at the same time. That > sort of thing seems totally reasonable to me. Considering that no pg_dump infrastructure for this exists, much less has ever been modified to accommodate a cross-version change, it seems a bit presumptuous to just say "yes we're all buying into that". If someone were to create that infrastructure, complete with the ability to support the already-committed 9.1 to 9.2 changes, then we would have a basis for discussing such a requirement. But until then it's moot. 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] pg_upgrade and statistics
On Thu, Mar 15, 2012 at 10:20:02AM -0500, Kevin Grittner wrote: > Bruce Momjian wrote: > > > I think we have two choices --- either migrate the statistics, or > > adopt my approach to generating incremental statistics quickly. > > Does anyone see any other options? > > Would it make any sense to modify the incremental approach to do a > first pass of any tables with target overrides, using the default > GUC setting, and then proceed through the passes you describe for > all tables *except* those? I'm thinking that any overrides were > probably set because the columns are particularly important in terms > of accurate statistics, and that running with different GUC settings > will just be a waste of time for those tables -- if they have a high > setting for any column, they will sample more blocks for every run, > right? I just added text telling users they might want to remove and re-add those per-table statistics. I could try coding up something to store and reset those values, but it is going to be complex, partly because they are in different databases. I would need to create a pg_upgrade schema in every database, store and reset the targets, and restore them once complete. I don't think it makes sense to do the custom targets first because it would likely delay all tables from getting any statistics. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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_upgrade and statistics
On Thu, Mar 15, 2012 at 11:15:42AM -0400, Andrew Dunstan wrote: > You're not the only person who could do that. I don't think this is > all down to you. It should just be understood that if the stats > format is changed, adjusting pg_upgrade needs to be part of the > change. When we modified how enums worked, we adjusted pg_upgrade at > the same time. That sort of thing seems totally reasonable to me. > > I haven't looked at it, but I'm wondering how hard it is going to be > in practice? Well, the reason I am not recommending migration is because the work will _not_ fall on me, but rather on the larger community of developers, who might or might not care about pg_upgrade. (I am concerned about pg_upgrade retarding development progress.) We are already telling developers not to change the binary storage format without considering pg_upgrade --- do we want to do the same for optimizer statistics? Does the optimizer statistics format change more frequently than the storage format? I think the answer is yes. Does it change too frequently to require migration work? I don't know the answer to that, partly because I would not be the one doing the work. Also, considering we are on the last 9.2 commit-fest, I doubt we can get something working for statistics migration for 9.2, I think the incremental statistics build approach is our only way to improve this for 9.2, and frankly, 9.1 users can also use the script once I post it. FYI, I have not received a huge number of complaints about the old analyze recommendation --- a few, but not a flood. The incremental build approach might be good enough. My wild guess is that even if we migrated all statistics, the migrated statistics will still not have any improvements we have made in statistics gathering, meaning there will still be some kind of analyze process necessary, hopefully just on the affected tables --- that would be shorter, but perhaps not shorter enough to warrant the work in migrating the statistics. I am attaching the updated script and script output. Please, don't think I am ungrateful for the offers of help in migrating statistics. I just remember how complex the discussion was when we modified the enum improvements to allow pg_upgrade, and how complex the checksum discussion was related to pg_upgrade. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + analyze_new_cluster.sh Description: Bourne shell script This script will generate minimal optimizer statistics rapidly so your system is usable, and then gather statistics twice more with increasing accuracy. When it is done, your system will have the default level of optimizer statistics. If you have used ALTER TABLE to modify the statistics target for any tables, you might want to remove them and restore them after running this script because they will delay fast statistics generation. If you would like default statistics as quickly as possible, cancel this script and run: vacuumdb --all --analyze-only Generating minimal optimizer statistics (1 target) -- vacuumdb: vacuuming database "postgres" vacuumdb: vacuuming database "template1" vacuumdb: vacuuming database "test" The server is now available with minimal optimizer statistics. Query performance will be optimal once this script completes. Generating medium optimizer statistics (10 targets) --- vacuumdb: vacuuming database "postgres" vacuumdb: vacuuming database "template1" vacuumdb: vacuuming database "test" Generating default (full) optimizer statistics (100 targets?) - vacuumdb: vacuuming database "postgres" vacuumdb: vacuuming database "template1" vacuumdb: vacuuming database "test" Done -- 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_upgrade and statistics
Bruce Momjian wrote: > I think we have two choices --- either migrate the statistics, or > adopt my approach to generating incremental statistics quickly. > Does anyone see any other options? Would it make any sense to modify the incremental approach to do a first pass of any tables with target overrides, using the default GUC setting, and then proceed through the passes you describe for all tables *except* those? I'm thinking that any overrides were probably set because the columns are particularly important in terms of accurate statistics, and that running with different GUC settings will just be a waste of time for those tables -- if they have a high setting for any column, they will sample more blocks for every run, right? -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] pg_upgrade and statistics
On 03/15/2012 11:03 AM, Bruce Momjian wrote: On Thu, Mar 15, 2012 at 08:22:24AM +0200, Peter Eisentraut wrote: On ons, 2012-03-14 at 17:36 -0400, Bruce Momjian wrote: Well, I have not had to make major adjustments to pg_upgrade since 9.0, meaning the code is almost complete unchanged and does not require additional testing for each major release. If we go down the road of dumping stats, we will need to adjust for stats changes and test this to make sure we have made the proper adjustment for every major release. I think this could be budgeted under keeping pg_dump backward compatible. You have to do that anyway for each catalog change, and so doing something extra for a pg_statistic change should be too shocking. Well, the big question is whether the community wants to buy into that workload. It isn't going to be possible for me to adjust the statistics dump/restore code based on the changes someone makes unless I can fully understand the changes by looking at the patch. You're not the only person who could do that. I don't think this is all down to you. It should just be understood that if the stats format is changed, adjusting pg_upgrade needs to be part of the change. When we modified how enums worked, we adjusted pg_upgrade at the same time. That sort of thing seems totally reasonable to me. I haven't looked at it, but I'm wondering how hard it is going to be in practice? 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] pg_upgrade and statistics
On Thu, Mar 15, 2012 at 08:22:24AM +0200, Peter Eisentraut wrote: > On ons, 2012-03-14 at 17:36 -0400, Bruce Momjian wrote: > > Well, I have not had to make major adjustments to pg_upgrade since 9.0, > > meaning the code is almost complete unchanged and does not require > > additional testing for each major release. If we go down the road of > > dumping stats, we will need to adjust for stats changes and test this to > > make sure we have made the proper adjustment for every major release. > > I think this could be budgeted under keeping pg_dump backward > compatible. You have to do that anyway for each catalog change, and so > doing something extra for a pg_statistic change should be too shocking. Well, the big question is whether the community wants to buy into that workload. It isn't going to be possible for me to adjust the statistics dump/restore code based on the changes someone makes unless I can fully understand the changes by looking at the patch. I think we have two choices --- either migrate the statistics, or adopt my approach to generating incremental statistics quickly. Does anyone see any other options? In an ideal world, analyze would generate minimal statistics on all tables/databases, then keep improving them until they are the default, but that is unlikely to happen because of the code complexity involved. My powers-of-10 approach is probably the best we are going to do in the short term. My current idea is to apply the incremental analyze script patch to 9.2, and blog about the patch and supply downloadable versions of the script people can use on 9.1 and get feedback on improvements. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Another review of URI for libpq, v7 submission
Daniel Farina writes: > I reviewed this and so far have not found any serious problems, > although as is par for the course it contains some of the fiddly bits > involved in any string manipulations in C. I made a few edits -- none > strictly necessary for correctness -- that the original author is free > audit and/or include[0]. Thank you for the review, Daniel! Apparently, I was on drugs when I've submitted v7, as it still contains the bug for which to fix I was forced to move parts of the code back into the main parser routine... > I did put in some defensive programming choices (instead of if/else > if/elseif/else raise an error, even if the latter is allegedly > impossible) that I think are a good idea. Yes, this is a good idea, I'll incorporate them in the patch. However, this one doesn't work: https://github.com/fdr/postgres/commit/4fad90fb243d9266b1003cfbcf8397f67269fad3 Neither '@' or '/' are mandatory in the URI anywhere after "scheme://", so we can't just say it "should never happen." Running the regression test with this commit applied highlights the problem. > One thing I found puzzling was that in the latest revision the tests > appeared to be broken for me: all "@" signs were translated to "(at)". > Is that mangling applied by the archives, or something? This is definitely mangling by the lists. I can see this has happened to people before, e.g.: http://archives.postgresql.org/pgsql-hackers/2010-01/msg00938.php But that discussion didn't seem to lead to a solution for the problem. I wonder if there's any evidence as to that mangling the email addresses helps to reduce spam at all? I mean replacing "(at)" back to "@" and "(dot)" to "." is piece of cake for a spam crawler. What I see though, is that most of the message-id URLs are broken by the mangling. Also I don't think everyone should just tolerate that it messes with the attachments. Should we all suddenly use application/octet-stream or application/gzip instead of text/plain? > The test suite neatly tries to copy pg_regress's general "make > installcheck" style, but it likes to use my username as the database > rather than the standard "regression" as seen by pg_regress. It is > nice that a place to test connection strings and such is there, where > there was none before. Oh, I don't see why we can't use "regression" too. While testing this I've also noticed that there was some funny behavior when you left out the final slash after hostname, e.g.: "postgres://localhost/" vs. "postgres://localhost". It turned out in the former case we were setting dbname conninfo parameter to an empty string and in the latter one we didn't set it at all. The difference is that when we do set it, the default dbname is derived from username, but when we don't--$PGDATABASE is used. I've fixed this, so now both URIs work the same way (both do not set dbname.) It also appeared to me that with the current code, a wide range of funny-looking URIs are considered valid, e.g.: postgres://user@ postgres://@host postgres://host:/ and, taking approach to the extreme: postgres://:@: This specifies empty user, password, host and port, and looks really funny. I've added (actually revived) some checks to forbid setting empty URI parts where it doesn't make much sense. Finally, attached is v8. Hopefully I didn't mess things up too much. -- Regards, Alex binaKreQKSDSW.bin Description: libpq-uri-v8.patch -- 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] EquivalenceClasses and subqueries and PlaceHolderVars, oh my
Yeb Havinga writes: > On 2012-03-15 02:29, Tom Lane wrote: >>> There is an EquivalenceClass for each of "t1" and "t2", and if we don't >>> do something like wrapping the constants with distinct PHVs, then >>> add_child_rel_equivalences will end up pushing identical constants into >>> both ECs, thus totally bollixing the fundamental rule that any expression >>> should match at most one EC. > I'm having a hard time imagining that add_child_rel_equivalences is not > just plain wrong. Even though it will only add child equivalence members > to a parent eq class when certain conditions are met, isn't it the case > that since a union (all) is addition of tuples and not joining, any kind > of propagating restrictions on a append rel child member to other areas > of the plan can cause unwanted results, like the ones currently seen? None of the known problems are the fault of that, really. The child entries don't cause merging of ECs, which would be the only way that they'd affect the semantics of the query at large. So in that sense they are not really members of the EC but just some auxiliary entries that ease figuring out whether a child expression matches an EC. Having said that, I have been wondering whether there's a better way to do that task. Right now prepare_sort_from_pathkeys has to do a lot of fairly ugly, heuristic stuff to do that matching at all. The other area of the code that has to match child expressions to ECs is index path generation, and we already know that code isn't terribly happy with the PHV-based solution either ... 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] Another review of URI for libpq, v7 submission
Excerpts from Daniel Farina's message of jue mar 15 05:49:50 -0300 2012: > One thing I found puzzling was that in the latest revision the tests > appeared to be broken for me: all "@" signs were translated to "(at)". > Is that mangling applied by the archives, or something? Ugh, ouch. Yeah, that was done by the archives. It seems that when attachments are text/plain Mhonarc applies anti-spamming to them :-( The original message doesn't have that problem. Sorry about that. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] CREATE FOREGIN TABLE LACUNA
Excerpts from David Fetter's message of jue mar 15 02:28:28 -0300 2012: > On Wed, Mar 14, 2012 at 12:06:20PM -0400, Robert Haas wrote: > > On Wed, Mar 14, 2012 at 10:22 AM, David Fetter wrote: > > >> I think that instead of inventing new grammar productions and a new > > >> node type for this, you should just reuse the existing productions for > > >> LIKE clauses and then reject invalid options during parse analysis. > > > > > > OK. Should I first merge CREATE FOREIGN TABLE with CREATE TABLE and > > > submit that as a separate patch? > > > > I don't see any reason to do that. I merely meant that you could > > reuse TableLikeClause or maybe even TableElement in the grammer for > > CreateForeignTableStmt. > > Next WIP patch attached implementing this via reusing TableLikeClause > and refactoring transformTableLikeClause(). > > What say? Looks much better to me, but the use of strcmp() doesn't look good. ISTM that stmtType is mostly used for error messages. I think you should add some kind of identifier (such as the original parser Node) into the CreateStmtContext so that you can do a IsA() test instead -- a bit more invasive as a patch, but much cleaner. Also the error messages need more work. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] libpq should have functions for escaping data for use in COPY FROM
Robert Haas writes: > Considering all the above, this seems like it might be a solution in > search of a problem. It's not actually that hard to write code to do > proper escaping for a *given* encoding and a *given* set of COPY > options, but trying to write something general sounds like a job and a > half. Yeah, it wouldn't be easy. The other point to keep in mind here is that one of the key requirements for anything dealing with COPY is that it be fast. It seems likely to me that bespoke code for a particular encoding and set of copy options would outrun something that's general-purpose. Now maybe people would be happy to use something slower if it meant they didn't have to write it themselves, but ... 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] Command Triggers, patch v11
On 14 March 2012 21:33, Dimitri Fontaine wrote: > Ok, I've implemented that. No patch attached because I need to merge > with master again and I'm out to sleep now, it sometimes ring when being > on-call… > > Curious people might have a look at my github repository where the > command_triggers branch is updated: Will you also be committing the trigger function variable changes shortly? I don't wish to test anything prior to this as that will involve a complete re-test from my side anyway. -- Thom -- 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] CREATE FOREGIN TABLE LACUNA
(2012/03/15 0:29), Tom Lane wrote: > The posted patch for file_fdw takes the > approach of silently filtering out rows for which they're not true, > which is not obviously the right thing either --- quite aside from > whether that's a sane semantics, it's not going to scale to foreign key > constraints, and even for simple NOT NULL and CHECK constraints it > results in a runtime penalty on selects, which is not what people would > expect from a constraint. I investigated DB2 a little bit. In DB2, the user can specify the VALIDATE_DATA_FILE option as a generic option for an external table attached to a data file, which specifies if the wrapper verifies that the data file is sorted. How about introducing this kind of option to file_fdw? It might be better that the default value for the option is 'false', and if the value is set to 'true', then file_fdw verifies NOT NULL, CHECK, and foreign key constraints. Best regards, Etsuro Fujita -- 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] EquivalenceClasses and subqueries and PlaceHolderVars, oh my
On 2012-03-15 02:29, Tom Lane wrote: > > explain select * from > (select thousand as t1, tenthous as t2 from tenk1 a > union all > select 42 as t1, 42 as t2 from tenk1 b) c > order by t1, t2; > > There is an EquivalenceClass for each of "t1" and "t2", and if we don't > do something like wrapping the constants with distinct PHVs, then > add_child_rel_equivalences will end up pushing identical constants into > both ECs, thus totally bollixing the fundamental rule that any expression > should match at most one EC. I'm having a hard time imagining that add_child_rel_equivalences is not just plain wrong. Even though it will only add child equivalence members to a parent eq class when certain conditions are met, isn't it the case that since a union (all) is addition of tuples and not joining, any kind of propagating restrictions on a append rel child member to other areas of the plan can cause unwanted results, like the ones currently seen? regards, Yeb Havinga -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Another review of URI for libpq, v7 submission
I reviewed this and so far have not found any serious problems, although as is par for the course it contains some of the fiddly bits involved in any string manipulations in C. I made a few edits -- none strictly necessary for correctness -- that the original author is free audit and/or include[0]. I did put in some defensive programming choices (instead of if/else if/elseif/else raise an error, even if the latter is allegedly impossible) that I think are a good idea. Loops around pointer increments are very fastidiously checked for NUL-byteness, and those that aren't are carefully guarded by invariants that seem like they should prevent an overrun. The nature of the beast, I suppose, short of giving libpq a "StringData" like struct and a small lexer to make it more clear that a subtle overrun is not creeping in. One thing I found puzzling was that in the latest revision the tests appeared to be broken for me: all "@" signs were translated to "(at)". Is that mangling applied by the archives, or something? The test suite neatly tries to copy pg_regress's general "make installcheck" style, but it likes to use my username as the database rather than the standard "regression" as seen by pg_regress. It is nice that a place to test connection strings and such is there, where there was none before. I am happy with the range and style of accepted URIs, and I think this can stem the bleeding of the fragmentation already taking place at large. [0]: https://github.com/fdr/postgres/tree/uri, commit e50ef375b7a731ca79bf5d3ca8b0bd69c97a9e71, aka the 'uri' branch -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers