Re: [HACKERS] psql expanded auto

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

[Marti Raudsepp]

On Mon, Oct 31, 2011 at 12:54, Marcin Mańk  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 ..."?

[Brendan Jurd]

On 1 November 2011 00:14, Andrew Dunstan  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

[Magnus Hagander]

On Tue, Nov 1, 2011 at 00:18, Scott Mead  wrote:
>
>
> On Mon, Oct 31, 2011 at 6:13 PM, Robert Haas  wrote:
>>
>> On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander 
>> 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='').
>  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 5:59 AM, Fujii Masao  wrote:
> On Mon, Oct 31, 2011 at 5:23 PM, Simon Riggs  wrote:
>> On Mon, Oct 31, 2011 at 7:38 AM, Fujii Masao  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

[Robert Haas]

On Tue, Nov 1, 2011 at 7:38 AM, Magnus Hagander  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 
and  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

[Robert Haas]

On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs  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

[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

[Simon Riggs]

On Tue, Nov 1, 2011 at 12:06 PM, Robert Haas  wrote:
> On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs  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

[Simon Riggs]

On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas  wrote:
> On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander  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

[Magnus Hagander]

On Tue, Nov 1, 2011 at 13:19, Simon Riggs  wrote:
> On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas  wrote:
>> On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 12:41 PM, Magnus Hagander  wrote:
> On Tue, Nov 1, 2011 at 13:19, Simon Riggs  wrote:
>> On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas  wrote:
>>> On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander  
>>> 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 ..."?

[Merlin Moncure]

On Sat, Oct 29, 2011 at 5:26 PM, Eric Ridge  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

[Robert Haas]

On Tue, Nov 1, 2011 at 8:14 AM, Simon Riggs  wrote:
> On Tue, Nov 1, 2011 at 12:06 PM, Robert Haas  wrote:
>> On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs  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

[Robert Haas]

On Tue, Nov 1, 2011 at 8:41 AM, Magnus Hagander  wrote:
> On Tue, Nov 1, 2011 at 13:19, Simon Riggs  wrote:
>> On Mon, Oct 31, 2011 at 10:13 PM, Robert Haas  wrote:
>>> On Mon, Oct 31, 2011 at 5:45 PM, Magnus Hagander  
>>> 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

[Tom Lane]

Simon Riggs  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

[Tom Lane]

Simon Riggs  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 1:52 PM, Tom Lane  wrote:
> Simon Riggs  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

[Robert Haas]

On Tue, Nov 1, 2011 at 9:45 AM, Simon Riggs  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

[Robert Haas]

On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane  wrote:
> Simon Riggs  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

[Marti Raudsepp]

On Mon, Oct 31, 2011 at 23:37, Scott Mead  wrote:
>    So I wrote the attached patch, it just turns  in transaction into:
>  " in transaction\n: Previous: ".  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 1:23 PM, Robert Haas  wrote:
> On Tue, Nov 1, 2011 at 8:14 AM, Simon Riggs  wrote:
>> On Tue, Nov 1, 2011 at 12:06 PM, Robert Haas  wrote:
>>> On Tue, Nov 1, 2011 at 7:46 AM, Simon Riggs  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 2:04 PM, Robert Haas  wrote:
> On Tue, Nov 1, 2011 at 9:45 AM, Simon Riggs  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 1:48 PM, Tom Lane  wrote:
> Simon Riggs  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

[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

[Tom Lane]

Robert Haas  writes:
> On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane  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

[Magnus Hagander]

On Mon, Oct 31, 2011 at 20:59, Magnus Hagander  wrote:
> On Mon, Oct 31, 2011 at 20:58, Tom Lane  wrote:
>> Magnus Hagander  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?)

[Marti Raudsepp]

On Fri, Oct 28, 2011 at 20:58, Robert Haas  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?)

[Robert Haas]

On Tue, Nov 1, 2011 at 10:47 AM, Marti Raudsepp  wrote:
> On Fri, Oct 28, 2011 at 20:58, Robert Haas  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

[Cédric Villemain]

2011/11/1 Marti Raudsepp :
> On Mon, Oct 31, 2011 at 23:37, Scott Mead  wrote:
>>    So I wrote the attached patch, it just turns  in transaction into:
>>  " in transaction\n: Previous: ".  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 6:36 PM, Josh Berkus  wrote:
> On 11/1/11 10:34 AM, Simon Riggs wrote:
>> On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus  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 ..."?

[Pavel Stehule]

2011/11/1 Eric Ridge :
> On Tue, Nov 1, 2011 at 12:24 PM, Pavel Stehule  
> 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 ) 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

[Dimitri Fontaine]

Peter Eisentraut  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)

[Kohei KaiGai]

2011/10/21 Robert Haas :
> On Fri, Oct 21, 2011 at 12:44 PM, Kohei KaiGai  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 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_upgrade if 'postgres' database is dropped

[Bruce Momjian]

Robert Haas wrote:
> On Tue, Nov 1, 2011 at 2:49 PM, Bruce Momjian  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  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_upgrade if 'postgres' database is dropped

[Bruce Momjian]

Robert Haas wrote:
> On Tue, Nov 1, 2011 at 1:53 PM, Bruce Momjian  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  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_upgrade if 'postgres' database is dropped

