Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-11-01 Thread Fujii Masao
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

2011-11-01 Thread Jan Lentfer
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 ...?

2011-11-01 Thread Marti Raudsepp
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 ...?

2011-11-01 Thread Brendan Jurd
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

2011-11-01 Thread Magnus Hagander
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Magnus Hagander
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

2011-11-01 Thread Simon Riggs
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 ...?

2011-11-01 Thread Merlin Moncure
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Tom Lane
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

2011-11-01 Thread Tom Lane
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Marti Raudsepp
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Jeroen Vermeulen

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

2011-11-01 Thread Tom Lane
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

2011-11-01 Thread Magnus Hagander
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?)

2011-11-01 Thread Marti Raudsepp
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?)

2011-11-01 Thread Robert Haas
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-01 Thread Cédric Villemain
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

2011-11-01 Thread Simon Riggs
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-01 Thread Pavel Stehule
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

2011-11-01 Thread Dimitri Fontaine
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-11-01 Thread Kohei KaiGai
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

2011-11-01 Thread Bruce Momjian
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

2011-11-01 Thread Bruce Momjian
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

2011-11-01 Thread Bruce Momjian
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

2011-11-01 Thread Bruce Momjian
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Joshua Berkus
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

2011-11-01 Thread Magnus Hagander
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)

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Josh Berkus
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

2011-11-01 Thread Robert Haas
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-01 Thread Pavel Stehule
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?)

2011-11-01 Thread Martijn van Oosterhout
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

2011-11-01 Thread Scott Mead
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

2011-11-01 Thread Magnus Hagander
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

2011-11-01 Thread Robert Treat
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 ...?

2011-11-01 Thread Eric Ridge
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?)

2011-11-01 Thread Merlin Moncure
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

2011-11-01 Thread Robert Treat
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

2011-11-01 Thread Andrew Dunstan



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 ...?

2011-11-01 Thread Ross J. Reedstrom
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 ...?

2011-11-01 Thread Tom Lane
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 ...?

2011-11-01 Thread Eric B. Ridge
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

2011-11-01 Thread Robert Treat
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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Scott Mead
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

2011-11-01 Thread Ross J. Reedstrom
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-01 Thread Pavel Stehule
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-01 Thread Kohei KaiGai
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 ...?

2011-11-01 Thread Eric Ridge
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Tom Lane
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 ...?

2011-11-01 Thread Eric Ridge
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'?

2011-11-01 Thread Josh Berkus
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

2011-11-01 Thread Josh Berkus
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

2011-11-01 Thread Alcione Benacchio



Re: [HACKERS] Thoughts on SELECT * EXCLUDING (...) FROM ...?

2011-11-01 Thread Stephen Frost
* 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

2011-11-01 Thread Simon Riggs
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

2011-11-01 Thread Robert Haas
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

2011-11-01 Thread Chris Redekop
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 ...?

2011-11-01 Thread Tom Lane
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

2011-11-01 Thread David E. Wheeler
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'?

2011-11-01 Thread Brar Piening


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