Re: [HACKERS] unite recovery.conf and postgresql.conf
On Mon, Oct 31, 2011 at 5:23 PM, Simon Riggs si...@2ndquadrant.com wrote: On Mon, Oct 31, 2011 at 7:38 AM, Fujii Masao masao.fu...@gmail.com wrote: In 9.2 the presence of recovery.conf can and therefore should continue to act as it does in 9.1. This means that recovery.conf is renamed to recovery.done at the end of recovery. IOW, all settings in recovery.conf are reset when recovery ends. Then if you run pg_ctl reload after recovery, you'll get something like the following error message and the reload will always fail; LOG: parameter standby_mode cannot be changed without restarting the server To resolve this issue, This issue exists whether or not we have recovery.conf etc., so yes, we must solve the problem. No. If we don't have recovery.conf, all parameters exist in postgresql.conf. The above issue would not occur unless a user makes a mistake, e.g., change the value of the parameter which cannot be changed without the server restart. I think that we need to introduce new GUC flag indicating parameters are used only during recovery, and need to define recovery parameters with that flag. Whenever pg_ctl reload is executed, all the processes check whether recovery is in progress, and ignore silently the parameters with that flag if not. I'm not sure how easy we can implement this. In addition to introducing that flag, we might need to change some processes which cannot access to the shared memory so that they can. Otherwise, they cannot know whether recovery is in progress. Or we might need to change them so that they always ignore recovery parameters. The postmaster knows whether its in recovery or not without checking shared memory. Various postmaster states describe this. If not postmaster, other backends can run recoveryinprogress() as normal. AFAIR archiver and syslogger cannot access to the shared memory, i.e., they cannot run RecoveryInProgress(). They don't use any recovery parameters for now, so we can change them so that they always ignore those parameters. Though I'm not inclined to add the process-specific code like the following into the GUC mechanism as much as possible. if (I am postmaster) { if (recovery is NOT in progress) reset the recovery parameters; } else if (I am archiver or syslogger) /* always ignore */ else { if (recovery is NOT in progress) reset the recovery parameters; } 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] psql expanded auto
I have not tried the patch (yet), but Informix'sl dbacess would do about the same - and it's something I really missed. Jan -- Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet. -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On Mon, Oct 31, 2011 at 12:54, Marcin Mańk marcin.m...@gmail.com wrote: How about an option for psql to truncate too long columns to X characters ? I would really want this in some form or another; for example, being able to hide bytea values entirely, or set limits to how many characters are displayed for fields. Unfortunately it's far less efficient. Fields would be truncated in psql, so full values are still detoasted and transmitted over the network. Regards, Marti -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On 1 November 2011 00:14, Andrew Dunstan and...@dunslane.net wrote: On 10/30/2011 10:00 PM, Christopher Browne wrote: I don't think I wish it. We're telling our developers not to use select *, and I don't think having select * except would change that policy, beyond requiring us to waste time explaining : No, we're not changing policy. The fact that PGDG added this to 9.2 does *not* imply our policy was wrong. That's fine, and it's a good policy. A good policy might well exclude use of a number of available features (e.g. one place I know bans doing joins with ',' instead of explicit join operators). But I don't think it helps us decide what to support. The fact is that if you have 100 columns and want 95 of them, it's very tedious to have to specify them all, especially for ad hoc queries where the house SQL standards really don't matter that much. I couldn't agree more with Andrew's comment. What's good for an ad hoc psql query isn't congruent with what's good for your application queries. We could have * EXCLUDING and still say that it is undesirable in all the same contexts that * is undesirable. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 00:18, Scott Mead sco...@openscg.com wrote: On Mon, Oct 31, 2011 at 6:13 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote: Actually, for the future, it might be useful to have a state column, that holds the idle/in transaction/running status, instead of the tools having to parse the query text to get that information... +1 for doing it this way. Splitting current_query into query and state would be more elegant and easier to use all around. I'm all for splitting it out actually. My concern was that I would break the 'ba-gillion' monitoring tools that already have support for pg_stat_activity if I dropped a column. What if we had: 'state' : idle | in transaction | running ( per Robert ) If we're going with breaking compatibility, waiting should be a state as well, I think. Actually, it should be even if we're not breaking compatibilty, but just adding a state field. 'current_query' : the most recent query (either last / currently running) That may be a bit tougher to get across to people though (especially in the case where state='IDLE'). I'll rework this when I don't have trick-or-treaters coming to the front door :) I think the question is how Ok it is to break compatibility. We could always leave current_query in there and create a new field for last_query *and* introduce a state... And then advertise a change in the future. But that might be too much... If we are doing it, it might be useful to just call it query, so that it is dead obvious to people that the definition changed.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 5:59 AM, Fujii Masao masao.fu...@gmail.com wrote: On Mon, Oct 31, 2011 at 5:23 PM, Simon Riggs si...@2ndquadrant.com wrote: On Mon, Oct 31, 2011 at 7:38 AM, Fujii Masao masao.fu...@gmail.com wrote: In 9.2 the presence of recovery.conf can and therefore should continue to act as it does in 9.1. This means that recovery.conf is renamed to recovery.done at the end of recovery. IOW, all settings in recovery.conf are reset when recovery ends. Then if you run pg_ctl reload after recovery, you'll get something like the following error message and the reload will always fail; LOG: parameter standby_mode cannot be changed without restarting the server To resolve this issue, This issue exists whether or not we have recovery.conf etc., so yes, we must solve the problem. No. If we don't have recovery.conf, all parameters exist in postgresql.conf. The above issue would not occur unless a user makes a mistake, e.g., change the value of the parameter which cannot be changed without the server restart. I don't understand what you are saying. You seem to be suggesting that it would be OK for someone to set standby_mode = on and reload the config and for that to go completely unremarked. If we are adding new parameters to postgresql.conf then its clear that some people will misconfigure them and we need to have error messages for that. If you change a parameter that only has effect during recovery then must get an error if it is changed during normal running. That is the reason we need to mark the recovery parameters with a new flag, so that we can ignore any errors caused by changing them. That has nothing at all to do with recovery.conf - you need it even if you put everything in postgresql.conf. I think that we need to introduce new GUC flag indicating parameters are used only during recovery, and need to define recovery parameters with that flag. Whenever pg_ctl reload is executed, all the processes check whether recovery is in progress, and ignore silently the parameters with that flag if not. I'm not sure how easy we can implement this. In addition to introducing that flag, we might need to change some processes which cannot access to the shared memory so that they can. Otherwise, they cannot know whether recovery is in progress. Or we might need to change them so that they always ignore recovery parameters. The postmaster knows whether its in recovery or not without checking shared memory. Various postmaster states describe this. If not postmaster, other backends can run recoveryinprogress() as normal. AFAIR archiver and syslogger cannot access to the shared memory, i.e., they cannot run RecoveryInProgress(). They don't use any recovery parameters for now, so we can change them so that they always ignore those parameters. Though I'm not inclined to add the process-specific code like the following into the GUC mechanism as much as possible. if (I am postmaster) { if (recovery is NOT in progress) reset the recovery parameters; } else if (I am archiver or syslogger) /* always ignore */ else { if (recovery is NOT in progress) reset the recovery parameters; } I don't see the problem. The code above could easily be simplified and only needs to exist in one place, not copied around for each GUC. When we had recovery.conf and postgresql.conf you knew which parameters took effects at specific times. That metadata needs to be added back into the system so we can report errors properly. Considering the amount of code we will be removing, a couple of extra lines seems trivial. AFAICS we need to reset all recovery parameters at the end of recovery anyway. Having SHOW recovery_target_xid return a value other than 0 is not appropriate during normal running, same for standby_mode and the other parameters. -- 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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 7:38 AM, Magnus Hagander mag...@hagander.net wrote: If we are doing it, it might be useful to just call it query, so that it is dead obvious to people that the definition changed.. Yeah. Otherwise, people who are parsing the hard-coded strings idle and idle in transaction are likely to get confused. -- 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs si...@2ndquadrant.com wrote: If you change a parameter that only has effect during recovery then must get an error if it is changed during normal running. I don't see why. If you're in normal running and someone changes a parameter that is irrelevant during normal running, that should be a no-op, not an error. -- 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
[HACKERS] Avoiding shutdown checkpoint at failover
When a server fails, we need to promote a standby as quickly as possible. Currently when we promote a standby to a primary we need to run a shutdown checkpoint before users can begin write transactions, which in many cases can take minutes. The reason we run a shutdown checkpoint is to prevent needing to re-enter recovery if we crash after promotion. When we only had file based replication, all WAL files were reloaded from archive each time, so the restartpoint prior to the end of recovery was not guaranteed to be available in pg_xlog. Once we had exited archive recovery it would be difficult to re-access the archive. Now with streaming replication, we keep the WAL files in pg_xlog directly, so the last restartpoint is always available if we should crash. So if streaming replication is active at the point we promote, then we can skip the shutdown checkpoint. It's that simple. To make it even simpler, I suggest we also change file de-archiving so that it writes normal WAL files, not RECOVERYXLOG, so that way we can avoid the checkpoint in all cases. There are comments saying we can only increment a timeline via a shutdown checkpoint, but if we were smart we'd have noticed the timeline change via the WAL file numbering anyway. Best way seems to be to have a XLOG_TIMELINE_CHANGE record written instead of the shutdown checkpoint. When I say skip the shutdown checkpoint, I mean remove it from the critical path of required actions at the end of recovery. We can still have a normal checkpoint kicked off at that time, but that no longer needs to be on the critical path. Any problems foreseen? If not, looks like a quick 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 12:06 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs si...@2ndquadrant.com wrote: If you change a parameter that only has effect during recovery then must get an error if it is changed during normal running. I don't see why. If you're in normal running and someone changes a parameter that is irrelevant during normal running, that should be a no-op, not an error. How will it be made into a no-op, except by having a specific flag to show that it is irrelevant during normal running? Fujii is saying we only need to mark GUCs if we keep recovery.conf. I am saying we need to mark them whatever we do elsewhere. -- 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] IDLE in transaction introspection
On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote: Actually, for the future, it might be useful to have a state column, that holds the idle/in transaction/running status, instead of the tools having to parse the query text to get that information... +1 for doing it this way. Splitting current_query into query and state would be more elegant and easier to use all around. Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. -- 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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 13:19, Simon Riggs si...@2ndquadrant.com wrote: On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote: Actually, for the future, it might be useful to have a state column, that holds the idle/in transaction/running status, instead of the tools having to parse the query text to get that information... +1 for doing it this way. Splitting current_query into query and state would be more elegant and easier to use all around. Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would be the backwards compatible way I suggested. That said, I think there's still value in exposing a state column, and to encourage people not to rely on the text in the query column. Then you can add it to your list of things to remove in 10.0 :-) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 12:41 PM, Magnus Hagander mag...@hagander.net wrote: On Tue, Nov 1, 2011 at 13:19, Simon Riggs si...@2ndquadrant.com wrote: On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote: Actually, for the future, it might be useful to have a state column, that holds the idle/in transaction/running status, instead of the tools having to parse the query text to get that information... +1 for doing it this way. Splitting current_query into query and state would be more elegant and easier to use all around. Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would be the backwards compatible way I suggested. Sorry Magnus, I hadn't read the whole thread. +1 to your suggestion. -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On Sat, Oct 29, 2011 at 5:26 PM, Eric Ridge eeb...@gmail.com wrote: Would y'all accept a patch that extended the SELECT * syntax to let you list fields to exclude from the A_Star? Quite regularly I'll be testing queries via psql and want to see all the columns from a fairly wide table except maybe a giant text or xml column. A syntax like: SELECT * EXCLUDING (big_col1, big_col2) FROM foo; would be pretty handy. It would definitely save some typing in certain cases. It seems like such a syntax would better document the intent of a query too, rather than leaving one wondering if big_col1 was supposed to be omitted from the target list or not. Anyways, I just wanted to run the idea by youse guys before I put too much more effort into it. I've already made what appear to be the minimum necessary changes to gram.y, and a few quick greps through the code make me think the rest will be pretty easy. Maybe the SQL spec says something about this and nobody's done the work yet? Thanks for your input! FWIW, this seems to come up all the time for me and I've often wondered about something like this. Just be advised that the bar for syntax extensions is very high because they can burn you down the line quite easily. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 8:14 AM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Nov 1, 2011 at 12:06 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs si...@2ndquadrant.com wrote: If you change a parameter that only has effect during recovery then must get an error if it is changed during normal running. I don't see why. If you're in normal running and someone changes a parameter that is irrelevant during normal running, that should be a no-op, not an error. How will it be made into a no-op, except by having a specific flag to show that it is irrelevant during normal running? By default, changing a GUC just updates the value of some global variable inside every backend. But unless there's some code that makes use of that global variable for some purpose, it doesn't have any practical effect. Apart from whatever complexities may be imposed by our choice of implementation, I don't see how this would be any different from setting maintenance_work_mem in a particular session and then not running any CREATE INDEX or VACUUM commands in that session. -- 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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 8:41 AM, Magnus Hagander mag...@hagander.net wrote: On Tue, Nov 1, 2011 at 13:19, Simon Riggs si...@2ndquadrant.com wrote: On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander mag...@hagander.net wrote: Actually, for the future, it might be useful to have a state column, that holds the idle/in transaction/running status, instead of the tools having to parse the query text to get that information... +1 for doing it this way. Splitting current_query into query and state would be more elegant and easier to use all around. Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would be the backwards compatible way I suggested. That said, I think there's still value in exposing a state column, and to encourage people not to rely on the text in the query column. Then you can add it to your list of things to remove in 10.0 :-) Personally, I think we're getting a bit overexcited about backward compatibility here. We make backward-incompatible changes in every release. Things that change the behavior of user queries (like reserving keywords, or other changes in syntax, or having the same syntax now mean something different) cause a fair amount of user pain, and we're justifiably cautious about making them. But changes that have to do with server administration, as far as I can see, result in much less pain. Splitting the current_query field into query and state is going to require only the most minimal adjustment to user code, and anyone for whom it's really a problem can easily create their own view that mimics the old behavior. On the flip side, keeping both fields around is going to make the pg_stat_activity, which is already extremely wide, even more difficult to read in a window of any reasonable width, especially because the field we're proposing to duplicate is the widest one by far. I also doubt very much that it creates a meaningful migration path; most people will just keep doing it the old way until it breaks, and even new users may not realize that one column is nominally deprecated. -- 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] Avoiding shutdown checkpoint at failover
Simon Riggs si...@2ndquadrant.com writes: The reason we run a shutdown checkpoint is to prevent needing to re-enter recovery if we crash after promotion. That's *a* reason, it's not necessarily the only reason. This proposal worries me, especially your blithe dismissal of the timeline issues; but in any case I would not trust it without a detailed review of all WAL replay activities, which you don't sound to have done. 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] IDLE in transaction introspection
Simon Riggs si...@2ndquadrant.com writes: Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. I could go either way on whether to rename it. If anyone's really hot about backward compatibility, it would not be very hard to create a view that replicates the old behavior working from a state column and a current-or-last-query column. 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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 1:52 PM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. I could go either way on whether to rename it. That's a good reason. If anyone's really hot about backward compatibility, it would not be very hard to create a view that replicates the old behavior working from a state column and a current-or-last-query column. I'm in favour of change, when that has a purpose, just like you. -- 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 9:45 AM, Simon Riggs si...@2ndquadrant.com wrote: Why do we have this log message then, if it is OK to ignore changes that have no effect? LOG: parameter shared_buffers cannot be changed without restarting the server I believe we're logging the fact that we were unable to make the change, not the fact that it didn't have any effect. Certainly, it *would* have an effect, if we were able to make it. But we can't, without a restart, so we tell that to the user. But, for example, the hot_standby GUC - which already exists - does not do anything in normal running. We don't need to (and don't) complain if the user tries to change the value in normal running, though: they're presumably just hoping it will take effect the next time they start up, which it will. And the autovacuum parameter does not do anything during hot standby, but we don't need to (and don't) complain if the user changes it them; it just takes effect when we enter normal running. The only time I think we need to complain about an effort to change a GUC is when the GUC won't take effect as soon as the user might normally expect. -- 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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. Not really. You could just store it once in shared memory, and put the complexity in the view definition. -- 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] IDLE in transaction introspection
On Mon, Oct 31, 2011 at 23:37, Scott Mead sco...@openscg.com wrote: So I wrote the attached patch, it just turns IDLE in transaction into: IDLE in transaction\n: Previous: last query executed. After seeing how quickly our dev's fixed the issue once they saw prepared statement XYZ, Solving this problem is a good idea, but I don't particularly like the proposed solution. I think the proposed state/query split is going to make pg_stat_activity more confusing, especially if even idle connections get a query string. And if we display the last query there, why not the one before that? etc. (Adding a state column might not be a bad idea though) I'd very much like to see a more generic solution: a runtime query log facility that can be queried in any way you want. pg_stat_statements comes close, but is limited too due to its (arbitrary, I find) deduplication -- you can't query for 10 last statements from process N since it has no notion of processes, just users and databases. So far my developers are simply grepping pg_log, but you can't use the full power of SQL there. I know there's csvlog, but the pains aren't big enough to make attempt to use that. Regards, Marti -- 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 1:23 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 1, 2011 at 8:14 AM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Nov 1, 2011 at 12:06 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs si...@2ndquadrant.com wrote: If you change a parameter that only has effect during recovery then must get an error if it is changed during normal running. I don't see why. If you're in normal running and someone changes a parameter that is irrelevant during normal running, that should be a no-op, not an error. How will it be made into a no-op, except by having a specific flag to show that it is irrelevant during normal running? By default, changing a GUC just updates the value of some global variable inside every backend. But unless there's some code that makes use of that global variable for some purpose, it doesn't have any practical effect. Apart from whatever complexities may be imposed by our choice of implementation, I don't see how this would be any different from setting maintenance_work_mem in a particular session and then not running any CREATE INDEX or VACUUM commands in that session. Why do we have this log message then, if it is OK to ignore changes that have no effect? LOG: parameter shared_buffers cannot be changed without restarting the server -- 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 2:04 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 1, 2011 at 9:45 AM, Simon Riggs si...@2ndquadrant.com wrote: Why do we have this log message then, if it is OK to ignore changes that have no effect? LOG: parameter shared_buffers cannot be changed without restarting the server I believe we're logging the fact that we were unable to make the change, not the fact that it didn't have any effect. Certainly, it *would* have an effect, if we were able to make it. But we can't, without a restart, so we tell that to the user. But, for example, the hot_standby GUC - which already exists - does not do anything in normal running. We don't need to (and don't) complain if the user tries to change the value in normal running, though: they're presumably just hoping it will take effect the next time they start up, which it will. If there is precedence then we should follow it. So no error messages. The only reason this point has any importance is that Fujii is suggesting the code will be much more complicated if we retain recovery.conf compatibility, since we need to mark GUCs with a flag as to whether they can take effect during recovery. I don't think that's a lot of work to add, and would make the code clearer than it is now. And the autovacuum parameter does not do anything during hot standby, but we don't need to (and don't) complain if the user changes it them; it just takes effect when we enter normal running. That one needs to be set on a standby, in case it becomes a primary. -- 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] Avoiding shutdown checkpoint at failover
On Tue, Nov 1, 2011 at 1:48 PM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: The reason we run a shutdown checkpoint is to prevent needing to re-enter recovery if we crash after promotion. That's *a* reason, it's not necessarily the only reason. This proposal worries me, especially your blithe dismissal of the timeline issues; but in any case I would not trust it without a detailed review of all WAL replay activities, which you don't sound to have done. What timeline issues are you thinking of? Timelines were invented to avoid confusion with PITR. The reality is that they don't have much reason to exist in the world of replication and could be dispensed with in that context easily if there are issues associated with them. I believe the solution to be simple and wish it had occurred to me earlier. If you can think of a reason to not do this, let me know. -- 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] IDLE in transaction introspection
On 2011-11-01 21:13, Andrew Dunstan wrote: Rename it please. current_query will just be wrong. I'd be inclined just to call it query or query_string and leave it to the docs to define the exact semantics. I think query for a query that isn't ongoing would be just as wrong. How about last_query? Jeroen -- 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] IDLE in transaction introspection
Robert Haas robertmh...@gmail.com writes: On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. Not really. You could just store it once in shared memory, and put the complexity in the view definition. I understood the proposal to be store the previous query in addition to the current-query-if-any. If that's not what was meant, then my objection was incorrect. However, like you, I'm pretty dubious of having two mostly-redundant fields in the view definition, just because of window width issues. 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] LDAP server docs
On Mon, Oct 31, 2011 at 20:59, Magnus Hagander mag...@hagander.net wrote: On Mon, Oct 31, 2011 at 20:58, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: So once again I forgot about the fact that you can specify multiple LDAP server in our ldapserver parameter (because both openldap and winldap accept a space separated list). Any objections to just applying the attached docs patch? space-separated list is more in keeping with our usual terminology, I think, but otherwise please do. FWIW, the use of the word blank was just because I copied it off the ldap manpage. I agree space is better :-) Updated and applied. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Compiler branch prediction hints (was: So, is COUNT(*) fast now?)
On Fri, Oct 28, 2011 at 20:58, Robert Haas robertmh...@gmail.com wrote: I tried sprinkling a little branch-prediction magic on this code using GCC's __builtin_expect(). That initially seemed to help, but once I changed the BufferIsValid() test to !BufferIsInvalid() essentially all of the savings disappeared. Sounds like mere chance that the compiler decided to lay it out in one way or another. A different compiler version could pick a different path. I quite like the likely() and unlikely() macros used in Linux kernel; much more readable than __builtin_expect and they might also be useful for (usually redundant) error checks and asserts in hot code paths. It looks like this: #ifdef __GNUC__ # define unlikely(xpr) __builtin_expect(xpr, 0) #else # define unlikely(xpr) (xpr) #endif if (unlikely(blkno = rel-rd_smgr-smgr_vm_nblocks)) { /* unlikely branch here */ } However, the wins are probably minor because most of the time, adaptive CPU branch prediction will override that. Do you think this would be a useful patch to attempt? Regards, Marti -- 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] Compiler branch prediction hints (was: So, is COUNT(*) fast now?)
On Tue, Nov 1, 2011 at 10:47 AM, Marti Raudsepp ma...@juffo.org wrote: On Fri, Oct 28, 2011 at 20:58, Robert Haas robertmh...@gmail.com wrote: I tried sprinkling a little branch-prediction magic on this code using GCC's __builtin_expect(). That initially seemed to help, but once I changed the BufferIsValid() test to !BufferIsInvalid() essentially all of the savings disappeared. Sounds like mere chance that the compiler decided to lay it out in one way or another. A different compiler version could pick a different path. I quite like the likely() and unlikely() macros used in Linux kernel; much more readable than __builtin_expect and they might also be useful for (usually redundant) error checks and asserts in hot code paths. It looks like this: #ifdef __GNUC__ # define unlikely(xpr) __builtin_expect(xpr, 0) #else # define unlikely(xpr) (xpr) #endif if (unlikely(blkno = rel-rd_smgr-smgr_vm_nblocks)) { /* unlikely branch here */ } However, the wins are probably minor because most of the time, adaptive CPU branch prediction will override that. Do you think this would be a useful patch to attempt? Well, the obvious problem is that we might end up spending a lot of work on something that doesn't actually improve performance, or even makes it worse, if our guesses about what's likely and unlikely turn out to be wrong. If we can show cases where it reliably produces a significant speedup, then I would think it would be worthwhile, but I think we should probably start by looking at what's slow and see how it can best be made faster rather than by looking specifically for places to use likely() and unlikely(). -- 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] IDLE in transaction introspection
2011/11/1 Marti Raudsepp ma...@juffo.org: On Mon, Oct 31, 2011 at 23:37, Scott Mead sco...@openscg.com wrote: So I wrote the attached patch, it just turns IDLE in transaction into: IDLE in transaction\n: Previous: last query executed. After seeing how quickly our dev's fixed the issue once they saw prepared statement XYZ, Solving this problem is a good idea, but I don't particularly like the proposed solution. I think the proposed state/query split is going to make pg_stat_activity more confusing, especially if even idle connections get a query string. And if we display the last query there, why not the one before that? etc. (Adding a state column might not be a bad idea though) I'd very much like to see a more generic solution: a runtime query log facility that can be queried in any way you want. pg_stat_statements comes close, but is limited too due to its (arbitrary, I find) deduplication -- you can't query for 10 last statements from process N since it has no notion of processes, just users and databases. I like the idea to have a pg_stat_activity with an history, it can be in another view with a GUC to define how many queries to keep per pid. So far my developers are simply grepping pg_log, but you can't use the full power of SQL there. I know there's csvlog, but the pains aren't big enough to make attempt to use that. Regards, Marti -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation -- 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 6:36 PM, Josh Berkus j...@agliodbs.com wrote: On 11/1/11 10:34 AM, Simon Riggs wrote: On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus j...@agliodbs.com wrote: So, we have four potential paths regarding recovery.conf: 1) Break backwards compatibility entirely, and stop supporting recovery.conf as a trigger file at all. Note that is exactly what I have suggested when using standby mode from pg_ctl. I wasn't clear on that from the description of your proposal. So are you suggesting that, if we start postgresql with pg_ctl standby then recovery.conf would not behave as a trigger file? Yes -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
2011/11/1 Eric Ridge eeb...@gmail.com: On Tue, Nov 1, 2011 at 12:24 PM, Pavel Stehule pavel.steh...@gmail.com wrote: some other idea - but only for psql we can define a special values, that ensure a some necessary preexecution alchemy with entered query \pset star_exclude_names col1, col2, col3 \pset star_exclude_types xml, bytea, text(unlimited) Sure, something like that could be useful too. It might be confusing to users if they forget that they set an exclusion list, but there's probably ways to work around that. However, the nice thing about the feature being in SQL is that you can use it from all clients, and even in other useful ways. COPY would be an example (something I also do frequently): COPY (SELECT * EXCLUDING (a, b, c) FROM big query) TO 'somefile.csv' WITH CSV; Right now, if you want to exclude a column, you have to list all the others out manually, or just dump everything and deal with it in an external tool. I generally agree with everyone that says using this in application code is a bad idea, but I don't think that's reason alone to reject the idea on its face. eric -- 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 expanded auto
Peter Eisentraut pete...@gmx.net writes: Here is a finalized patch for this. The first hunk of the patch is the documentation change, so you can see there how it's supposed to work. Let me know what you think. +1 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] [v9.2] Object access hooks with arguments support (v1)
2011/10/21 Robert Haas robertmh...@gmail.com: On Fri, Oct 21, 2011 at 12:44 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: I had checked my older implementation based on 8.4.x or 9.0.x that includes all the features that I want to implement. At least, it does not require so much different information from ones needed by DAC model, although SELECT INTO was an exception. It might be quite natural because both works similar things. OK. It seems like it might be helpful to put together a list of all of the things you want to check permissions on, maybe on a wiki page somewhere, and indicate in there which ones are done and which ones are not done, and what additional information you think is needed in each case, and flag any MAC/DAC discrepancies that you are concerned about. I think that might help us reach agreement on the best way forward. Also, then, as we get things committed, we can track progress versus that roadmap. I tried to summarize permission checks of DAC/MAC on several object classes that are allowed to assign security label right now. http://wiki.postgresql.org/index.php?title=SEPostgreSQL/Permissions In most of checks, required contextual information by SELinux are commonly used to DAC also, as listed. On the creation of relations, SELinux requires relkind and TupleDesc to identify relation type and columns. Unlike a flag of select-into, I'm not sure whether it is a model specific and hard to manage without knowledge about sepgsql internal. In some cases, DAC uses superuser privilege as a substitute for individual permission checks, such as DefineType(). It seems to me its original implementation checked CREATE permission of the namespace and ownership of the functions that implements type input, output or others, then, we commented out these code because superuser is obviously allowed these checks. However, MAC does not have concept of superuser, so, SELinux wants to check db_procedure:{install} for each functions, thus, it requires oid of the functions to be used for the new type. Of course, we can see here is no difference between DAC and MAC, if we consider DAC implicitly allows these checks by superuser(). I think it is a good start to implement first ddl permissions on creation of the seven object classes as listed; also to proof the concept; 1) we put permission checks around existing DAC checks, 2) we deliver contextual data (basically) used in existing DAC also. I guess DROP or some of ALTER code reworking should be done prior to deploy object_access_hook around their permission checks, to minimize maintain efforts. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- 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 if 'postgres' database is dropped
Robert Haas wrote: On Tue, Nov 1, 2011 at 2:49 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: It turns out there was only one place that expected a 1-1 mapping of old and new databases (file transfer), so I just modified that code to allow skipping a database in the new cluster that didn't exist in the old cluster. Urp. ?But that means that if someone has any data in that database, pg_upgrade will basically eat it. ?That does not seem like a step forward. Please clarify? ?We already check that all the new cluster databases are empty, so we are effectively skipping the transfering of files into empty new cluster databases. ?It processes all old cluster databases and forces a new cluster match --- it is only empty new cluster database that are being skipped. Aren't you saying that if a postgres database exists in the old database (and potentially contains data) but is missing in the new database, we'll just fail to migrate it? No, the reverse. ?If the 'postgres' database exists in the new cluster, but not in the old, we allow it to upgrade (we skip over the 'postgres' database in the new cluster use the loop in the patch). Oh, OK. That seems fine - in fact, that seems perfect. Unless I am missing something. ?Did you see something odd in the patch or in my wording? Your wording confused me, but on further review I think I'm just easily confused. The concept is pretty confusing and I had to think a while before I came up with this approach. -- Bruce Momjian br...@momjian.ushttp://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 if 'postgres' database is dropped
Robert Haas wrote: On Tue, Nov 1, 2011 at 1:53 PM, Bruce Momjian br...@momjian.us wrote: Bruce Momjian wrote: What I would prefer is to have the upgrade succeed, and just ignore the existence of a postgres database in the new cluster. ?Maybe give the user a notice and let them decide whether they wish to take any action. ?I understand that failing is probably less code, but IMHO one of the biggest problems with pg_upgrade is that it's too fragile: there are too many seemingly innocent things that can make it croak (which isn't good, when you consider that anyone using pg_upgrade is probably in a hurry to get the upgrade done and the database back on-line). ?It seems like this is an opportunity to get rid of one of those unnecessary failure cases. OK, then the simplest fix, once you modify pg_dumpall, would be to modify pg_upgrade to remove reference to the postgres database in the new cluster if it doesn't exist in the old one. ?That would allow pg_upgrade to maintain a 1-1 matching of databases in the old and new cluster --- it allows the change to be locallized without affecting much code. I fixed this a different way. ?I originally thought I could skip over the 'postgres' database in the new cluster if it didn't exist in the old cluster, but we have do things like check it is empty, so that was going to be awkward. It turns out there was only one place that expected a 1-1 mapping of old and new databases (file transfer), so I just modified that code to allow skipping a database in the new cluster that didn't exist in the old cluster. Urp. But that means that if someone has any data in that database, pg_upgrade will basically eat it. That does not seem like a step forward. Please clarify? We already check that all the new cluster databases are empty, so we are effectively skipping the transfering of files into empty new cluster databases. It processes all old cluster databases and forces a new cluster match --- it is only empty new cluster database that are being skipped. -- Bruce Momjian br...@momjian.ushttp://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 if 'postgres' database is dropped
Robert Haas wrote: It turns out there was only one place that expected a 1-1 mapping of old and new databases (file transfer), so I just modified that code to allow skipping a database in the new cluster that didn't exist in the old cluster. Urp. ?But that means that if someone has any data in that database, pg_upgrade will basically eat it. ?That does not seem like a step forward. Please clarify? ?We already check that all the new cluster databases are empty, so we are effectively skipping the transfering of files into empty new cluster databases. ?It processes all old cluster databases and forces a new cluster match --- it is only empty new cluster database that are being skipped. Aren't you saying that if a postgres database exists in the old database (and potentially contains data) but is missing in the new database, we'll just fail to migrate it? No, the reverse. If the 'postgres' database exists in the new cluster, but not in the old, we allow it to upgrade (we skip over the 'postgres' database in the new cluster use the loop in the patch). Unless I am missing something. Did you see something odd in the patch or in my wording? -- Bruce Momjian br...@momjian.ushttp://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 if 'postgres' database is dropped
Bruce Momjian wrote: What I would prefer is to have the upgrade succeed, and just ignore the existence of a postgres database in the new cluster. Maybe give the user a notice and let them decide whether they wish to take any action. I understand that failing is probably less code, but IMHO one of the biggest problems with pg_upgrade is that it's too fragile: there are too many seemingly innocent things that can make it croak (which isn't good, when you consider that anyone using pg_upgrade is probably in a hurry to get the upgrade done and the database back on-line). It seems like this is an opportunity to get rid of one of those unnecessary failure cases. OK, then the simplest fix, once you modify pg_dumpall, would be to modify pg_upgrade to remove reference to the postgres database in the new cluster if it doesn't exist in the old one. That would allow pg_upgrade to maintain a 1-1 matching of databases in the old and new cluster --- it allows the change to be locallized without affecting much code. I fixed this a different way. I originally thought I could skip over the 'postgres' database in the new cluster if it didn't exist in the old cluster, but we have do things like check it is empty, so that was going to be awkward. It turns out there was only one place that expected a 1-1 mapping of old and new databases (file transfer), so I just modified that code to allow skipping a database in the new cluster that didn't exist in the old cluster. Attached patch applied. This allows an upgrade if the 'postgres' database is missing from the old cluster. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c new file mode 100644 index e400814..d32a84c *** a/contrib/pg_upgrade/check.c --- b/contrib/pg_upgrade/check.c *** *** 14,20 static void set_locale_and_encoding(ClusterInfo *cluster); static void check_new_cluster_is_empty(void); - static void check_old_cluster_has_new_cluster_dbs(void); static void check_locale_and_encoding(ControlData *oldctrl, ControlData *newctrl); static void check_is_super_user(ClusterInfo *cluster); --- 14,19 *** check_new_cluster(void) *** 127,133 check_new_cluster_is_empty(); check_for_prepared_transactions(new_cluster); - check_old_cluster_has_new_cluster_dbs(); check_loadable_libraries(); --- 126,131 *** check_new_cluster_is_empty(void) *** 382,420 /* - * If someone removes the 'postgres' database from the old cluster and - * the new cluster has a 'postgres' database, the number of databases - * will not match. We actually could upgrade such a setup, but it would - * violate the 1-to-1 mapping of database counts, so we throw an error - * instead. We would detect this as a database count mismatch during - * upgrade, but we want to detect it during the check phase and report - * the database name. - */ - static void - check_old_cluster_has_new_cluster_dbs(void) - { - int old_dbnum, - new_dbnum; - - for (new_dbnum = 0; new_dbnum new_cluster.dbarr.ndbs; new_dbnum++) - { - for (old_dbnum = 0; old_dbnum old_cluster.dbarr.ndbs; old_dbnum++) - if (strcmp(old_cluster.dbarr.dbs[old_dbnum].db_name, - new_cluster.dbarr.dbs[new_dbnum].db_name) == 0) - break; - if (old_dbnum == old_cluster.dbarr.ndbs) - { - if (strcmp(new_cluster.dbarr.dbs[new_dbnum].db_name, postgres) == 0) - pg_log(PG_FATAL, The \postgres\ database must exist in the old cluster\n); - else - pg_log(PG_FATAL, New cluster database \%s\ does not exist in the old cluster\n, - new_cluster.dbarr.dbs[new_dbnum].db_name); - } - } - } - - - /* * create_script_for_old_cluster_deletion() * * This is particularly useful for tablespace deletion. --- 380,385 *** create_script_for_old_cluster_deletion(c *** 462,468 fprintf(script, RM_CMD %s%s/PG_VERSION\n, os_info.tablespaces[tblnum], old_cluster.tablespace_suffix); ! for (dbnum = 0; dbnum new_cluster.dbarr.ndbs; dbnum++) { fprintf(script, RMDIR_CMD %s%s/%d\n, os_info.tablespaces[tblnum], old_cluster.tablespace_suffix, --- 427,433 fprintf(script, RM_CMD %s%s/PG_VERSION\n, os_info.tablespaces[tblnum], old_cluster.tablespace_suffix); ! for (dbnum = 0; dbnum old_cluster.dbarr.ndbs; dbnum++) { fprintf(script, RMDIR_CMD %s%s/%d\n, os_info.tablespaces[tblnum], old_cluster.tablespace_suffix, diff --git a/contrib/pg_upgrade/function.c b/contrib/pg_upgrade/function.c new file mode 100644 index 0f80089..b154f03 *** a/contrib/pg_upgrade/function.c --- b/contrib/pg_upgrade/function.c *** get_loadable_libraries(void) *** 132,139 int totaltups; int dbnum;
Re: [HACKERS] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 6:12 PM, Robert Treat r...@xzilla.net wrote: On Tue, Nov 1, 2011 at 1:34 PM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus j...@agliodbs.com wrote: So, we have four potential paths regarding recovery.conf: 1) Break backwards compatibility entirely, and stop supporting recovery.conf as a trigger file at all. Note that is exactly what I have suggested when using standby mode from pg_ctl. But you already know that, since you said If it's possible to run a replica without having a recovery.conf file, then I'm fine with your solution, and I already confirmed back to you that would be possible. It's possible to run a replica without having a recovery.conf file is not the same thing as If someone makes a recovery.conf file, it won't break my operations. AIUI, you are not supporting the latter. Yes, that is part of the combined proposal, which allows both old and new APIs. New API pg_ctl standby will startup a server in standby mode, do not implicitly include recovery.conf and disallow recovery_target parameters in postgresql.conf (you may, if you wish, explicitly have include recovery.conf in your postgresql.conf, should you desire that) Old API pg_ctl start and a recovery.conf has been created will startup a server in PITR and/or replication, recovery.conf will be read automatically (as now) so the presence of the recovery.conf acts as a trigger, only if we issue start So the existing syntax works exactly as now, but a new API has been created to simplify things in exactly the way requested. The old and the new API don't mix, so there is no confusion between them. You must still use the old API when you wish to perform a PITR, as explained by me, following comments by Peter. There is no significant additional code or complexity required to do this, but it adds considerable usefulness. -- 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] pg_upgrade if 'postgres' database is dropped
On Tue, Nov 1, 2011 at 2:36 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: On Tue, Nov 1, 2011 at 1:53 PM, Bruce Momjian br...@momjian.us wrote: Bruce Momjian wrote: What I would prefer is to have the upgrade succeed, and just ignore the existence of a postgres database in the new cluster. ?Maybe give the user a notice and let them decide whether they wish to take any action. ?I understand that failing is probably less code, but IMHO one of the biggest problems with pg_upgrade is that it's too fragile: there are too many seemingly innocent things that can make it croak (which isn't good, when you consider that anyone using pg_upgrade is probably in a hurry to get the upgrade done and the database back on-line). ?It seems like this is an opportunity to get rid of one of those unnecessary failure cases. OK, then the simplest fix, once you modify pg_dumpall, would be to modify pg_upgrade to remove reference to the postgres database in the new cluster if it doesn't exist in the old one. ?That would allow pg_upgrade to maintain a 1-1 matching of databases in the old and new cluster --- it allows the change to be locallized without affecting much code. I fixed this a different way. ?I originally thought I could skip over the 'postgres' database in the new cluster if it didn't exist in the old cluster, but we have do things like check it is empty, so that was going to be awkward. It turns out there was only one place that expected a 1-1 mapping of old and new databases (file transfer), so I just modified that code to allow skipping a database in the new cluster that didn't exist in the old cluster. Urp. But that means that if someone has any data in that database, pg_upgrade will basically eat it. That does not seem like a step forward. Please clarify? We already check that all the new cluster databases are empty, so we are effectively skipping the transfering of files into empty new cluster databases. It processes all old cluster databases and forces a new cluster match --- it is only empty new cluster database that are being skipped. Aren't you saying that if a postgres database exists in the old database (and potentially contains data) but is missing in the new database, we'll just fail to migrate it? -- 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] unite recovery.conf and postgresql.conf
Robert, In most cases we either break backwards compatibility or require some type of switch to turn on backwards compatibility for those who want it. While the above plan tries to do one better, it leaves me feeling that the thing I don't like about this is that it sounds like you are forcing backwards compatibility on people who would much rather just do things the new way. Given that, I foresee a whole new generation of confused users who end up setting their configs one way only to have someone else set the same config in the other file, or some tool dump out some config file, overriding what was really intended. This will also make things *harder* for those tool providers you are trying to help, as they will be forced to support the behavior *both ways*. I'd much rather see some type of switch which turns on the old behavior for those who really want it, because while you can teach the new behavior, if you can't prevent the old behavior, you're creating operational headaches for yourself. This is a good point. There's also the second drawback, which is complexity of code, which I believe that Tom Lane has brought up before; having two separate-but-equal paths for configuration is liable to lead to a lot of bugs. So, we have four potential paths regarding recovery.conf: 1) Break backwards compatibility entirely, and stop supporting recovery.conf as a trigger file at all. 2) Offer backwards compatibility only if recovery_conf='filename' is set in postgresql.conf, then behave like Simon's compromise. 3) Simon's compromise. 4) Don't ever change how recovery.conf works. The only two of the above I see as being real options are (1) and (2). (3) would, as Robert points out, cause DBAs to have unpleasant surprises when some third-party tool creates a recovery.conf they weren't expecting. So: (1) pros: * new, clean API * makes everyone update their tools * no confusion on how to do failover * code simplicity cons: * breaks a bunch of 3rd-party tools * or forces them to maintain separate 9.1 and 9.2 branches (2) pros: * allows people to use only new API if they want * allows gradual update of tools * can also lump in relocatable recovery.conf as feature cons: * puts off the day when vendors pay attention to the new API (and even more kicking screaming when that day comes) * confusion about how to do failover * code complexity -- 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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote: On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. Not really. You could just store it once in shared memory, and put the complexity in the view definition. I understood the proposal to be store the previous query in addition to the current-query-if-any. If that's not what was meant, then my objection was incorrect. However, like you, I'm pretty dubious of having two mostly-redundant fields in the view definition, just because of window width issues. The biggest reason I dislike the multi-field approach is because it limits us to only the [single] previous_query in the system with all the overhead we talked about (memory, window width and messing with system catalogs in general). That's actually why I implemented it the way I did, just by appending the last query on the end of the string when it's IDLE in transaction. Well, by appending it in that field, you require the end user/monitoring app to parse out the string anyway, so you're not exactly making life easier on the consumer of the information.. Marti wrote: I'd very much like to see a more generic solution: a runtime query log facility that can be queried in any way you want. pg_stat_statements comes close, but is limited too due to its (arbitrary, I find) deduplication -- you can't query for 10 last statements from process N since it has no notion of processes, just users and databases. This is what I'd really like to see (just haven't had time as it is a much bigger project). The next question my devs ask is what were the last five queries that ran... can you show me an overview of an entire transaction etc... That being said, having the previous_query available feels like it fixes about 80% of the *problem*; transaction profiling, or looking back 10 / 15 / 20 queries is [immensely] useful, but I find that the bigger need is the ability to short-circuit dba / dev back-n-forth by just saying Your app refused to commit/rollback after running query XYZ. This would be great, but as you say, it's a different project. Perhaps something could be made along the line of each backend keeping it's *own* set of old queries, and then making it available to a specific function (pg_get_last_queries_for_backend(nnn)) - since this is not the type of information you'd ask for often, it would be ok if getting this data took longer.. And you seldom want give me the last 5 queries for each backend at once. Robert Wrote: Yeah. Otherwise, people who are parsing the hard-coded strings idle and idle in transaction are likely to get confused. I would be interested ( and frankly very surprised ) to find out if many monitoring tools are actually parsing that field. Most that I see just dump whatever is in current_query to the user. I would imaging that, so long as the server obeyed pgstat_track_activity_size most tools would behave nicely. Really? I'd assume every single monitoring tool that graphs how many active connections you have (vs idle vs idle in transaction) would do just this. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] [v9.2] Object access hooks with arguments support (v1)
On Tue, Nov 1, 2011 at 1:32 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: I tried to summarize permission checks of DAC/MAC on several object classes that are allowed to assign security label right now. http://wiki.postgresql.org/index.php?title=SEPostgreSQL/Permissions In most of checks, required contextual information by SELinux are commonly used to DAC also, as listed. What's up with this: a flag to inform whether CASCADE or RESTRICT That doesn't seem like it should be needed. We should consider whether CREATE TABLE should be considered to consist of creating a table and then n attributes, rather than trying to shove the attribute information wholesale into the create table check. I guess DROP or some of ALTER code reworking should be done prior to deploy object_access_hook around their permission checks, to minimize maintain efforts. +1. -- 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_upgrade if 'postgres' database is dropped
On Tue, Nov 1, 2011 at 2:49 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: It turns out there was only one place that expected a 1-1 mapping of old and new databases (file transfer), so I just modified that code to allow skipping a database in the new cluster that didn't exist in the old cluster. Urp. ?But that means that if someone has any data in that database, pg_upgrade will basically eat it. ?That does not seem like a step forward. Please clarify? ?We already check that all the new cluster databases are empty, so we are effectively skipping the transfering of files into empty new cluster databases. ?It processes all old cluster databases and forces a new cluster match --- it is only empty new cluster database that are being skipped. Aren't you saying that if a postgres database exists in the old database (and potentially contains data) but is missing in the new database, we'll just fail to migrate it? No, the reverse. If the 'postgres' database exists in the new cluster, but not in the old, we allow it to upgrade (we skip over the 'postgres' database in the new cluster use the loop in the patch). Oh, OK. That seems fine - in fact, that seems perfect. Unless I am missing something. Did you see something odd in the patch or in my wording? Your wording confused me, but on further review I think I'm just easily confused. -- 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] unite recovery.conf and postgresql.conf
On 11/1/11 10:34 AM, Simon Riggs wrote: On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus j...@agliodbs.com wrote: So, we have four potential paths regarding recovery.conf: 1) Break backwards compatibility entirely, and stop supporting recovery.conf as a trigger file at all. Note that is exactly what I have suggested when using standby mode from pg_ctl. I wasn't clear on that from the description of your proposal. So are you suggesting that, if we start postgresql with pg_ctl standby then recovery.conf would not behave as a trigger file? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade if 'postgres' database is dropped
On Tue, Nov 1, 2011 at 1:53 PM, Bruce Momjian br...@momjian.us wrote: Bruce Momjian wrote: What I would prefer is to have the upgrade succeed, and just ignore the existence of a postgres database in the new cluster. Maybe give the user a notice and let them decide whether they wish to take any action. I understand that failing is probably less code, but IMHO one of the biggest problems with pg_upgrade is that it's too fragile: there are too many seemingly innocent things that can make it croak (which isn't good, when you consider that anyone using pg_upgrade is probably in a hurry to get the upgrade done and the database back on-line). It seems like this is an opportunity to get rid of one of those unnecessary failure cases. OK, then the simplest fix, once you modify pg_dumpall, would be to modify pg_upgrade to remove reference to the postgres database in the new cluster if it doesn't exist in the old one. That would allow pg_upgrade to maintain a 1-1 matching of databases in the old and new cluster --- it allows the change to be locallized without affecting much code. I fixed this a different way. I originally thought I could skip over the 'postgres' database in the new cluster if it didn't exist in the old cluster, but we have do things like check it is empty, so that was going to be awkward. It turns out there was only one place that expected a 1-1 mapping of old and new databases (file transfer), so I just modified that code to allow skipping a database in the new cluster that didn't exist in the old cluster. Urp. But that means that if someone has any data in that database, pg_upgrade will basically eat it. That does not seem like a step forward. -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
2011/11/1 Eric Ridge eeb...@gmail.com: On Tue, Nov 1, 2011 at 12:24 PM, Pavel Stehule pavel.steh...@gmail.com wrote: some other idea - but only for psql we can define a special values, that ensure a some necessary preexecution alchemy with entered query \pset star_exclude_names col1, col2, col3 \pset star_exclude_types xml, bytea, text(unlimited) Sure, something like that could be useful too. It might be confusing to users if they forget that they set an exclusion list, but there's probably ways to work around that. However, the nice thing about the feature being in SQL is that you can use it from all clients, and even in other useful ways. COPY would be an example (something I also do frequently): COPY (SELECT * EXCLUDING (a, b, c) FROM big query) TO 'somefile.csv' WITH CSV; Right now, if you want to exclude a column, you have to list all the others out manually, or just dump everything and deal with it in an external tool. sorry, I don't accept it. I am able to understand your request for adhoc queries. But not for COPY. and if you need it - you can write C function. I generally agree with everyone that says using this in application code is a bad idea, but I don't think that's reason alone to reject the idea on its face. I can accept a PostgreSQL extensions if there are no other way how do it effective. But it is not this case. eric -- 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] Compiler branch prediction hints (was: So, is COUNT(*) fast now?)
On Tue, Nov 01, 2011 at 10:55:02AM -0400, Robert Haas wrote: Well, the obvious problem is that we might end up spending a lot of work on something that doesn't actually improve performance, or even makes it worse, if our guesses about what's likely and unlikely turn out to be wrong. If we can show cases where it reliably produces a significant speedup, then I would think it would be worthwhile, but I think we should probably start by looking at what's slow and see how it can best be made faster rather than by looking specifically for places to use likely() and unlikely(). The biggest problem here is that (usually) the time is going in branches other than the ones you're thinking of. Something that might be interesting to try profile branch feedback in GCC. If you let the profiler see how often each branch was taken and then feed this back into the compiler you get in principle the best case result. This would represent the possible best case scenario. If you can prove a measurable difference then you know it's worth pursuing. If there's no result, then you know that too... Mind you, getting profile data with postgresql's multiprocess architechture can be challenge. Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ He who writes carelessly confesses thereby at the very outset that he does not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander mag...@hagander.net wrote: On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote: On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. Not really. You could just store it once in shared memory, and put the complexity in the view definition. I understood the proposal to be store the previous query in addition to the current-query-if-any. If that's not what was meant, then my objection was incorrect. However, like you, I'm pretty dubious of having two mostly-redundant fields in the view definition, just because of window width issues. The biggest reason I dislike the multi-field approach is because it limits us to only the [single] previous_query in the system with all the overhead we talked about (memory, window width and messing with system catalogs in general). That's actually why I implemented it the way I did, just by appending the last query on the end of the string when it's IDLE in transaction. Well, by appending it in that field, you require the end user/monitoring app to parse out the string anyway, so you're not exactly making life easier on the consumer of the information.. Marti wrote: I'd very much like to see a more generic solution: a runtime query log facility that can be queried in any way you want. pg_stat_statements comes close, but is limited too due to its (arbitrary, I find) deduplication -- you can't query for 10 last statements from process N since it has no notion of processes, just users and databases. This is what I'd really like to see (just haven't had time as it is a much bigger project). The next question my devs ask is what were the last five queries that ran... can you show me an overview of an entire transaction etc... That being said, having the previous_query available feels like it fixes about 80% of the *problem*; transaction profiling, or looking back 10 / 15 / 20 queries is [immensely] useful, but I find that the bigger need is the ability to short-circuit dba / dev back-n-forth by just saying Your app refused to commit/rollback after running query XYZ. This would be great, but as you say, it's a different project. Perhaps something could be made along the line of each backend keeping it's *own* set of old queries, and then making it available to a specific function (pg_get_last_queries_for_backend(nnn)) Yeah, I was kind of thinking this too, I just feel dirty adding a (*n * track_activity_query_size) *overhead to shared memory for tracking that if we're already concerned about adding just a 'previous_query' string. It's easy enough to either hard-code or set a limit on 'n', but, if I were to do that, is it something that would be accepted? (my ability to code not-withstanding :-) - since this is not the type of information you'd ask for often, it would be ok if getting this data took longer.. And you seldom want give me the last 5 queries for each backend at once. thinking-aloud I'm more concerned with the overhead of managing that array every single time that a backend updates its status than I am on retrieval. I guess if the array were small enough and the the logic clean, it could end up having about the same overhead as what I'm doing now (obviously, I'm still gobbling more memory). /thinking-aloud Doing that *would* allow us to get away from concerns about breaking monitoring tools and the like. -- Scott Mead OpenSCG http://www.openscg.com Robert Wrote: Yeah. Otherwise, people who are parsing the hard-coded strings idle and idle in transaction are likely to get confused. I would be interested ( and frankly very surprised ) to find out if many monitoring tools are actually parsing that field. Most that I see just dump whatever is in current_query to the user. I would imaging that, so long as the server obeyed pgstat_track_activity_size most tools would behave nicely. Really? I'd assume every single monitoring tool that graphs how many active connections you have (vs idle vs idle in transaction) would do just this. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 18:40, Scott Mead sco...@openscg.com wrote: On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander mag...@hagander.net wrote: On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote: On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. Not really. You could just store it once in shared memory, and put the complexity in the view definition. I understood the proposal to be store the previous query in addition to the current-query-if-any. If that's not what was meant, then my objection was incorrect. However, like you, I'm pretty dubious of having two mostly-redundant fields in the view definition, just because of window width issues. The biggest reason I dislike the multi-field approach is because it limits us to only the [single] previous_query in the system with all the overhead we talked about (memory, window width and messing with system catalogs in general). That's actually why I implemented it the way I did, just by appending the last query on the end of the string when it's IDLE in transaction. Well, by appending it in that field, you require the end user/monitoring app to parse out the string anyway, so you're not exactly making life easier on the consumer of the information.. Marti wrote: I'd very much like to see a more generic solution: a runtime query log facility that can be queried in any way you want. pg_stat_statements comes close, but is limited too due to its (arbitrary, I find) deduplication -- you can't query for 10 last statements from process N since it has no notion of processes, just users and databases. This is what I'd really like to see (just haven't had time as it is a much bigger project). The next question my devs ask is what were the last five queries that ran... can you show me an overview of an entire transaction etc... That being said, having the previous_query available feels like it fixes about 80% of the *problem*; transaction profiling, or looking back 10 / 15 / 20 queries is [immensely] useful, but I find that the bigger need is the ability to short-circuit dba / dev back-n-forth by just saying Your app refused to commit/rollback after running query XYZ. This would be great, but as you say, it's a different project. Perhaps something could be made along the line of each backend keeping it's *own* set of old queries, and then making it available to a specific function (pg_get_last_queries_for_backend(nnn)) Yeah, I was kind of thinking this too, I just feel dirty adding a (n * track_activity_query_size) overhead to shared memory for tracking that if we're already concerned about adding just a 'previous_query' string. It's easy enough to either hard-code or set a limit on 'n', but, if I were to do that, is it something that would be accepted? (my ability to code not-withstanding :-) No, I meant storing it in backend local memory, and then transfer it upon request. That would remove locking requirements etc. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 2:34 PM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Nov 1, 2011 at 6:12 PM, Robert Treat r...@xzilla.net wrote: On Tue, Nov 1, 2011 at 1:34 PM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus j...@agliodbs.com wrote: So, we have four potential paths regarding recovery.conf: 1) Break backwards compatibility entirely, and stop supporting recovery.conf as a trigger file at all. Note that is exactly what I have suggested when using standby mode from pg_ctl. But you already know that, since you said If it's possible to run a replica without having a recovery.conf file, then I'm fine with your solution, and I already confirmed back to you that would be possible. It's possible to run a replica without having a recovery.conf file is not the same thing as If someone makes a recovery.conf file, it won't break my operations. AIUI, you are not supporting the latter. Yes, that is part of the combined proposal, which allows both old and new APIs. New API pg_ctl standby will startup a server in standby mode, do not implicitly include recovery.conf and disallow recovery_target parameters in postgresql.conf (you may, if you wish, explicitly have include recovery.conf in your postgresql.conf, should you desire that) Old API pg_ctl start and a recovery.conf has been created will startup a server in PITR and/or replication, recovery.conf will be read automatically (as now) so the presence of the recovery.conf acts as a trigger, only if we issue start So the existing syntax works exactly as now, but a new API has been created to simplify things in exactly the way requested. The old and the new API don't mix, so there is no confusion between them. You must still use the old API when you wish to perform a PITR, as explained by me, following comments by Peter. Ah, thanks for clarifying, your earlier proposal didn't read that way to me. It still doesn't solve the problem for tool makers of needing to be able to deal with two possible implementation methods, but it should be easier for them to make a choice. One thing though, I think it would be better to have this work the other way around. ISTM we're going to end up telling people to avoid using pg_ctl start and instead use pg_ctl standby, which doesn't feel like the right answer. Ie. Starting in 9.2, you should use pg_ctl standby to launch your database for normal operations and/or in cases where you are writing init scripts to control your production databases. For backwards compatibility, if you require the old behavior of using a recovery.conf, we would recommend you use pg_ctl start instead. Robert Treat conjecture: xzilla.net consulting: omniti.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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On Tue, Nov 1, 2011 at 12:03 PM, Stephen Frost sfr...@snowman.net wrote: Note- I haven't looked at the * production or tried to do anything w/ gram.y to support this yet, but it's a heck of a lot shorter.. My original thought, that I probably didn't explain too clearly, was to make the EXCLUDING (...) bit a modifier to the A_Star node. The idea being that you could write * EXCLUDING (...) anywhere you can currently write *. It's dead simple for the case of: SELECT * FROM ... but because of the indirection productions in gram.y, it's literally impossible for: SELECT tablename.* FROM ... without possibly breaking existing queries. Nonetheless, even if it were only available for the first form, it would be very useful. For the ad-hoc type stuff I do, it'd still be great to write: SELECT * EXCLUDING (x.a, x.b, x.c) FROM (SELECT x); I've already got gram.y working the way it needs to, and I've started to get the exclusion list passed into the places it needs to go. If y'all would be willing to accept it in this limited form, I'll continue to work on it. eric -- 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] Compiler branch prediction hints (was: So, is COUNT(*) fast now?)
On Tue, Nov 1, 2011 at 9:55 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 1, 2011 at 10:47 AM, Marti Raudsepp ma...@juffo.org wrote: On Fri, Oct 28, 2011 at 20:58, Robert Haas robertmh...@gmail.com wrote: I tried sprinkling a little branch-prediction magic on this code using GCC's __builtin_expect(). That initially seemed to help, but once I changed the BufferIsValid() test to !BufferIsInvalid() essentially all of the savings disappeared. Sounds like mere chance that the compiler decided to lay it out in one way or another. A different compiler version could pick a different path. I quite like the likely() and unlikely() macros used in Linux kernel; much more readable than __builtin_expect and they might also be useful for (usually redundant) error checks and asserts in hot code paths. It looks like this: #ifdef __GNUC__ # define unlikely(xpr) __builtin_expect(xpr, 0) #else # define unlikely(xpr) (xpr) #endif if (unlikely(blkno = rel-rd_smgr-smgr_vm_nblocks)) { /* unlikely branch here */ } However, the wins are probably minor because most of the time, adaptive CPU branch prediction will override that. Do you think this would be a useful patch to attempt? Well, the obvious problem is that we might end up spending a lot of work on something that doesn't actually improve performance, or even makes it worse, if our guesses about what's likely and unlikely turn out to be wrong. If we can show cases where it reliably produces a significant speedup, then I would think it would be worthwhile, but I think we should probably start by looking at what's slow and see how it can best be made faster rather than by looking specifically for places to use likely() and unlikely(). The first thing that jumps to mind is the hint bit checks in HeapTupleSatisifiesMVCC(). merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander mag...@hagander.net wrote: On Tue, Nov 1, 2011 at 18:11, Scott Mead sco...@openscg.com wrote: On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. Not really. You could just store it once in shared memory, and put the complexity in the view definition. I understood the proposal to be store the previous query in addition to the current-query-if-any. If that's not what was meant, then my objection was incorrect. However, like you, I'm pretty dubious of having two mostly-redundant fields in the view definition, just because of window width issues. The biggest reason I dislike the multi-field approach is because it limits us to only the [single] previous_query in the system with all the overhead we talked about (memory, window width and messing with system catalogs in general). That's actually why I implemented it the way I did, just by appending the last query on the end of the string when it's IDLE in transaction. Well, by appending it in that field, you require the end user/monitoring app to parse out the string anyway, so you're not exactly making life easier on the consumer of the information.. +1 Marti wrote: I'd very much like to see a more generic solution: a runtime query log facility that can be queried in any way you want. pg_stat_statements comes close, but is limited too due to its (arbitrary, I find) deduplication -- you can't query for 10 last statements from process N since it has no notion of processes, just users and databases. This is what I'd really like to see (just haven't had time as it is a much bigger project). The next question my devs ask is what were the last five queries that ran... can you show me an overview of an entire transaction etc... That being said, having the previous_query available feels like it fixes about 80% of the *problem*; transaction profiling, or looking back 10 / 15 / 20 queries is [immensely] useful, but I find that the bigger need is the ability to short-circuit dba / dev back-n-forth by just saying Your app refused to commit/rollback after running query XYZ. This would be great, but as you say, it's a different project. +1 (but I'd like to see that different project) Perhaps something could be made along the line of each backend keeping it's *own* set of old queries, and then making it available to a specific function (pg_get_last_queries_for_backend(nnn)) - since this is not the type of information you'd ask for often, it would be ok if getting this data took longer.. And you seldom want give me the last 5 queries for each backend at once. Robert Wrote: Yeah. Otherwise, people who are parsing the hard-coded strings idle and idle in transaction are likely to get confused. I would be interested ( and frankly very surprised ) to find out if many monitoring tools are actually parsing that field. Most that I see just dump whatever is in current_query to the user. I would imaging that, so long as the server obeyed pgstat_track_activity_size most tools would behave nicely. Really? I'd assume every single monitoring tool that graphs how many active connections you have (vs idle vs idle in transaction) would do just this. Having written and/or patched dozens of these types of things, your spot on; all of the ones with anything other than the most brain dead of monitoring parse for IDLE and IDLE in transaction. That said, I'm happy to see the {active|idle|idle in txn} status field and query_string fields show up and break backwards compatibility. Updating the tools will be simple for those who need it, and make a view to work around it will be simple for those who don't. Happy to add an example view definition to the docs if it will help. Robert Treat conjecture: xzilla.net consulting: omniti.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] IDLE in transaction introspection
On 11/01/2011 09:52 AM, Tom Lane wrote: Simon Riggssi...@2ndquadrant.com writes: Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. +1 I could go either way on whether to rename it. Rename it please. current_query will just be wrong. I'd be inclined just to call it query or query_string and leave it to the docs to define the exact semantics. 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On Mon, Oct 31, 2011 at 09:14:48AM -0400, Andrew Dunstan wrote: The fact is that if you have 100 columns and want 95 of them, it's very tedious to have to specify them all, especially for ad hoc queries where the house SQL standards really don't matter that much. It's made more tedious by the fact that there is no real help in constructing the query. This gets particularly bad with views, which developers often seem to stuff with every available column that might be needed by some query instead of creating views tailored to particular queries. Not long ago annoyance with this prompted my to write a little utility function that would give me a query with all the columns specified so I could cut and paste it, and delete the columns I didn't want. (Another advantage is that the result is guaranteed typo free, which my typing certainly is not.) See https://gist.github.com/818490. It's far from perfect, but I still find myself using it several times a month, mainly for the very purpose intended by this suggested feature. As I do the ad hoc query thing more than I'd like to admit, I think there's a place for some form of negation for *. A workaround similar to what you describe here would be to add special tab completion to psql that would expand * to the full list (probably on double tab ...) Ross -- Ross Reedstrom, Ph.D. reeds...@rice.edu Systems Engineer Admin, Research Scientistphone: 713-348-6166 Connexions http://cnx.orgfax: 713-348-3665 Rice University MS-375, Houston, TX 77005 GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
Eric Ridge eeb...@gmail.com writes: My original thought, that I probably didn't explain too clearly, was to make the EXCLUDING (...) bit a modifier to the A_Star node. The idea being that you could write * EXCLUDING (...) anywhere you can currently write *. I can think of a number of places where you can write * where I'm pretty sure we *don't* want this. It should be restricted to top-level entries in SELECT targetlists, IMO. 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On Nov 1, 2011, at 6:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: I can think of a number of places where you can write * where I'm pretty sure we *don't* want this. It should be restricted to top-level entries in SELECT targetlists, IMO. Yes. That is the exact conclusion I've come to. However, why is select table.* foo from table allowed? What does that even mean? eric -- 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 1:34 PM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus j...@agliodbs.com wrote: So, we have four potential paths regarding recovery.conf: 1) Break backwards compatibility entirely, and stop supporting recovery.conf as a trigger file at all. Note that is exactly what I have suggested when using standby mode from pg_ctl. But you already know that, since you said If it's possible to run a replica without having a recovery.conf file, then I'm fine with your solution, and I already confirmed back to you that would be possible. It's possible to run a replica without having a recovery.conf file is not the same thing as If someone makes a recovery.conf file, it won't break my operations. AIUI, you are not supporting the latter. Robert Treat conjecture: xzilla.net consulting: omniti.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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus j...@agliodbs.com wrote: So, we have four potential paths regarding recovery.conf: 1) Break backwards compatibility entirely, and stop supporting recovery.conf as a trigger file at all. Note that is exactly what I have suggested when using standby mode from pg_ctl. But you already know that, since you said If it's possible to run a replica without having a recovery.conf file, then I'm fine with your solution, and I already confirmed back to you that would be possible. 2) Offer backwards compatibility only if recovery_conf='filename' is set in postgresql.conf, then behave like Simon's compromise. 3) Simon's compromise. See above. Calling it a compromise in this way implies nobody has been given exactly what they ask for, but that is not the case. 4) Don't ever change how recovery.conf works. The only two of the above I see as being real options are (1) and (2). (3) would, as Robert points out, cause DBAs to have unpleasant surprises when some third-party tool creates a recovery.conf they weren't expecting. So: Please read my proposal again. I'll be happy to answer questions if you have any. -- 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] IDLE in transaction introspection
On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. Not really. You could just store it once in shared memory, and put the complexity in the view definition. I understood the proposal to be store the previous query in addition to the current-query-if-any. If that's not what was meant, then my objection was incorrect. However, like you, I'm pretty dubious of having two mostly-redundant fields in the view definition, just because of window width issues. The biggest reason I dislike the multi-field approach is because it limits us to only the [single] previous_query in the system with all the overhead we talked about (memory, window width and messing with system catalogs in general). That's actually why I implemented it the way I did, just by appending the last query on the end of the string when it's IDLE in transaction. Marti wrote: I'd very much like to see a more generic solution: a runtime query log facility that can be queried in any way you want. pg_stat_statements comes close, but is limited too due to its (arbitrary, I find) deduplication -- you can't query for 10 last statements from process N since it has no notion of processes, just users and databases. This is what I'd really like to see (just haven't had time as it is a much bigger project). The next question my devs ask is what were the last five queries that ran... can you show me an overview of an entire transaction etc... That being said, having the previous_query available feels like it fixes about 80% of the *problem*; transaction profiling, or looking back 10 / 15 / 20 queries is [immensely] useful, but I find that the bigger need is the ability to short-circuit dba / dev back-n-forth by just saying Your app refused to commit/rollback after running query XYZ. Robert Wrote: Yeah. Otherwise, people who are parsing the hard-coded strings idle and idle in transaction are likely to get confused. I would be interested ( and frankly very surprised ) to find out if many monitoring tools are actually parsing that field. Most that I see just dump whatever is in current_query to the user. I would imaging that, so long as the server obeyed pgstat_track_activity_size most tools would behave nicely. Now... all that being said, I've implemented the 'previous_query' column and (maybe just for my own benefit), is the PostgreSQL community interested in the patch? -- Scott Mead OpenSCG http://www.openscg.com regards, tom lane
Re: [HACKERS] IDLE in transaction introspection
On Tue, Nov 01, 2011 at 10:13:52AM -0400, Andrew Dunstan wrote: On 11/01/2011 09:52 AM, Tom Lane wrote: Simon Riggssi...@2ndquadrant.com writes: Why not leave it exactly as it is, and add a previous_query column? That gives you exactly what you need without breaking anything. That would cost twice as much shared memory for query strings, and twice as much time to update the strings, for what seems pretty marginal value. I'm for just redefining the query field as current or last query. +1 I could go either way on whether to rename it. Rename it please. current_query will just be wrong. I'd be inclined just to call it query or query_string and leave it to the docs to define the exact semantics. +1 on providing the most-recent-query info +1 on the state/query split as a means +1 rename from current_query (i.e. break it) personalbikeshedcolor: query_string Personal opinion on backwards compatability matches Robert's: things that affect admin functionality are less of an issue than those that impact user (i.e. coder) SQL. And definitely break it: I may chose to fix it by bodging in a view for the old behavior myself, but that's my choice. Perhaps provide an example view def in change notes if you really want to. For myself, when making fixes to monitor scripts for this type of change, my reaction is probably Yes, finally, I'm not regexing on the d*mn query string anymore! Ross P.S. though apparently it doesn't bother me enough to create and submit a patch myself. :-( -- Ross Reedstrom, Ph.D. reeds...@rice.edu Systems Engineer Admin, Research Scientistphone: 713-348-6166 Connexions http://cnx.orgfax: 713-348-3665 Rice University MS-375, Houston, TX 77005 GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
2011/11/1 Eric Ridge eeb...@gmail.com: On Tue, Nov 1, 2011 at 1:33 PM, Pavel Stehule pavel.steh...@gmail.com wrote: COPY (SELECT * EXCLUDING (a, b, c) FROM big query) TO 'somefile.csv' WITH CSV; sorry, I don't accept it. I am able to understand your request for adhoc queries. But not for COPY. I apologize if that example was confusing. I wasn't also suggesting expanding COPY's syntax. I was merely pointing out that if EXCLUDING(…) were implemented, you'd be able to use it within the query given to the COPY command. I understand it, it is really bad idea use a star in export queries Pavel eric ps, it seems my messages aren't hitting the list? weird. -- 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] Object access hooks with arguments support (v1)
2011/11/1 Robert Haas robertmh...@gmail.com: On Tue, Nov 1, 2011 at 1:32 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: I tried to summarize permission checks of DAC/MAC on several object classes that are allowed to assign security label right now. http://wiki.postgresql.org/index.php?title=SEPostgreSQL/Permissions In most of checks, required contextual information by SELinux are commonly used to DAC also, as listed. What's up with this: a flag to inform whether CASCADE or RESTRICT That doesn't seem like it should be needed. Ah, it is needed to determine the scope of objects to be deleted. If a table being referenced by views, deletion of this table with cascade involves the views, even if these are owned by others. DAC does not check permissions to drop on depending objects, so, it is a headache for me. However, the worth of drop permission checks on involved objects is not perfectly clear, because the user is allowed to drop the table being reference by views at least in above example. If so, views to reference non-existent table is nonsense. I'm not 100% sure why existing DAC does not check permissions on depending objects even if these are owned by other users. I'd like to know the reason of this behavior. We should consider whether CREATE TABLE should be considered to consist of creating a table and then n attributes, rather than trying to shove the attribute information wholesale into the create table check. I don't see tangible difference between them except for simpleness of implementation. One point we should consider is the timing to store security label of table and columns. If creation checks of table/columns are consolidated into one hook, we can check permissions to create them and write-back default security labels of them into one private datum to be delivered to post-creation hook. If creation checks of table/columns are separated into individual invocation, the later column-checks needs security label of the new table as contextual information, however, it requires us to know internals of sepgsql why it needs table's label to check permission to create a column, because both of checks shall be done DefineRelation / OpenIntoRel prior to heap_create_with_catalog that invokes post-creation hook. So, I think it works better with the approach that also delivers TupleDesc on creation of relation checks, rather than separated invocations. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On Tue, Nov 1, 2011 at 12:24 PM, Pavel Stehule pavel.steh...@gmail.com wrote: some other idea - but only for psql we can define a special values, that ensure a some necessary preexecution alchemy with entered query \pset star_exclude_names col1, col2, col3 \pset star_exclude_types xml, bytea, text(unlimited) Sure, something like that could be useful too. It might be confusing to users if they forget that they set an exclusion list, but there's probably ways to work around that. However, the nice thing about the feature being in SQL is that you can use it from all clients, and even in other useful ways. COPY would be an example (something I also do frequently): COPY (SELECT * EXCLUDING (a, b, c) FROM big query) TO 'somefile.csv' WITH CSV; Right now, if you want to exclude a column, you have to list all the others out manually, or just dump everything and deal with it in an external tool. I generally agree with everyone that says using this in application code is a bad idea, but I don't think that's reason alone to reject the idea on its face. eric -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] removing =(text, text) in 9.2
On Thu, Oct 6, 2011 at 12:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: +1. However, if that's the route we're traveling down, I think we had better go ahead and remove the one remaining = operator from hstore in 9.2: Fair enough. So, I tried to work up a patch for this, but I'm actually a bit confused about what needs to be done here. I'll attach what I've got so far as a starting point for discussion. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company hstore-drop-arrow.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
[HACKERS] Testing for safe fetching of TOAST values
I'm working on fixing the stale-toast-pointer problem discussed in http://archives.postgresql.org/message-id/2348.1319591...@sss.pgh.pa.us In that thread, it was pointed out that it's unsafe to fetch a toasted value unless we are advertising a MyProc-xmin that's old enough to prevent removal of the tuple holding the toast pointer. Otherwise, someone could commit a deletion of that tuple and its subsidiary toast tuples after we decide it's good, and then VACUUM could remove the toast tuples before we finish fetching the toast tuples. I tried to add an Assert that at least checks that we're advertising *some* xmin at the start of toast_fetch_datum(): Assert(TransactionIdIsNormal(MyProc-xmin)); The regression tests run through without complaint with this in place, but initdb blows up while processing system_views.sql. I found that the problem is that exec_simple_query() issues a CommandCounterIncrement between queries while not holding any snapshot. (exec_simple_query itself doesn't take one because the queries are all utility commands, ie CREATE VIEW. PortalRunUtility does take a snapshot while running CREATE VIEW, but then drops it again.) This causes a relcache flush and reload for the just-created view (since it was just created, RelationFlushRelation tries to rebuild not just drop the relcache entry). If the view's rule is long enough to be toasted, boom! Now, in fact this access pattern is perfectly safe, because if the view was created in the current transaction then there's no way for VACUUM to remove the tuples describing it. The whole thing seems rather delicate, and yet I can't put my finger on anyplace that's doing something wrong. I'm going to commit the patch without this Assert, but I wonder if anyone has ideas about either a better test for dangerous fetches, or a way to rejigger the code to make this test work. 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] Thoughts on SELECT * EXCLUDING (...) FROM ...?
On Tue, Nov 1, 2011 at 1:33 PM, Pavel Stehule pavel.steh...@gmail.com wrote: COPY (SELECT * EXCLUDING (a, b, c) FROM big query) TO 'somefile.csv' WITH CSV; sorry, I don't accept it. I am able to understand your request for adhoc queries. But not for COPY. I apologize if that example was confusing. I wasn't also suggesting expanding COPY's syntax. I was merely pointing out that if EXCLUDING(…) were implemented, you'd be able to use it within the query given to the COPY command. eric ps, it seems my messages aren't hitting the list? weird. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Is there a good reason we don't have INTERVAL 'infinity'?
Hackers, Is there a reason why INTERVAL 'infinity' is not implemented? That is, an interval which is larger than all defined intervals, and which added to any timestamp turns it into 'infinity'. Or is it just Round TUITs? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] unite recovery.conf and postgresql.conf
On 11/1/11 12:29 PM, Robert Treat wrote: Starting in 9.2, you should use pg_ctl standby to launch your database for normal operations and/or in cases where you are writing init scripts to control your production databases. For backwards compatibility, if you require the old behavior of using a recovery.conf, we would recommend you use pg_ctl start instead. Gah. There is no way we're getting distro packagers to switch from pg_ctl start. Also, a lot of distros use the postgres command rather than pg_ctl anything. Messing with pg_ctl is not really a solution for this, since few people in production environments call it directly. Nobody I know, anyway. So Simon's suggested compromise still puts backwards compatibility ahead of promoting the new API. This would result in nobody supporting the new API until the day we remove the old one from the code. I think adding recovery_conf_location = '' to postgresql.conf is a much better compromise. Assuming we can stand the code complexity ... -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] OK
Re: [HACKERS] Thoughts on SELECT * EXCLUDING (...) FROM ...?
* Marti Raudsepp (ma...@juffo.org) wrote: Unfortunately it's far less efficient. Fields would be truncated in psql, so full values are still detoasted and transmitted over the network. I'm thinking that we're not too worried about performance of ad-hoc psql queries..? At least, for the queries that I'd use this for, I wouldn't be worried about that. The various syntax proposals do seem overly long for this, however.. I was just wondering about something like: select ~* blah, blah, blah FROM ... Strikes me as pretty unlikely that making a new 'version' of * like this is going to break anything or be broken by the SQL standard. Note- I haven't looked at the * production or tried to do anything w/ gram.y to support this yet, but it's a heck of a lot shorter.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Separating bgwriter and checkpointer
On Wed, Oct 19, 2011 at 3:58 PM, Simon Riggs si...@2ndquadrant.com wrote: On Wed, Oct 19, 2011 at 3:29 PM, Fujii Masao masao.fu...@gmail.com wrote: On Wed, Oct 19, 2011 at 9:45 PM, Robert Haas robertmh...@gmail.com wrote: I don't really see any reason to break the monitoring view just because we did some internal refactoring. I'd rather have backward compatibility. Fair enough. The patch doesn't change any document, but at least the description of pg_stat_bgwriter seems to need to be changed. Thanks for the review. Will follow up on suggestions. I'll add this in as a separate item after commit. -- 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] unite recovery.conf and postgresql.conf
On Tue, Nov 1, 2011 at 9:11 PM, Josh Berkus j...@agliodbs.com wrote: On 11/1/11 12:29 PM, Robert Treat wrote: Starting in 9.2, you should use pg_ctl standby to launch your database for normal operations and/or in cases where you are writing init scripts to control your production databases. For backwards compatibility, if you require the old behavior of using a recovery.conf, we would recommend you use pg_ctl start instead. Gah. There is no way we're getting distro packagers to switch from pg_ctl start. Also, a lot of distros use the postgres command rather than pg_ctl anything. Messing with pg_ctl is not really a solution for this, since few people in production environments call it directly. Nobody I know, anyway. So Simon's suggested compromise still puts backwards compatibility ahead of promoting the new API. This would result in nobody supporting the new API until the day we remove the old one from the code. Which will never happen, since part of the proposal is that PITR will only be supported using the old method. I think adding recovery_conf_location = '' to postgresql.conf is a much better compromise. Assuming we can stand the code complexity ... I think that might have some possibilities. But how does that work in detail? If you set it to empty, then the recovery_* parameters are just GUCs, I suppose: which seems fine. But if you set it to a non-empty value then what happens, exactly? The recovery.conf settings clobber anything in postgresql.conf, and when we exit recovery we reload the config, discarding any settings we got from recovery.conf? That might not be too bad. I think we need to back up and figure out what problem we're trying to solve here. IMV, the answer is setting up a standby is too complicated and requiring yet another configuration file to do it makes it even more complicated. If the mechanism we introduce to solve that problem is more complicated than what we have now, it might end up being a net regression in terms of usability. I feel like changing everything that's currently in recovery.conf to GUCs and implementing SET PERSISTENT would give everyone what they need, admittedly without perfect backward compatibility, but perhaps close enough for government work, and a step forward overall. -- 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] Hot Backup with rsync fails at pg_clog if under load
looks like the v3 patch re-introduces the pg_subtrans issue... On Tue, Nov 1, 2011 at 9:33 AM, Simon Riggs si...@2ndquadrant.com wrote: On Thu, Oct 27, 2011 at 4:25 PM, Simon Riggs si...@2ndquadrant.com wrote: StartupMultiXact() didn't need changing, I thought, but I will review further. Good suggestion. On review, StartupMultiXact() could also suffer similar error to the clog failure. This was caused *because* MultiXact is not maintained by recovery, which I had thought meant it was protected from such failure. Revised patch attached. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Thoughts on SELECT * EXCLUDING (...) FROM ...?
Eric B. Ridge eeb...@gmail.com writes: However, why is select table.* foo from table allowed? What does that even mean? Doesn't mean anything, I think --- the SQL standard seems to exclude it. It's fairly hard to prevent it at the grammar level, since we regard foo.* as a type of primitive expression, but I suspect it might be a good idea for transformTargetList to throw an error instead of silently ignoring the column label. 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] removing =(text, text) in 9.2
On Nov 1, 2011, at 11:19 AM, Robert Haas wrote: Fair enough. So, I tried to work up a patch for this, but I'm actually a bit confused about what needs to be done here. I'll attach what I've got so far as a starting point for discussion. Looks reasonable, if verbose. (Yes, the extension design currently requires that). What about doc updates? Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Is there a good reason we don't have INTERVAL 'infinity'?
Josh Berkus wrote: Hackers, Is there a reason why INTERVAL 'infinity' is not implemented? That is, an interval which is larger than all defined intervals, and which added to any timestamp turns it into 'infinity'. Or is it just Round TUITs? Probably the latter. There is even a function |isfinite(interval)| which doesn't seem to do anything useful. See complaint in http://archives.postgresql.org/message-id/200101241913.f0ojduu45...@hub.org Although the operation used in this complaint isn't obviously defined there certainly are operations that are defined like infinity + infinity = infinity. See http://de.wikipedia.org/wiki/Unendlichkeit#Analysis (Sorry for linking the german wikipedia - the english text is ways less verbose on this.) Regards, Brar