[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  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_upgrade if 'postgres' database is dropped

[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  http://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 
  	i

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

[Simon Riggs]

On Tue, Nov 1, 2011 at 6:12 PM, Robert Treat  wrote:
> On Tue, Nov 1, 2011 at 1:34 PM, Simon Riggs  wrote:
>> On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus  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

[Robert Haas]

On Tue, Nov 1, 2011 at 2:36 PM, Bruce Momjian  wrote:
> Robert Haas wrote:
>> On Tue, Nov 1, 2011 at 1:53 PM, Bruce Momjian  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

[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

[Magnus Hagander]

On Tue, Nov 1, 2011 at 18:11, Scott Mead  wrote:
>
> On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane  wrote:
>>
>> Robert Haas  writes:
>> > On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane  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  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 
>> and  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)

[Robert Haas]

On Tue, Nov 1, 2011 at 1:32 PM, Kohei KaiGai  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

[Robert Haas]

On Tue, Nov 1, 2011 at 2:49 PM, Bruce Momjian  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

[Josh Berkus]

On 11/1/11 10:34 AM, Simon Riggs wrote:
> On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus  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

[Robert Haas]

On Tue, Nov 1, 2011 at 1:53 PM, Bruce Momjian  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 ..."?

[Pavel Stehule]

2011/11/1 Eric Ridge :
> On Tue, Nov 1, 2011 at 12:24 PM, Pavel Stehule  
> 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 ) 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?)

[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  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

[Scott Mead]

On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander  wrote:

> On Tue, Nov 1, 2011 at 18:11, Scott Mead  wrote:
> >
> > On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane  wrote:
> >>
> >> Robert Haas  writes:
> >> > On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane  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  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".
>


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


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 
> >> and  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

[Magnus Hagander]

On Tue, Nov 1, 2011 at 18:40, Scott Mead  wrote:
>
>
> On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander  wrote:
>>
>> On Tue, Nov 1, 2011 at 18:11, Scott Mead  wrote:
>> >
>> > On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane  wrote:
>> >>
>> >> Robert Haas  writes:
>> >> > On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane  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  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

[Robert Treat]

On Tue, Nov 1, 2011 at 2:34 PM, Simon Riggs  wrote:
> On Tue, Nov 1, 2011 at 6:12 PM, Robert Treat  wrote:
>> On Tue, Nov 1, 2011 at 1:34 PM, Simon Riggs  wrote:
>>> On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus  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 ..."?

[Eric Ridge]

On Tue, Nov 1, 2011 at 12:03 PM, Stephen Frost  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?)

[Merlin Moncure]

On Tue, Nov 1, 2011 at 9:55 AM, Robert Haas  wrote:
> On Tue, Nov 1, 2011 at 10:47 AM, Marti Raudsepp  wrote:
>> On Fri, Oct 28, 2011 at 20:58, Robert Haas  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

[Robert Treat]

On Tue, Nov 1, 2011 at 1:20 PM, Magnus Hagander  wrote:
> On Tue, Nov 1, 2011 at 18:11, Scott Mead  wrote:
>>
>> On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane  wrote:
>>>
>>> Robert Haas  writes:
>>> > On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane  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  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 
>>> and  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  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

[Andrew Dunstan]

On 11/01/2011 09:52 AM, Tom Lane wrote:

Simon Riggs  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 ..."?

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

[Tom Lane]

Eric Ridge  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 ..."?

[Eric B. Ridge]

On Nov 1, 2011, at 6:47 PM, Tom Lane  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

[Robert Treat]

On Tue, Nov 1, 2011 at 1:34 PM, Simon Riggs  wrote:
> On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus  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

[Simon Riggs]

On Tue, Nov 1, 2011 at 5:11 PM, Joshua Berkus  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

[Scott Mead]

On Tue, Nov 1, 2011 at 10:40 AM, Tom Lane  wrote:

> Robert Haas  writes:
> > On Tue, Nov 1, 2011 at 9:52 AM, Tom Lane  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  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 
> and  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

[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 Riggs  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 ..."?

[Pavel Stehule]

2011/11/1 Eric Ridge :
> On Tue, Nov 1, 2011 at 1:33 PM, Pavel Stehule  wrote:
>>> COPY (SELECT * EXCLUDING (a, b, c) FROM ) 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)

[Kohei KaiGai]

2011/11/1 Robert Haas :
> On Tue, Nov 1, 2011 at 1:32 PM, Kohei KaiGai  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 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Eric Ridge]

On Tue, Nov 1, 2011 at 12:24 PM, Pavel Stehule  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 ) 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

[Robert Haas]

On Thu, Oct 6, 2011 at 12:46 PM, Tom Lane  wrote:
> Robert Haas  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

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

[Eric Ridge]

On Tue, Nov 1, 2011 at 1:33 PM, Pavel Stehule  wrote:
>> COPY (SELECT * EXCLUDING (a, b, c) FROM ) 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'?

[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

[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

[Alcione Benacchio]

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

[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

[Simon Riggs]

On Wed, Oct 19, 2011 at 3:58 PM, Simon Riggs  wrote:
> On Wed, Oct 19, 2011 at 3:29 PM, Fujii Masao  wrote:
>> On Wed, Oct 19, 2011 at 9:45 PM, Robert Haas  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

[Robert Haas]

On Tue, Nov 1, 2011 at 9:11 PM, Josh Berkus  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

[Chris Redekop]

looks like the v3 patch re-introduces the pg_subtrans issue...


On Tue, Nov 1, 2011 at 9:33 AM, Simon Riggs  wrote:

> On Thu, Oct 27, 2011 at 4:25 PM, Simon Riggs 
> 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 ..."?

[Tom Lane]

"Eric B. Ridge"  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

[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'?

[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