Re: [HACKERS] ALTER TABLE SET STATISTICS requires AccessExclusiveLock
On Sun, 2010-07-18 at 22:47 -0400, Robert Haas wrote: But it seems that it's far from clear what to do about it, and it's not the job of this patch to fix it anyway. Agreed. Regarding the actual patch, it looks mostly good. Questions: 1. Why in rewriteSupport.c are we adding a call to heap_inplace_update() in some situations? Doesn't seem like this is something we should need or want to be monkeying with. Hmm, yes, that looks like a hangover. Will change. No others similar. 2. Instead of AlterTableGreatestLockLevel(), how about AlterTableGetLockLevel()? Yeah, it's going to be the highest lock level required by any subcommand, but it seems mildly overspecified. I don't feel strongly about this one, though, if someone has a strong contrary opinion... I felt it indicated the process it's using. Happy to change. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and 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] Hostnames in pg_hba.conf
On tor, 2010-02-11 at 14:13 +0100, Bart Samwel wrote: I've been working on a patch to add hostname support to pg_hba.conf. It's not ready for public display yet, but I would just like to run a couple of issues / discussion points past everybody. What's the status of this? -- 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] SHOW TABLES
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 1. \d isn't exactly the most intuitive thing ever Seems fairly mnemomic to me (d=describe) and it packs a *lot* of information into a single letter (see below). Things that are done often should have short keystrokes, and not require learning Yet Another Meta-Language. And it's pretty clear that we have been heading into some increasingly cryptic bits of fruit salad of \dfzb+-meta-bucky-alt-foo No arguments there, but that's the nature of the beast. I don't think it's as bad as is made out, however, as \d covers 99% of everyday usage and certainly the show tables that started this thread. Having SHOW THIS and SHOW THAT which are a bit more readily guessed would be somewhat nice. I'm not sure why easily guessed is thrown out in this thread as such a great thing. To achieve that goal, we simply need the help system that has been proposed many times: entering in SHOW anything gives you a quick rundown of the backslash system. As far as SHOW THIS, there is a big difference from a plain \dt and \d tablename. The former could be emulated quite easily with a SHOW command (although even our \dt prints out more information than mysql's SHOW TABLES), but the latter includes a crazy amount of information that would lead to quite a large SHOW... statement. Also, if it were made a server-side thing, how would you return things like indexes on a table in a SRF? Have a meta-column describing what the other columns represent? Ugly. information_schema doesn't have some useful things that we'd like ait to have ... Alas, I don't see a good way to improve on this :-( newsysviews seems the way out of that particular mess. I'm also not particularly opposed to adding new views or columns to information_schema. We would still support the standard by having all the required views and columns. The \? commands are *solely* for psql, and it would be nice to have the Improvement work on server side so it's not only usable with the one client. Agreed, but is there some other command-line client? If it's not command-line, free-form SQL typing, it inevitably already has support for querying the catalogs built in. At least, every GUI, app, and driver I can think of does. I've seen too many QA scripts that do awk parsing of output of psql \d commands that are vulnerable to all kinds of awfulness. They should be querying information_schema. I'd sure like to be able to write queries that *don't* involve array smashing or using grep on \z output to analyze object permissions. Yeah, that would be a better information_schema. :) - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201007191011 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkxEXS0ACgkQvJuQZxSWSshLKwCffkfe0T3tELInxRqG7yCDS5Vr Ku8AoLUtOu7tTplGZZLPOEuDfKHt+EEm =Oubu -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SHOW TABLES
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Robert Haas (robertmh...@gmail.com) wrote: I think LIST COMMENTS ON SYSTEM AGGREGATES would be an epic step forward in usability. Perhaps. But it would behoove you to come up with a less er... arcane example. I've been using Postgres a long time, and I can count the number of times I've needed to see comments on system aggregates on my hand. With at least four fingers left over. ... in the alphabet soup paragraph above. I don't think there's anything WRONG with letting \dFp show text search dictionaries and \dfwS+ list system window functions with additional detail - but I'd like an alternative that emphasizes ease of remembering over brevity, works in every client, and can be extended in whatever reasonable ways the community decides are worth having. ... I don't know that I'd necessarily remember all those any better, and would certainly not enjoy typing out: LIST TEST SEARCH DICTIONARIES I don't have to remember \dFp - all I have to remember is \?. For the more common ones that I use day to day and don't have to look up (\d \dt \df \l etc.) the advantage of a two or three character string is strong. (There is some devil's advocate in there - a standard cross client (and dare I say it, cross RDBMS?) way would be nice) ... being powerful rings totally hollow for me. For ordinary, day to day tasks like listing all my tables, or looking at the details of a particular table, they're great. I use them all the time and would still use them even if some other syntax were available. But there is no reasonable way to pass options to them, and that to me is a pretty major drawback. Well, there's the rub. You're arguing this from a hacker's persepective, while the SHOW syntax seems to be overwhelmingly agreed upon to be either helpful for clueless noobs, or some nice syntactic sugar for average users. I'm not sure where to draw the line but implementing a proper shortcut interface for cammands is something taht should be done on the client side because not every client is the same and the needs of psql might be radically different from any other client (like pgadmin or a fancy Web 2.0 AJAX thingy - those will likely always use custom catalog queries). Maybe a differnet way to look at the whole thing is to reconsider our own catalogs (anyone remember newsysview?) and add a bunch of views to abstract away most of the current complexity for these usecases? Yep, agreed. Now, if we can just agree to put information_schema in the default search_path, because nobody enjoys having to type out information_schema... - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201007191021 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkxEYDgACgkQvJuQZxSWSsikFwCdGo88Ehdcm8OHi2+VxISTG60Y b9sAoLsetxcpdMSconsCwj+3Xa1fCCzo =3aM1 -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SHOW TABLES
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Kevin Grittner wrote: Any solution which only works within psql isn't a solution for a large part of the problem space people are trying to address. One important goal is that if someone spends a day to whip up a GUI query tool (as I did when I first started working in Java), it's easy to get displays like we get from the psql backslash commands (as it was in Sybase, which is what we were using at the time, through sp_help and related stored procedures). I don't agree that this is an important goal. Certainly someone writing a GUI (or a new driver) should be expected to be familiar with the system catalogs. Moreover, a GUI relies on an underlying driver, and every driver should already be providing things like a list of tables natively. - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201007191030 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkxEYZ0ACgkQvJuQZxSWSsiIlQCfdXDgTqletVez/r+pKHY4EcW6 QAsAoPLUmblzN2aNEw5DveHEav3XyB/K =TGq1 -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SHOW TABLES
Stephen Frost sfr...@snowman.net wrote: You think that the users of the libpq() interface (or even the protocol itself) are going to handle getting \dt-type output back somehow..? If you look back in the thread, you'll see that I admitted my ignorance of whether this could be properly implemented in the back end without a protocol change. Ignorance being bliss, I can revel in the dreams of *having* such a feature without being dragged down by the potential pain of its implementation. ;-) I know, though, that the JDBC spec supports such things -- you can keep pulling ResultSet objects off the wire, each with its own distinct set of columns. (That is, each ResultSet has its own ResultSetMetaData which specifies how many columns that particular ResultSet has, what the column names are, what the data type is for each column, etc. Each ResultSet returned from a response stream for a request can be entirely different in all of these characteristics.) As what, a single-column result of type text? No, that would be horrible. That has been mentioned as a possibility, and it makes me shudder. And then they'll use non-fixed-width fonts, undoubtably, which means the results will end up looking rather ugly, even if we put in the effort to format the results. With, for example, Sybase's sp_help, each result set can be listed any way the client chooses -- I've seen it put into character format like the psql \d commands, I've seen each result set put into a table for brower-based query tools, and I've seen each result set put into a JTable for Java Swing applications. If a client gets back a series of result sets, the sky is the limit. -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] SHOW TABLES
* Kevin Grittner (kevin.gritt...@wicourts.gov) wrote: I know, though, that the JDBC spec supports such things -- you can keep pulling ResultSet objects off the wire, each with its own distinct set of columns. (That is, each ResultSet has its own ResultSetMetaData which specifies how many columns that particular ResultSet has, what the column names are, what the data type is for each column, etc. Each ResultSet returned from a response stream for a request can be entirely different in all of these characteristics.) I'm pretty sure we don't have the ability to support that at either the libpq or the protocol level today, but in general I do think it's a good idea and would be good to support. To be honest, I seem to recall someone talking about working on it (or perhaps it's on the TODO?). In any case, long way there from here. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] SHOW TABLES
On Mon, Jul 19, 2010 at 10:25 AM, Greg Sabino Mullane g...@turnstep.com wrote: in the alphabet soup paragraph above. I don't think there's anything WRONG with letting \dFp show text search dictionaries and \dfwS+ list system window functions with additional detail - but I'd like an alternative that emphasizes ease of remembering over brevity, works in every client, and can be extended in whatever reasonable ways the community decides are worth having. ... I don't know that I'd necessarily remember all those any better, and would certainly not enjoy typing out: LIST TEST SEARCH DICTIONARIES I don't have to remember \dFp - all I have to remember is \?. For the more common ones that I use day to day and don't have to look up (\d \dt \df \l etc.) the advantage of a two or three character string is strong. I don't think anyone is proposing getting rid of backslash commands. That would be nuts. What's being proposed is to try to create a better way to list objects, a way that involves some server-side support so that clients don't need to muck about with system catalogs quite so much. I like psql as well as anyone, but saying that it's easy to do this stuff because you can use \? to get the appropriate backslash command seems to me to be missing the point. Suppose you regularly use PGadmin to access the database, or some other graphical client, and you want to find a query to list the comments on every item in the database. Good luck! You'll need to figure out how to use psql, discover that it has a -E switch (of which I was ignorant for my first 10 years of using PostgreSQL), and get the query out of there. Then you'll find that the query psql uses is more than 50 lines long and also wrong: it omits half the object types. Woohoo! And even supposing that you fix the query, there's no guarantee that it won't be wrong again when PG version X+1 comes out. Take a look at describe.c. It's riddled with special cases for particular PG versions, special cases that must be replicated in every other client that wants to work with multiple PG versions. So, basically, our advice to anyone who wants a simple, portable way to list objects of particular types in a cross-PG version compatible way is - copy the logic in describe.c, adapt it to your application, and update it every time a new major release comes out. Is that really the best we can do, and do we really think that's adequate? being powerful rings totally hollow for me. For ordinary, day to day tasks like listing all my tables, or looking at the details of a particular table, they're great. I use them all the time and would still use them even if some other syntax were available. But there is no reasonable way to pass options to them, and that to me is a pretty major drawback. Well, there's the rub. You're arguing this from a hacker's persepective, while the SHOW syntax seems to be overwhelmingly agreed upon to be either helpful for clueless noobs, or some nice syntactic sugar for average users. I use the darn database, too. The machinations I've gone through to get some of the information are ridiculously complex. As Larry Wall one said, a good programming language should make simple things simple and complicated things possible; so, I don't believe that having a simple interface and a powerful interface are mutually exclusive goals. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] ALTER TABLE SET STATISTICS requires AccessExclusiveLock
On Mon, Jul 19, 2010 at 2:46 AM, Simon Riggs si...@2ndquadrant.com wrote: On Sun, 2010-07-18 at 22:47 -0400, Robert Haas wrote: But it seems that it's far from clear what to do about it, and it's not the job of this patch to fix it anyway. Agreed. Regarding the actual patch, it looks mostly good. Questions: 1. Why in rewriteSupport.c are we adding a call to heap_inplace_update() in some situations? Doesn't seem like this is something we should need or want to be monkeying with. Hmm, yes, that looks like a hangover. Will change. No others similar. 2. Instead of AlterTableGreatestLockLevel(), how about AlterTableGetLockLevel()? Yeah, it's going to be the highest lock level required by any subcommand, but it seems mildly overspecified. I don't feel strongly about this one, though, if someone has a strong contrary opinion... I felt it indicated the process it's using. Happy to change. Cool. I think with those two changes it's time to commit this. It's possible there's some case we've overlooked, but I think that we've been over this fairly thoroughly, so hopefully not. Anyway, we're doing this at the beginning of the 9.1 cycle rather than the end, so there's hopefully time for any lingering bugs to be found... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] leaky views, yet again
2010/7/8 KaiGai Kohei kai...@ak.jp.nec.com: (2010/07/08 22:08), Robert Haas wrote: I think we pretty much have conceptual agreement on the shape of the solution to this problem: when a view is created with CREATE SECURITY VIEW, restrict functions and operators that might disclose tuple data from being pushed down into view (unless, perhaps, the user invoking the view has sufficient privileges to execute the underlying query anyhow, e.g. superuser or view owner). What we have not resolved is exactly how we're going to decide which functions and operators might do such a dastardly thing. I think the viable options are as follows: 1. Adopt Heikki's proposal of treating indexable operators as non-leaky. http://archives.postgresql.org/pgsql-hackers/2010-06/msg00291.php Or, perhaps, and even more restrictively, treat only hashable/mergeable operators as non-leaky. 2. Add an explicit flag to pg_proc, which can only be set by superusers (and which is cleared if a non-superuser modifies it in any way), allowing a function to be tagged as non-leaky. I believe that it would be reasonable to set this flag for all of our built-in indexable operators (though I'd read the code before doing it), but it would remove the need for the assumption that third-party operators are equally sane. CREATE OR REPLACE FUNCTION doesnt_leak() RETURNS integer AS $$SELECT 42$$ IMMUTABLE SEAWORTHY; -- doesn't leak This problem is not going away, so we should try to decide on something. I'd like to vote the second option, because this approach will be also applied on another aspect of leaky views. When leaky and non-leaky functions are chained within a WHERE clause, it will be ordered by the cost of functions. So, we have possibility that leaky functions are executed earlier than non-leaky functions. It will not be an easy work to mark built-in functions as either leaky or non-leaky, but it seems to me the most straight forward solution. Does anyone else have an opinion on this? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] leaky views, yet again
On 09/07/10 06:47, KaiGai Kohei wrote: When leaky and non-leaky functions are chained within a WHERE clause, it will be ordered by the cost of functions. So, we have possibility that leaky functions are executed earlier than non-leaky functions. No, that needs to be forbidden as part of the fix. Leaky functions must not be executed before all the quals from the view are evaluated. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] leaky views, yet again
On 19/07/10 20:08, Robert Haas wrote: 2010/7/8 KaiGai Koheikai...@ak.jp.nec.com: (2010/07/08 22:08), Robert Haas wrote: I think we pretty much have conceptual agreement on the shape of the solution to this problem: when a view is created with CREATE SECURITY VIEW, restrict functions and operators that might disclose tuple data from being pushed down into view (unless, perhaps, the user invoking the view has sufficient privileges to execute the underlying query anyhow, e.g. superuser or view owner). What we have not resolved is exactly how we're going to decide which functions and operators might do such a dastardly thing. I think the viable options are as follows: 1. Adopt Heikki's proposal of treating indexable operators as non-leaky. http://archives.postgresql.org/pgsql-hackers/2010-06/msg00291.php Or, perhaps, and even more restrictively, treat only hashable/mergeable operators as non-leaky. 2. Add an explicit flag to pg_proc, which can only be set by superusers (and which is cleared if a non-superuser modifies it in any way), allowing a function to be tagged as non-leaky. I believe that it would be reasonable to set this flag for all of our built-in indexable operators (though I'd read the code before doing it), but it would remove the need for the assumption that third-party operators are equally sane. CREATE OR REPLACE FUNCTION doesnt_leak() RETURNS integer AS $$SELECT 42$$ IMMUTABLE SEAWORTHY; -- doesn't leak This problem is not going away, so we should try to decide on something. I'd like to vote the second option, because this approach will be also applied on another aspect of leaky views. When leaky and non-leaky functions are chained within a WHERE clause, it will be ordered by the cost of functions. So, we have possibility that leaky functions are executed earlier than non-leaky functions. It will not be an easy work to mark built-in functions as either leaky or non-leaky, but it seems to me the most straight forward solution. Does anyone else have an opinion on this? I have a bad feeling that marking functions explicitly as seaworthy is going to be hard to get right, and every time you get that wrong it's a security issue. On the other hand, if it's enough from a performance point of view to review and mark only a few built-in functions like index operators, maybe it's ok. It would be easier to assess this if we had a patch to play with that contained all the planner changes to keep track of the seaworthiness of functions and apply the quals in right order. You could then try out different scenarios to see what the performance is like. I guess what I'm saying is write a patch, and I'll shoot it down when you're done :-/. But hopefully the planner changes don't depend much on how we deduce which quals are leaky. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SHOW TABLES
On Mon, Jul 19, 2010 at 02:12:19PM -, Greg Sabino Mullane wrote: -BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 1. \d isn't exactly the most intuitive thing ever Seems fairly mnemomic to me (d=describe) and it packs a *lot* of information into a single letter (see below). Things that are done often should have short keystrokes, and not require learning Yet Another Meta-Language. And it's pretty clear that we have been heading into some increasingly cryptic bits of fruit salad of \dfzb+-meta-bucky-alt-foo No arguments there, but that's the nature of the beast. I don't think it's as bad as is made out, however, as \d covers 99% of everyday usage and certainly the show tables that started this thread. It covers 0% of cases where people are not using psql. Cheers, David. -- David Fetter da...@fetter.org 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] SHOW TABLES
On Mon, 2010-07-19 at 10:23 -0700, David Fetter wrote: On Mon, Jul 19, 2010 at 02:12:19PM -, Greg Sabino Mullane wrote: -BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 1. \d isn't exactly the most intuitive thing ever Seems fairly mnemomic to me (d=describe) and it packs a *lot* of information into a single letter (see below). Things that are done often should have short keystrokes, and not require learning Yet Another Meta-Language. And it's pretty clear that we have been heading into some increasingly cryptic bits of fruit salad of \dfzb+-meta-bucky-alt-foo No arguments there, but that's the nature of the beast. I don't think it's as bad as is made out, however, as \d covers 99% of everyday usage and certainly the show tables that started this thread. It covers 0% of cases where people are not using psql. Which is probably 85% of our users. (No I have no actual metric) Every single corp I have walked into is using at least PgAdmin as well. Until this project as a whole recognizes that for the majority to the populace the command line is dead; we will continue to have these arguments for no apparent reason but to make sure spam filters are still reading our emails. JD -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering -- 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] leaky views, yet again
On Mon, Jul 19, 2010 at 1:19 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I guess what I'm saying is write a patch, and I'll shoot it down when you're done :-/. But hopefully the planner changes don't depend much on how we deduce which quals are leaky. Oh, great... I love that. /me rolls eyes -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] SHOW TABLES
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 David Fetter wrote: No arguments there, but that's the nature of the beast. I don't think it's as bad as is made out, however, as \d covers 99% of everyday usage and certainly the show tables that started this thread. It covers 0% of cases where people are not using psql. Yes, and everything else already has a show tables. See for example, PPA: http://phppgadmin.sourceforge.net/images/4.png - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201007191342 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkxEj4kACgkQvJuQZxSWSshrwgCg65eIziE2SW8XhdTSHwVMzxnm ynIAoLPOc0yuKyrE2kaaJFq5UiDb45Nd =veva -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SHOW TABLES
On Mon, Jul 19, 2010 at 09:31:06AM -0500, Kevin Grittner wrote: Stephen Frost sfr...@snowman.net wrote: You think that the users of the libpq() interface (or even the protocol itself) are going to handle getting \dt-type output back somehow..? If you look back in the thread, you'll see that I admitted my ignorance of whether this could be properly implemented in the back end without a protocol change. Ignorance being bliss, I can revel in the dreams of *having* such a feature without being dragged down by the potential pain of its implementation. ;-) I know, though, that the JDBC spec supports such things -- you can keep pulling ResultSet objects off the wire, each with its own distinct set of columns. (That is, each ResultSet has its own ResultSetMetaData which specifies how many columns that particular ResultSet has, what the column names are, what the data type is for each column, etc. Each ResultSet returned from a response stream for a request can be entirely different in all of these characteristics.) Would something like this do? Thanks to Andrew Gierth for helping me figure out how to get this working :) CREATE OR REPLACE FUNCTION multi_result() RETURNS SETOF REFCURSOR LANGUAGE plpgsql AS $$ DECLARE r RECORD; ref REFCURSOR; BEGIN FOR r IN SELECT table_name FROM information_schema.tables WHERE table_schema = 'information_schema' LOOP ref := 'multi_result_' || quote_ident(r.table_name); OPEN ref FOR EXECUTE 'SELECT * FROM information_schema.' || quote_ident(r.table_name); /* Not really needed. */ RETURN NEXT ref; ref := NULL; END LOOP; RETURN; END; $$; BEGIN; SELECT * FROM multi_result(); FETCH FORWARD ALL FROM multi_result_views; ROLLBACK; As what, a single-column result of type text? No, that would be horrible. That has been mentioned as a +1 on the shuddering. Add also nausea. :P And then they'll use non-fixed-width fonts, undoubtably, which means the results will end up looking rather ugly, even if we put in the effort to format the results. With, for example, Sybase's sp_help, each result set can be listed any way the client chooses -- I've seen it put into character format like the psql \d commands, I've seen each result set put into a table for brower-based query tools, and I've seen each result set put into a JTable for Java Swing applications. If a client gets back a series of result sets, the sky is the limit. Ad astra per PostgreSQL! Cheers, David. -- David Fetter da...@fetter.org 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] Reworks of DML permission checks
2010/7/12 KaiGai Kohei kai...@ak.jp.nec.com: (2010/07/10 5:53), Robert Haas wrote: 2010/6/14 KaiGai Koheikai...@ak.jp.nec.com: The attached patch tries to rework DML permission checks. It was mainly checked at the ExecCheckRTEPerms(), but same logic was implemented in COPY TO/FROM statement and RI_Initial_Check(). This patch tries to consolidate these permission checks into a common function to make access control decision on DML permissions. It enables to eliminate the code duplication, and improve consistency of access controls. This patch is listed on the CommitFest page, but I'm not sure if it represents the latest work on this topic. At a minimum, it needs to be rebased. I am not excited about moving ExecCheckRT[E]Perms to some other place in the code. It seems to me that will complicate back-patching with no corresponding advantage. I'd suggest we not do that. The COPY and RI code can call ExecCheckRTPerms() where it is. Maybe at some point we will have a grand master plan for how this should all be laid out, but right now I'd prefer localized changes. OK, I rebased and revised the patch not to move ExecCheckRTPerms() from executor/execMain.c. In the attached patch, DoCopy() and RI_Initial_Check() calls that function to consolidate dml access control logic. This patch contains a number of copies of the following code: + { + if (ereport_on_violation) + aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, + get_rel_name(relOid)); + return false; + } What if we don't pass ereport_on_violation down to ExecCheckRTEPerms(), and just have it return a boolean? Then ExecCheckRTPerms() can throw the error if ereport_on_violation is true, and return false otherwise. With this patch, ri_triggers.c emits a compiler warning, apparently due to a missing include. Otherwise, the changes look pretty sensible, though I haven't tested them yet. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] TODO 9.0 done items removed
Excerpts from Bruce Momjian's message of sáb jul 17 21:21:52 -0400 2010: Since we branched 9.1 before we released Postgres 9.0, I had to remove the 9.0 TODO items before 9.0 was released, or people might have marked items as done when they were done only in 9.1. Should we create a TodoDone90 page like this one? http://wiki.postgresql.org/wiki/TodoDone84 -- 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] TODO 9.0 done items removed
Alvaro Herrera wrote: Excerpts from Bruce Momjian's message of s??b jul 17 21:21:52 -0400 2010: Since we branched 9.1 before we released Postgres 9.0, I had to remove the 9.0 TODO items before 9.0 was released, or people might have marked items as done when they were done only in 9.1. Should we create a TodoDone90 page like this one? http://wiki.postgresql.org/wiki/TodoDone84 Sure. How is that done? :-O -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + None of us is going to be here forever. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Patch review: make RAISE without arguments work like Oracle
I'd like to mark this as Ready for Committer :) Review below. Cheers, David. == Submission review == * Is the patch in context diff format? Yes. * Does it apply cleanly to the current CVS HEAD? Yes, with a few offsets. patch -p0 ../reraise_issue_PG_v1.patch patching file src/pl/plpgsql/src/pl_exec.c Hunk #9 succeeded at 2427 (offset 2 lines). Hunk #10 succeeded at 2663 (offset 2 lines). patching file src/pl/plpgsql/src/plpgsql.h * Does it include reasonable tests, necessary doc patches, etc? No. Please find new patch attached with one test and one change to the docs. == Usability review == Read what the patch is supposed to do, and consider: * Does the patch actually implement that? Yes. * Do we want that? While the discussion was not long or extensive, the consensus seemed to be yes. * Do we already have it? No. * Does it follow SQL spec, or the community-agreed behavior? Not applicable, as far as I understand the spec. * Does it include pg_dump support (if applicable)? Not applicable. * Are there dangers? Old code may depend on the previous behavior. * Have all the bases been covered? == Feature test == Apply the patch, compile it and test: * Does the feature work as advertised? Yes. * Are there corner cases the author has failed to consider? Not that I've found. * Are there any assertion failures or crashes? No. **Review should be done with the ''configure'' options ''--enable-cassert'' and ''--enable-debug'' turned on; see [[Working with CVS]] for a full example. Those will help find issues with the code that might otherwise be missed. A copy of PostgreSQL built using these parameters will be substantially slower than one built without them. If you're working on something performance-related, such as testing whether a patch slows anything down, be sure to build without these flags before testing execution speed. You can turn off the assert testing, the larger of the slowdowns, at server start time by putting ''debug_assertions = false'' in your postgresql.conf. See [http://www.postgresql.org/docs/current/static/runtime-config-developer.html Developer Options] for more details about that setting; it defaults to true in builds done with --enable-cassert. == Performance review == * Does the patch slow down simple tests? Not that I've found, although anything involving exceptions in PL/pgsql performs pretty poorly in stock PostgreSQL. * If it claims to improve performance, does it? It makes no such claim. * Does it slow down other things? Not that I've found. == Coding review == Read the changes to the code in detail and consider: * Does it follow the project [http://developer.postgresql.org/pgdocs/postgres/source.html coding guidelines]? Yes. * Are there portability issues? Not that I can see. * Will it work on Windows/BSD etc? No reason it shouldn't. Haven't tested those platforms. * Are the comments sufficient and accurate? Yes. * Does it do what it says, correctly? Yes, as far as I can tell. * Does it produce compiler warnings? No. * Can you make it crash? No. == Architecture review == Consider the changes to the code in the context of the project as a whole: * Is everything done in a way that fits together coherently with other features/modules? Yes. * Are there interdependencies that can cause problems? Not that I've found, except as noted above re: apps based on the previous behavior. -- David Fetter da...@fetter.org 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 *** a/doc/src/sgml/plpgsql.sgml --- b/doc/src/sgml/plpgsql.sgml *** *** 2374,2379 SELECT merge_db(1, 'dennis'); --- 2374,2420 /para /example + example id=plpgsql-reraise-example + titleRe-Raising Exceptions/title + para + + This example shows how to re-raise an exception, which helps you + write more complex exception-handling in your functions: + programlisting + CREATE OR REPLACE FUNCTION raisetest() + RETURNS VOID + LANGUAGE plpgsql + AS $$ + BEGIN + BEGIN + RAISE syntax_error; + EXCEPTION + WHEN syntax_error THEN +BEGIN +raise notice 'exception thrown in inner block, reraising'; +RAISE; +EXCEPTION +WHEN OTHERS THEN +RAISE NOTICE 'RIGHT - exception caught in innermost block'; +END; + END; + EXCEPTION + WHEN OTHERS THEN + RAISE NOTICE 'WRONG - exception caught in outer block'; + END; + $$; + + select raisetest(); + NOTICE: exception thrown in inner block, reraising
Re: [HACKERS] standard_conforming_strings
On sön, 2010-07-18 at 09:42 -0700, David E. Wheeler wrote: On Jul 18, 2010, at 1:35 AM, Peter Eisentraut wrote: I think there are two ways we can do this, seeing that most appear to be in favor of doing it in the first place: Either we just flip the default, make a note in the release notes, and see what happens. Or we spend some time now and make, say, a list of driver versions and application versions that work with standard_conforming_strings = on, and then decide based on that, and also make that list a public resource for packagers etc. Do both. Turn them on, then make a list and inform driver maintainers who need to update. They've got a year, after all. Here we go then: http://wiki.postgresql.org/wiki/Standard_conforming_strings -- 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] SHOW TABLES
David Fetter da...@fetter.org wrote: Would something like this do? Thanks to Andrew Gierth for helping me figure out how to get this working :) CREATE OR REPLACE FUNCTION multi_result() RETURNS SETOF REFCURSOR With appropriate tweaks to JDBC and the other drivers, this would cover a lot of ground. You might be able to cover the last little bit by returning a SETOF some record with (at least) three columns, one of which would be filled in each row: REFCURSOR (for a result set), INTEGER (for a row count), and something which could carry an object which would map to a SQLWarning (which can be used with SQLSTATE '0' to deliver informational text or '01xxx' for actual warnings). A JDBC execute request (as opposed to executeUpdate or executeQuery) may get back any combination of the above in an ordered fashion. Essentially, this meta result set would need to be hidden within the Statement object. http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/sql/Statement.html#execute%28java.lang.String%29 http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/sql/Statement.html#getWarnings%28%29 -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] Parsing of aggregate ORDER BY clauses
2010/7/19 Tom Lane t...@sss.pgh.pa.us: I looked into the problem reported here: http://archives.postgresql.org/pgsql-bugs/2010-07/msg00119.php 1. Postpone coercion of the function inputs till after processing of the ORDER BY/DISTINCT decoration. This isn't too good because then we'll be using the wrong data type for deciding the semantics of ORDER BY/DISTINCT. That could lead to bizarre behavior or even crashes, eg if we try to use numeric sort operators on a value that actually has been coerced to float8. We could possibly go back and re-do the decisions about data types but it'd be a mess. 2. Split the processing of aggregates with ORDER BY/DISTINCT so that the sorting/uniqueifying is done in a separate expression node that can work with the native types of the given columns, and only after that do we perform coercion to the aggregate function's input types. This would be logically the cleanest thing, perhaps, but it'd represent a very major rework of the patch, with really no hope of getting it done for 9.0. 3. Do something so that we can still match t::text to t. This seems pretty awful on first glance but it's not actually that bad, because in the case we care about the cast will be marked as having been applied implicitly. Basically, instead of just equal() comparisons in findTargetlistEntrySQL99(), we'd strip off any implicit cast at the top of either expression, and only then do equal(). Since the implicit casts are, by definition, things the user didn't write, this would still have the expected behavior of matching expressions that were identical when the user wrote them. #3 seems the sanest fix, but I wonder if anyone has an objection or better idea. I didn't look at the code yet, #2 sounds like the way to go. But I see the breakage is unacceptable for 9.0, so #3 is the choice for 9.0 and will we fix it as #2 for 9.1 or later? Regards, -- Hitoshi Harada -- 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] Reworks of DML permission checks
(2010/07/20 3:13), Robert Haas wrote: 2010/7/12 KaiGai Koheikai...@ak.jp.nec.com: (2010/07/10 5:53), Robert Haas wrote: 2010/6/14 KaiGai Koheikai...@ak.jp.nec.com: The attached patch tries to rework DML permission checks. It was mainly checked at the ExecCheckRTEPerms(), but same logic was implemented in COPY TO/FROM statement and RI_Initial_Check(). This patch tries to consolidate these permission checks into a common function to make access control decision on DML permissions. It enables to eliminate the code duplication, and improve consistency of access controls. This patch is listed on the CommitFest page, but I'm not sure if it represents the latest work on this topic. At a minimum, it needs to be rebased. I am not excited about moving ExecCheckRT[E]Perms to some other place in the code. It seems to me that will complicate back-patching with no corresponding advantage. I'd suggest we not do that.The COPY and RI code can call ExecCheckRTPerms() where it is. Maybe at some point we will have a grand master plan for how this should all be laid out, but right now I'd prefer localized changes. OK, I rebased and revised the patch not to move ExecCheckRTPerms() from executor/execMain.c. In the attached patch, DoCopy() and RI_Initial_Check() calls that function to consolidate dml access control logic. This patch contains a number of copies of the following code: + { + if (ereport_on_violation) + aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, + get_rel_name(relOid)); + return false; + } What if we don't pass ereport_on_violation down to ExecCheckRTEPerms(), and just have it return a boolean? Then ExecCheckRTPerms() can throw the error if ereport_on_violation is true, and return false otherwise. All the error messages are indeed same, so it seems to me fair enough. As long as we don't need to report the error using aclcheck_error_col(), instead of aclcheck_error(), this change will keep the code simple. If it is preferable to show users the column-name in access violations, we need to raise an error from ExecCheckRTEPerms(). With this patch, ri_triggers.c emits a compiler warning, apparently due to a missing include. Oh, sorry, I'll fix it soon. Thanks, -- KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] standard_conforming_strings
On Mon, Jul 19, 2010 at 5:04 PM, Peter Eisentraut pete...@gmx.net wrote: On sön, 2010-07-18 at 09:42 -0700, David E. Wheeler wrote: On Jul 18, 2010, at 1:35 AM, Peter Eisentraut wrote: I think there are two ways we can do this, seeing that most appear to be in favor of doing it in the first place: Either we just flip the default, make a note in the release notes, and see what happens. Or we spend some time now and make, say, a list of driver versions and application versions that work with standard_conforming_strings = on, and then decide based on that, and also make that list a public resource for packagers etc. Do both. Turn them on, then make a list and inform driver maintainers who need to update. They've got a year, after all. Here we go then: http://wiki.postgresql.org/wiki/Standard_conforming_strings Looks like a good start. We might want to make an open items for 9.1 page and, as a first such open item, add a link to this page. Since it seems we have consensus, I will commit the patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] suppress automatic recovery after back crash
On Sat, Jul 17, 2010 at 10:16 AM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On Sun, 2010-06-27 at 20:54 -0400, Robert Haas wrote: automatic_restart = true # reinitialize after backend crash? automatic_restart makes me think when does that happen?. Can we call this restart_after_crash? Or similar. +1. automatic_restart is close to content-free. OK, committed that way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] standard_conforming_strings
On Mon, Jul 19, 2010 at 8:32 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Jul 19, 2010 at 5:04 PM, Peter Eisentraut pete...@gmx.net wrote: On sön, 2010-07-18 at 09:42 -0700, David E. Wheeler wrote: On Jul 18, 2010, at 1:35 AM, Peter Eisentraut wrote: I think there are two ways we can do this, seeing that most appear to be in favor of doing it in the first place: Either we just flip the default, make a note in the release notes, and see what happens. Or we spend some time now and make, say, a list of driver versions and application versions that work with standard_conforming_strings = on, and then decide based on that, and also make that list a public resource for packagers etc. Do both. Turn them on, then make a list and inform driver maintainers who need to update. They've got a year, after all. Here we go then: http://wiki.postgresql.org/wiki/Standard_conforming_strings Looks like a good start. We might want to make an open items for 9.1 page and, as a first such open item, add a link to this page. Since it seems we have consensus, I will commit the patch. So it turns out that (at least) the hstore and ECPG regression tests depend on the default setting of standard_conforming_strings. I have taken a crack at fixing this... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Reworks of DML permission checks
The attached patch is the revised one. * It was rebased to the latest git HEAD. * Prototype of ExecCheckRTEPerms() was changed; it become to return a bool value to inform the caller its access control decision, and its 'ereport_on_violation' argument has gone. * ExecCheckRTPerms() calls aclcheck_error() when ExecCheckRTEPerms() returned false, and 'ereport_on_violation' is true. * Add '#include executor/executor.h' on the ri_triggers.c. Thanks, (2010/07/20 9:24), KaiGai Kohei wrote: (2010/07/20 3:13), Robert Haas wrote: 2010/7/12 KaiGai Koheikai...@ak.jp.nec.com: (2010/07/10 5:53), Robert Haas wrote: 2010/6/14 KaiGai Koheikai...@ak.jp.nec.com: The attached patch tries to rework DML permission checks. It was mainly checked at the ExecCheckRTEPerms(), but same logic was implemented in COPY TO/FROM statement and RI_Initial_Check(). This patch tries to consolidate these permission checks into a common function to make access control decision on DML permissions. It enables to eliminate the code duplication, and improve consistency of access controls. This patch is listed on the CommitFest page, but I'm not sure if it represents the latest work on this topic. At a minimum, it needs to be rebased. I am not excited about moving ExecCheckRT[E]Perms to some other place in the code. It seems to me that will complicate back-patching with no corresponding advantage. I'd suggest we not do that.The COPY and RI code can call ExecCheckRTPerms() where it is. Maybe at some point we will have a grand master plan for how this should all be laid out, but right now I'd prefer localized changes. OK, I rebased and revised the patch not to move ExecCheckRTPerms() from executor/execMain.c. In the attached patch, DoCopy() and RI_Initial_Check() calls that function to consolidate dml access control logic. This patch contains a number of copies of the following code: + { + if (ereport_on_violation) + aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, + get_rel_name(relOid)); + return false; + } What if we don't pass ereport_on_violation down to ExecCheckRTEPerms(), and just have it return a boolean? Then ExecCheckRTPerms() can throw the error if ereport_on_violation is true, and return false otherwise. All the error messages are indeed same, so it seems to me fair enough. As long as we don't need to report the error using aclcheck_error_col(), instead of aclcheck_error(), this change will keep the code simple. If it is preferable to show users the column-name in access violations, we need to raise an error from ExecCheckRTEPerms(). With this patch, ri_triggers.c emits a compiler warning, apparently due to a missing include. Oh, sorry, I'll fix it soon. Thanks, -- KaiGai Kohei kai...@ak.jp.nec.com pgsql-v9.1-reworks-dml-checks.3.patch Description: application/octect-stream -- 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] [COMMITTERS] pgsql: pgindent run for 9.0, second run
On Tue, Jul 6, 2010 at 3:19 PM, Bruce Momjian momj...@postgresql.org wrote: Log Message: --- pgindent run for 9.0, second run It appears that the expletive git mirror has deduced the wrong contents for this commit. Apparently as a result, when I build from git master, the dblink regression tests fail. Can someone please fix this? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)
On Sun, Jul 18, 2010 at 2:00 PM, David Christensen da...@endpoint.com wrote: Updated the commitfest entry with the patch, updated the title to reflect the actual name of the command, and marked as ready for committer. I took a look at this patch. One problem is that it doesn't handle the case where there is no database connection (for example, shut down the database with pg_ctl, then do select 1, then do \conninfo). I've fixed that in the attached version. However, I'm also wondering about the output format. I feel like it would make sense to use a format similar to what we do for \c, which looks like this: You are now connected to database %s. It prints out more parameters if they've changed. The longest possible version is: You are now connected to database %s on host %s at port %s as user %s. My suggestion is that we use the same format, except (1) always include all the components, since that's the point; (2) don't include the word now; and (3) if there is no host, then print via local socket rather than on host...port So where the current patch prints: Connected to database: rhaas, user: rhaas, port: 5432 via local domain socket I would propose to print instead: You are connected to database rhaas via local socket as user rhaas. If people strongly prefer the way the patch does it now, I don't think it's horrible... but it seems like it would be nicer to be somewhat consistent with the existing message. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company psql-conninfo-v3.patch Description: Binary data -- 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] psql \conninfo command (was: Patch: psql \whoami option)
On Jul 19, 2010, at 10:34 PM, Robert Haas wrote: On Sun, Jul 18, 2010 at 2:00 PM, David Christensen da...@endpoint.com wrote: Updated the commitfest entry with the patch, updated the title to reflect the actual name of the command, and marked as ready for committer. I took a look at this patch. One problem is that it doesn't handle the case where there is no database connection (for example, shut down the database with pg_ctl, then do select 1, then do \conninfo). I've fixed that in the attached version. Thanks, I hadn't considered that case. However, I'm also wondering about the output format. I feel like it would make sense to use a format similar to what we do for \c, which looks like this: You are now connected to database %s. It prints out more parameters if they've changed. The longest possible version is: You are now connected to database %s on host %s at port %s as user %s. My suggestion is that we use the same format, except (1) always include all the components, since that's the point; (2) don't include the word now; and (3) if there is no host, then print via local socket rather than on host...port So where the current patch prints: Connected to database: rhaas, user: rhaas, port: 5432 via local domain socket I would propose to print instead: You are connected to database rhaas via local socket as user rhaas. If people strongly prefer the way the patch does it now, I don't think it's horrible... but it seems like it would be nicer to be somewhat consistent with the existing message. Thoughts? +1 from me; I don't care what color the bikeshed is, as long as it gets the point across, which this does, and is consistent to boot. Regards, David -- David Christensen End Point Corporation da...@endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Explicit psqlrc
On Wed, Jun 23, 2010 at 9:22 AM, Robert Haas robertmh...@gmail.com wrote: On Wed, Jun 23, 2010 at 9:17 AM, gabrielle gor...@gmail.com wrote: On Mon, Jun 21, 2010 at 6:16 PM, Robert Haas robertmh...@gmail.com wrote: Well, that might be a good idea, too, but my expectation is that: psql -f one -f two -f three ought to behave in a manner fairly similar to: cat one two three all psql -f all and it sounds like with this patch that's far from being the case. Correct. Each is handled individually. Should I continue to check on ON_ERROR_ROLLBACK results, or bounce this back to the author? It doesn't hurt to continue to review and find other problems so that the author can try to fix them all at once, but it certainly seems clear that it's not ready to commit at this point, so it definitely needs to go back to the author for a rework. Since it has been over a month since this review was posted and no new version of the patch has appeared, I think we should mark this patch as Returned with Feedback. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)
On Mon, Jul 19, 2010 at 11:41 PM, David Christensen da...@endpoint.com wrote: I took a look at this patch. One problem is that it doesn't handle the case where there is no database connection (for example, shut down the database with pg_ctl, then do select 1, then do \conninfo). I've fixed that in the attached version. Thanks, I hadn't considered that case. For some reason, I end up crashing a lot of databases during development (must be my lousy coding). So I've had occasion to run across this, ahem, a few times... +1 from me; I don't care what color the bikeshed is, as long as it gets the point across, which this does, and is consistent to boot. Great, committed that way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)
I would propose to print instead: You are connected to database rhaas via local socket as user rhaas. One minor quibble here; you lose the ability to see which pg instance you're running on if there are multiple ones running on different local sockets, so maybe either the port or the socket path should show up here still. Regards, David -- David Christensen End Point Corporation da...@endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql \conninfo command (was: Patch: psql \whoami option)
On Tue, Jul 20, 2010 at 12:02 AM, David Christensen da...@endpoint.com wrote: I would propose to print instead: You are connected to database rhaas via local socket as user rhaas. One minor quibble here; you lose the ability to see which pg instance you're running on if there are multiple ones running on different local sockets, so maybe either the port or the socket path should show up here still. Doh. Will fix. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] psql \conninfo command (was: Patch: psql \whoami option)
On Tue, Jul 20, 2010 at 12:07 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 20, 2010 at 12:02 AM, David Christensen da...@endpoint.com wrote: I would propose to print instead: You are connected to database rhaas via local socket as user rhaas. One minor quibble here; you lose the ability to see which pg instance you're running on if there are multiple ones running on different local sockets, so maybe either the port or the socket path should show up here still. Doh. Will fix. Something like the attached? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company psql-conninfo-fix.patch Description: Binary data -- 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] [COMMITTERS] pgsql: pgindent run for 9.0, second run
Robert Haas wrote: On Tue, Jul 6, 2010 at 3:19 PM, Bruce Momjian momj...@postgresql.org wrote: Log Message: --- pgindent run for 9.0, second run It appears that the expletive git mirror has deduced the wrong contents for this commit. Apparently as a result, when I build from git master, the dblink regression tests fail. Can someone please fix this? I despaired of this repo being anything like reliable months ago. AFAIK it is using a known to be broken version of fromcvs. 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] psql \conninfo command (was: Patch: psql \whoami option)
On Jul 19, 2010, at 11:10 PM, Robert Haas wrote: On Tue, Jul 20, 2010 at 12:07 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 20, 2010 at 12:02 AM, David Christensen da...@endpoint.com wrote: I would propose to print instead: You are connected to database rhaas via local socket as user rhaas. One minor quibble here; you lose the ability to see which pg instance you're running on if there are multiple ones running on different local sockets, so maybe either the port or the socket path should show up here still. Doh. Will fix. Something like the attached? Looks good to me. Regards, David -- David Christensen End Point Corporation da...@endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers