Re: [HACKERS] UTF16 surrogate pairs in UTF8 encoding

2010-08-23 Thread Florian Weimer
* Tom Lane:

 I just noticed that we are now advertising the ability to insert UTF16
 surrogate pairs in strings and identifiers (see section 4.1.2.2 in
 current docs, in particular).  Is this really wise?  I thought that
 surrogate pairs were specifically prohibited in UTF8 strings, because
 of the security hazards implicit in having more than one way to
 represent the same code point.

There is relatively little risk because surrogate pairs cannot encode
characters in the BMP, and presumably, most of the critical characters
are located there.

However, if this is converted to regular UTF-8, I really question the
sense of this.  Usually, people want CESU-8 to preserve ordering
between languages such as C# and Java and their database, and
conversion destroys this property.

-- 
Florian Weimerfwei...@bfk.de
BFK edv-consulting GmbH   http://www.bfk.de/
Kriegsstraße 100  tel: +49-721-96201-1
D-76133 Karlsruhe fax: +49-721-96201-99

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


Re: [HACKERS] git: uh-oh

2010-08-23 Thread Magnus Hagander
On Sat, Aug 21, 2010 at 08:15, Michael Haggerty mhag...@alum.mit.edu wrote:
 Max Bowsher wrote:
 On 20/08/10 19:07, Magnus Hagander wrote:
 On Fri, Aug 20, 2010 at 19:56, Max Bowsher m...@f2s.com wrote:
 On 20/08/10 18:43, Magnus Hagander wrote:
 On Fri, Aug 20, 2010 at 19:41, Max Bowsher m...@f2s.com wrote:
 On 20/08/10 18:30, Magnus Hagander wrote:
 On Fri, Aug 20, 2010 at 19:28, Tom Lane t...@sss.pgh.pa.us wrote:
 Max Bowsher m...@f2s.com writes:
 The history that cvs2svn is aiming to represent here is this:
 1) At the time of creation of the REL8_4_STABLE branch, 
 plperl_opmask.pl
 did *not* exist.
 2) Later, it was added to trunk.
 3) Then, someone retroactively added the branch tag to the file, 
 marking
 it as included in the REL8_4_STABLE branch. [This corresponds to the 
 git
 changeset that Robert is questioning]
 Uh, no.  We have never retroactively added anything to any branch.
 I don't know enough about the innards of CVS to know what its internal
 representation of this sort of thing is, but all that actually happened
 here was a cvs add and a cvs commit in REL8_4_STABLE long after the
 branch occurred.  We would like the git history to look like that too.
 Yeah.

 In fact, is the only thing that's wrong here the commit message?
 Because it's probably trivial to just patch that away.. Hmm, but i
 guess we'd like to hav ethe actual commit message and not just another
 fixed one..
 There is no actual commit message - the entire changeset is
 synthesized by cvs2git to represent the addition of a branch tag to the
 file - i.e. the logical equivalent of a cvs tag -b, which has no
 commit message.
 There is a commit message on the trunk when the file was added there.
 Is there any chance of being able to lift that message off trunk and
 just copy it into the branch, instead of saying this is a cherry-pick
 of this commit over here?
 It wouldn't be accurate to do so, because the synthetic commit is not
 copying the entire change, only registering the addition of (in this
 case) one file which happens to be part of the changeset. Please note
 that there is a changeset on the branch immediately following the
 synthetic one under discussion which contains the 'real' commit message
 used when committing to the branch.
 Hmm. Good point.

 I wonder if we should try to locate these places and fix them with git
 filter-branch or rebase -i after the fact, with history rewriting.

 There seem to be 57 of them.

 It sounds cumbersome.

 Michael Haggerty might be better placed than me to assess whether
 eliding them within cvs2git is practically achievable.

 I think this would be nontrivial.

snip


 Moreover, this is a pretty specialized request that would be useless to
 people who are not so disciplined about their repository as you seem to be.

Yeah, I think we're unusually disciplined with our repository - that's
one reason the change won't be that drastic wrt how things are done
:-)


 It seems like you already have a way to find these events in the git
 repository after conversion, so I think it would be more practical to
 use git-filter-branch to remove the unwanted commits *after* the conversion.

Not sure that we do have an automated way, but I agree that this is
probably going to be easier to do with git-filter-branch.

If we need to do it at all. Tom's latest lookover indicates that he
thinks it may be good the way it is, and we need some more detailed
checks. I know Robert has said he wants to dedicate some time to doing
such checks this week, and I'll see if I can find some time for that
as well. If anybody else would like to help us dig through mainly the
backbranches - with focus on branchpoints and taggings - to look for
any kind of weird stuff (meaning anything that's not a straight
commit), then please do so and let us know your results!


-- 
 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] More vacuum stats

2010-08-23 Thread Magnus Hagander
On Sun, Aug 22, 2010 at 18:17, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Sun, Aug 22, 2010 at 17:29, Tom Lane t...@sss.pgh.pa.us wrote:
 So I'd like to see a positive argument why this is important for users
 to know, rather than merely we should expose every conceivable detail
 by default.  Why wouldn't a user care more about last AV time for a
 specific table, which we already do expose?

 You need to connect to every database to do that. If you have many
 databases, that's a lot of overhead particularly if you're doing tihs
 for regular monitoring. Plus, those views will only track when
 autovacuum actually *did* something.

 Well, the last-launch-time doesn't prove that autovacuum actually *did*
 something ;-).

Well, it would tell you it considered doing something ;)


 Being able to see that autovacuum hasn't even touched a database for
 too long would be an early-indicator that you have some issues with
 it.

 With the current AV launch algorithm, unless you have very serious
 system-wide issues there will be a worker launched into each database
 approximately every autovacuum_naptime seconds.  AFAICS this does not
 tell you anything interesting about whether AV is getting its work done.

Well, if you have all your autovacuum workers tied up with vacuuming
large tables, then it wouldn't AFAIK. I'm not sure if that counts as
your very serious system-wide issues, but it's certainly a case
that's interesting for the admin to know about.

But thinking more about that, you ca nfigure that out with a SELECT
count(*) FROM pg_stat_activity WHERE current_query LIKE 'autovacuum:
%' if I'm not mistaken.

It can also be used to find out if the launcher is somehoiw stuck, but
that would be a bug and we don't generally put counters in the stats
views to expose possible bugs, only to track interesting statistics.

-- 
 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] UTF16 surrogate pairs in UTF8 encoding

2010-08-23 Thread Marko Kreen
On 8/22/10, Peter Eisentraut pete...@gmx.net wrote:
 On sön, 2010-08-22 at 14:29 -0400, Tom Lane wrote:
   I just noticed that we are now advertising the ability to insert UTF16
   surrogate pairs in strings and identifiers (see section 4.1.2.2 in
   current docs, in particular).  Is this really wise?  I thought that
   surrogate pairs were specifically prohibited in UTF8 strings, because
   of the security hazards implicit in having more than one way to
   represent the same code point.


 We combine the surrogate pair components to a single code point and
  encode that in UTF-8.  We don't encode the components separately; that
  would be wrong.

AFAICS our UTF8 validator (pg_utf8_islegal) detects and rejects
such sequences, if they are inserted via any means, eg. \x

Although it's not very obvious...

-- 
marko

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


Re: [HACKERS] patch (for 9.1) string functions

2010-08-23 Thread Itagaki Takahiro
On Sat, Aug 7, 2010 at 8:39 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 I made a discussion page in wiki for the compatibility issue.
 http://wiki.postgresql.org/wiki/String_Functions_and_Operators_Compatibility
 nice, thank you

I filled cells for SQL Server and DB2.

  * concat() is not compatible between MySQL and Oracle/DB2. Which do we buy?
 I prefer a our implementation - it skip a NULL values and it has a
 variadic arguments.

OK. I'm going to put both concat() and concat_ws() into core.

  * How do other databases behave in left() and right() with negative lengths?
 little bit by python substring operations.

I'll respect your proposal. The behaviors for negative lengths will
be our specific feature, but I don't see any problems there.
Since other databases raises errors, user should have negative-protections
in their existing codes.

 I don't agree. This function isn't designed to replace string
 concation. It is designed to build a SQL string (for dynamic SQL) or
 format messages. It isn't designed to replace to_char function. It is
 designed to work mainly inside PLpgSQL functions and then is
 consistent with RAISE statement.

OK. I'll revert my changes to your original format().

But please wait a moment to include sprintf() and contrib/stringfunc.
I think the function is useful, but don't want to have two versions
of formatting functions. So, the extended features will be merged
into format() with additional syntax something like {10s}. Then,
we could simplify the code because some of complex format syntax
are not so useful in SQL, especially length+string formatter (*s).

-- 
Itagaki Takahiro

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


Re: [HACKERS] patch (for 9.1) string functions

2010-08-23 Thread Pavel Stehule
2010/8/23 Itagaki Takahiro itagaki.takah...@gmail.com:
 On Sat, Aug 7, 2010 at 8:39 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 I made a discussion page in wiki for the compatibility issue.
 http://wiki.postgresql.org/wiki/String_Functions_and_Operators_Compatibility
 nice, thank you

 I filled cells for SQL Server and DB2.

  * concat() is not compatible between MySQL and Oracle/DB2. Which do we buy?
 I prefer a our implementation - it skip a NULL values and it has a
 variadic arguments.

 OK. I'm going to put both concat() and concat_ws() into core.

  * How do other databases behave in left() and right() with negative 
 lengths?
 little bit by python substring operations.

 I'll respect your proposal. The behaviors for negative lengths will
 be our specific feature, but I don't see any problems there.
 Since other databases raises errors, user should have negative-protections
 in their existing codes.

 I don't agree. This function isn't designed to replace string
 concation. It is designed to build a SQL string (for dynamic SQL) or
 format messages. It isn't designed to replace to_char function. It is
 designed to work mainly inside PLpgSQL functions and then is
 consistent with RAISE statement.

 OK. I'll revert my changes to your original format().

 But please wait a moment to include sprintf() and contrib/stringfunc.
 I think the function is useful, but don't want to have two versions
 of formatting functions. So, the extended features will be merged
 into format() with additional syntax something like {10s}. Then,
 we could simplify the code because some of complex format syntax
 are not so useful in SQL, especially length+string formatter (*s).

It's reason, why I moved sprintf to contrib. When you build a SQL
statement or error message, you don't need (usually) complex
formating. And when you need it, then you can use a contrib sprintf
function. I am not against to additional simply formating - but I
afraid so we will create a second printf function.  The formating
enhancing should be shared with RAISE NOTICE command. Probably we can
share code better now, when a PLpgSQL is in core.

Regards

Pavel



 --
 Itagaki Takahiro


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


Re: [HACKERS] [BUGS] BUG #5305: Postgres service stops when closing Windows session

2010-08-23 Thread Robert Haas
[moving to -hackers]

On Thu, Aug 19, 2010 at 9:43 PM, Robert Haas robertmh...@gmail.com wrote:
 I suspect this is the same problem as bug #4897, and probably also the
 same problem as this:
 http://archives.postgresql.org/pgsql-bugs/2009-08/msg00114.php

 and maybe also this and this:
 http://archives.postgresql.org/pgsql-bugs/2010-02/msg00179.php
 http://archives.postgresql.org/pgsql-admin/2009-05/msg00105.php

 Unfortunately, it seems that no one has been able to get a stack trace yet.

Bruce pointed out yet another report of this problem to me:

http://archives.postgresql.org/pgsql-general/2010-08/msg00550.php

After some discussion with Magnus, I think what is going on here is
that the postmaster kicks off a new child process, which terminates
before it actually starts running our code, either in OS-supplied code
or some sort of filter like anti-spam or anti-virus software.  It's
presumably NOT dying in our code because - at least AFAICS - we don't
exit(128) anywhere.  One way we could possibly improve the situation
is to not treat this as a child crash - that is, don't do a
crash-and-restart cycle; just treat that backend as having done
elog(FATAL).  The trick is that you need a reliable way to distinguish
between a regular child crash and an early child crash.  Magnus
suggested perhaps we could create a mutex that the child grabs before
mapping shared memory; the postmaster could check whether the mutex
had been taken.  If so, we handle the crash normally; if not, we just
chalk it up to experience and continue on.

This isn't really a fix for the bug in the sense that the nicest
thing of all would be to prevent the child from exiting abnormally in
the first place.  But it's far from clear that we can control that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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


Re: [HACKERS] More vacuum stats

2010-08-23 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Sun, Aug 22, 2010 at 18:17, Tom Lane t...@sss.pgh.pa.us wrote:
 With the current AV launch algorithm, unless you have very serious
 system-wide issues there will be a worker launched into each database
 approximately every autovacuum_naptime seconds.  AFAICS this does not
 tell you anything interesting about whether AV is getting its work done.

 Well, if you have all your autovacuum workers tied up with vacuuming
 large tables, then it wouldn't AFAIK. I'm not sure if that counts as
 your very serious system-wide issues, but it's certainly a case
 that's interesting for the admin to know about.

 But thinking more about that, you ca nfigure that out with a SELECT
 count(*) FROM pg_stat_activity WHERE current_query LIKE 'autovacuum:
 %' if I'm not mistaken.

 It can also be used to find out if the launcher is somehoiw stuck, but
 that would be a bug and we don't generally put counters in the stats
 views to expose possible bugs, only to track interesting statistics.

Yeah.  Given the current worker-launch algorithm, these times just don't
strike me as all that interesting in practice.  If we were to change to
a different algorithm, it's possible that it'd become worthwhile to
expose them --- but it's equally possible that some other data would be
useful instead.  So my feeling remains that we should leave well enough
alone.

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] patch (for 9.1) string functions

2010-08-23 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 ... The formating
 enhancing should be shared with RAISE NOTICE command.

I remain of the opinion that RAISE's approach to formatting is
completely broken and inextensible, and that any attempt to be somehow
compatible with it is going to lead to an unusably broken design.
You should leave RAISE alone and just think about printf.

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] More vacuum stats

2010-08-23 Thread Greg Smith

Tom Lane wrote:

So I'd like to see a positive argument why this is important for users
to know, rather than merely we should expose every conceivable detail
by default.  Why wouldn't a user care more about last AV time for a
specific table, which we already do expose?
  


What I actually want here is for the time that the last table autovacuum 
started, adding to the finish time currently exposed by 
pg_stat_user_tables.  How long did the last {auto}vacuum on x take to 
run? is a FAQ on busy systems here.  If I could compute that from a 
pair of columns, it's a major step toward answering even more 
interesting questions like how does this set of cost delay parameters 
turn into an approximate MB/s worth of processing rate on my tables?.  
This is too important of a difficult tuning exercise to leave to log 
scraping forever.


I'd rather have that and look at for SELECT max(last_autovacuum_start) 
FROM pg_stat_user_tables to diagnose the sort of problems this patch 
seems to aim at helping.


--
Greg Smith  2ndQuadrant US  Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com   www.2ndQuadrant.us


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


Re: [HACKERS] More vacuum stats

2010-08-23 Thread Magnus Hagander
On Mon, Aug 23, 2010 at 16:28, Greg Smith g...@2ndquadrant.com wrote:
 Tom Lane wrote:

 So I'd like to see a positive argument why this is important for users
 to know, rather than merely we should expose every conceivable detail
 by default.  Why wouldn't a user care more about last AV time for a
 specific table, which we already do expose?


 What I actually want here is for the time that the last table autovacuum
 started, adding to the finish time currently exposed by pg_stat_user_tables.
  How long did the last {auto}vacuum on x take to run? is a FAQ on busy
 systems here.  If I could compute that from a pair of columns, it's a major
 step toward answering even more interesting questions like how does this
 set of cost delay parameters turn into an approximate MB/s worth of
 processing rate on my tables?.  This is too important of a difficult tuning
 exercise to leave to log scraping forever.

Now, that would be quite useful. That'd require another stats message,
since we don't send anything on autovacuum start, but I don't think
the overhead of that is anything we need to worry about - in
comparison to an actual vacuum...

Do we want that for both vacuum and autovacuum? vacuum and analyze?

We could also store last_autovacuum_vacuum_duration - is that better
or worse than start and end time?


 I'd rather have that and look at for SELECT max(last_autovacuum_start) FROM
 pg_stat_user_tables to diagnose the sort of problems this patch seems to
 aim at helping.

Agreed. Consider this patch withdrawn.

-- 
 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] More vacuum stats

2010-08-23 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 23, 2010 at 16:28, Greg Smith g...@2ndquadrant.com wrote:
 What I actually want here is for the time that the last table autovacuum
 started, adding to the finish time currently exposed by pg_stat_user_tables.

 Now, that would be quite useful. That'd require another stats message,
 since we don't send anything on autovacuum start, but I don't think
 the overhead of that is anything we need to worry about - in
 comparison to an actual vacuum...

No, you wouldn't really need an extra message, you could just send both
start and finish times in the completion message.  I'm not sure that
having last start time update before last end time would be a good idea
anyway.

But in any case it's true that an extra message wouldn't be a
significant cost.  What I'd be more concerned about is the stats table
bloat from adding yet another per-table field.  That could be a lot of
space on an installation with lots of tables.

 We could also store last_autovacuum_vacuum_duration - is that better
 or worse than start and end time?

No, I think you want to know the actual time not only the duration.

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] More vacuum stats

2010-08-23 Thread Magnus Hagander
On Mon, Aug 23, 2010 at 16:38, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 23, 2010 at 16:28, Greg Smith g...@2ndquadrant.com wrote:
 What I actually want here is for the time that the last table autovacuum
 started, adding to the finish time currently exposed by pg_stat_user_tables.

 Now, that would be quite useful. That'd require another stats message,
 since we don't send anything on autovacuum start, but I don't think
 the overhead of that is anything we need to worry about - in
 comparison to an actual vacuum...

 No, you wouldn't really need an extra message, you could just send both
 start and finish times in the completion message.  I'm not sure that
 having last start time update before last end time would be a good idea
 anyway.

Hmm, good point. We'd just need an extra field in that message.


 But in any case it's true that an extra message wouldn't be a
 significant cost.  What I'd be more concerned about is the stats table
 bloat from adding yet another per-table field.  That could be a lot of
 space on an installation with lots of tables.

 We could also store last_autovacuum_vacuum_duration - is that better
 or worse than start and end time?

 No, I think you want to know the actual time not only the duration.

Well, you could calculate one from the other - especially if one takes
less size, per your comment above.


-- 
 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] patch (for 9.1) string functions

2010-08-23 Thread Pavel Stehule
2010/8/23 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 ... The formating
 enhancing should be shared with RAISE NOTICE command.

 I remain of the opinion that RAISE's approach to formatting is
 completely broken and inextensible, and that any attempt to be somehow
 compatible with it is going to lead to an unusably broken design.
 You should leave RAISE alone and just think about printf.


ok - then we don't need modify proposed patch. Format function is
enough for PL/pgSQL and other PL languages has own mutation of this
functions. There are not barrier for implementation as custom
function, so we can hold this function most simple.

Regards

Pavel


                        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] More vacuum stats

2010-08-23 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 23, 2010 at 16:38, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 We could also store last_autovacuum_vacuum_duration - is that better
 or worse than start and end time?
 
 No, I think you want to know the actual time not only the duration.

 Well, you could calculate one from the other - especially if one takes
 less size, per your comment above.

With alignment considerations, adding a field is going to cost 8 bytes;
whether it's a timestamp or a duration isn't going to matter.  I'd be
inclined to store the timestamp, it just seems more like the base datum.

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] [BUGS] BUG #5305: Postgres service stops when closing Windows session

2010-08-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 After some discussion with Magnus, I think what is going on here is
 that the postmaster kicks off a new child process, which terminates
 before it actually starts running our code, either in OS-supplied code
 or some sort of filter like anti-spam or anti-virus software.  It's
 presumably NOT dying in our code because - at least AFAICS - we don't
 exit(128) anywhere.

IIRC, in POSIX-compliant shells there's a specific convention about what
exit(128) means, and it's something that could result from exec()
failure.  It might be too much of a stretch to suppose that Windows is
following that, but if it is, that would square with your idea that this
is happening during child process startup.

 One way we could possibly improve the situation
 is to not treat this as a child crash - that is, don't do a
 crash-and-restart cycle; just treat that backend as having done
 elog(FATAL).

That seems to me like a great idea for decreasing reliability, not
increasing it.  If you mistakenly classify a child death as not
a crash then you're really seriously hosed; the best outcome you
can hope for is that the database freezes up without doing any
major damage to itself.

Furthermore, even if it is an early exit and you can afford to ignore
it, the client side is still going to see a dropped connection and tell
the user that the server crashed, and we're still going to get bug
reports about that.

I would be inclined to write this off as Windows randomness that's
unfixable on our end.  We could recommend that people take a closer
look at what AV software they have installed and maybe try some other
one.

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] [BUGS] BUG #5305: Postgres service stops when closing Windows session

2010-08-23 Thread Magnus Hagander
On Mon, Aug 23, 2010 at 17:09, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 After some discussion with Magnus, I think what is going on here is
 that the postmaster kicks off a new child process, which terminates
 before it actually starts running our code, either in OS-supplied code
 or some sort of filter like anti-spam or anti-virus software.  It's
 presumably NOT dying in our code because - at least AFAICS - we don't
 exit(128) anywhere.

 IIRC, in POSIX-compliant shells there's a specific convention about what
 exit(128) means, and it's something that could result from exec()
 failure.  It might be too much of a stretch to suppose that Windows is
 following that, but if it is, that would square with your idea that this
 is happening during child process startup.

It is (assuming the idea is correct).

The problem is that the error code is not delivered at CreateProcess()
time - it's delivered later.


 One way we could possibly improve the situation
 is to not treat this as a child crash - that is, don't do a
 crash-and-restart cycle; just treat that backend as having done
 elog(FATAL).

 That seems to me like a great idea for decreasing reliability, not
 increasing it.  If you mistakenly classify a child death as not
 a crash then you're really seriously hosed; the best outcome you
 can hope for is that the database freezes up without doing any
 major damage to itself.

 Furthermore, even if it is an early exit and you can afford to ignore
 it, the client side is still going to see a dropped connection and tell
 the user that the server crashed, and we're still going to get bug
 reports about that.

Yes, but it's Less Evil.


 I would be inclined to write this off as Windows randomness that's
 unfixable on our end.  We could recommend that people take a closer
 look at what AV software they have installed and maybe try some other
 one.

It may well be, but we can at least attempt to mitigate it, no?

-- 
 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] [BUGS] BUG #5305: Postgres service stops when closing Windows session

2010-08-23 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 23, 2010 at 17:09, Tom Lane t...@sss.pgh.pa.us wrote:
 I would be inclined to write this off as Windows randomness that's
 unfixable on our end.  We could recommend that people take a closer
 look at what AV software they have installed and maybe try some other
 one.

 It may well be, but we can at least attempt to mitigate it, no?

I'm not excited about a mitigation approach that introduces new
data-loss hazards of its very own.  That doesn't meet the Less Evil
standard in my eyes.

[ thinks for a bit... ]  Although maybe it'd be all right to piggyback
on the dead-man-switch code that already exists in pmsignal.c.  If the
child process hasn't got as far as doing MarkPostmasterChildActive,
then in principle it should be okay to assume it hasn't touched shared
memory.  This really is independent of what exit code it returned.

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] WIP: extensible enums

2010-08-23 Thread Alvaro Herrera
Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010:

 To add a label at the end of the list, do:
 
   ALTER TYPE myenum ADD 'newlabel';
 
 To add a label somewhere else, do:
 
   ALTER TYPE myenum ADD 'newlabel' BEFORE 'existinglabel';
 
 or
 
   ALTER TYPE myenum ADD 'newlabel' AFTER 'existinglabel';

What do you need AFTER for?  Seems to me that BEFORE should be enough.
(You already have the unadorned syntax for adding an item after the last
one, which is the corner case that BEFORE alone doesn't cover).

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Alvaro Herrera
Excerpts from Bruce Momjian's message of dom ago 22 12:51:47 -0400 2010:

 Well, the reason that value is 200 million is for pg_clog cleanup, not
 for xid wraparound protection.  The next sentence does relate to xid
 wraparound, but it seems to fit because the previous sentence ends with
 xid wraparound:
 
 Note that the system will launch autovacuum processes to
 prevent wraparound even when autovacuum is otherwise disabled.
 
 If we were worried about just xid wraparound I assume the value would be
 2 billion.
 
 Do you have a suggestion?  Reorder the items?

I'd add another para before that one saying that this value also
affects pg_clog truncation.  I agree that putting pg_clog truncation as
the first item here is not an improvement.  For most people, having
those pg_clog files there or not is going to be a wash, compared to data
size.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Bruce Momjian's message of dom ago 22 12:51:47 -0400 2010:
 Do you have a suggestion?  Reorder the items?

 I'd add another para before that one saying that this value also
 affects pg_clog truncation.  I agree that putting pg_clog truncation as
 the first item here is not an improvement.  For most people, having
 those pg_clog files there or not is going to be a wash, compared to data
 size.

I was going to suggest that the point about pg_clog should be in a
separate paragraph *after* this one, since it seems like a secondary
issue.  But anyway, I agree with putting this para back as it was and
talking about clog in a separate para.

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] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Alvaro Herrera
Excerpts from Tom Lane's message of lun ago 23 12:40:32 -0400 2010:
 Alvaro Herrera alvhe...@commandprompt.com writes:
  Excerpts from Bruce Momjian's message of dom ago 22 12:51:47 -0400 2010:
  Do you have a suggestion?  Reorder the items?
 
  I'd add another para before that one saying that this value also
  affects pg_clog truncation.  I agree that putting pg_clog truncation as
  the first item here is not an improvement.  For most people, having
  those pg_clog files there or not is going to be a wash, compared to data
  size.
 
 I was going to suggest that the point about pg_clog should be in a
 separate paragraph *after* this one, since it seems like a secondary
 issue.  But anyway, I agree with putting this para back as it was and
 talking about clog in a separate para.

Sorry, yes, I was also thinking after.  I don't know what made me
write before but it wasn't clarity of thought.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David E. Wheeler
On Aug 23, 2010, at 2:35 AM, Andrew Dunstan wrote:

 I'm not wedded to the syntax. Let the bikeshedding begin.

Seems pretty good to me as-is.

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] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Josh Berkus
All,

FYI, the system which sparked this original discussion turned out, on
extensive analysis, to have ZFS-level filesystem corruption.  The
polling issues were related to that rather than to Postgres.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.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] WIP: extensible enums

2010-08-23 Thread Andrew Dunstan
On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote:
 Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010:

 To add a label at the end of the list, do:

   ALTER TYPE myenum ADD 'newlabel';

 To add a label somewhere else, do:

   ALTER TYPE myenum ADD 'newlabel' BEFORE 'existinglabel';

 or

   ALTER TYPE myenum ADD 'newlabel' AFTER 'existinglabel';

 What do you need AFTER for?  Seems to me that BEFORE should be enough.
 (You already have the unadorned syntax for adding an item after the last
 one, which is the corner case that BEFORE alone doesn't cover).


You're right. Strictly speaking we don't need it. But it doesn't hurt much
to provide it for a degree of symmetry.

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] WIP: extensible enums

2010-08-23 Thread Andrew Dunstan
On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote:
 Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010:

 To add a label at the end of the list, do:

   ALTER TYPE myenum ADD 'newlabel';

 To add a label somewhere else, do:

   ALTER TYPE myenum ADD 'newlabel' BEFORE 'existinglabel';

 or

   ALTER TYPE myenum ADD 'newlabel' AFTER 'existinglabel';

 What do you need AFTER for?  Seems to me that BEFORE should be enough.
 (You already have the unadorned syntax for adding an item after the last
 one, which is the corner case that BEFORE alone doesn't cover).


You're right. Strictly speaking we don't need it. But it doesn't hurt much
to provide it for a degree of symmetry.

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] WIP: extensible enums

2010-08-23 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote:
 What do you need AFTER for?  Seems to me that BEFORE should be enough.
 (You already have the unadorned syntax for adding an item after the last
 one, which is the corner case that BEFORE alone doesn't cover).

 You're right. Strictly speaking we don't need it. But it doesn't hurt much
 to provide it for a degree of symmetry.

I'm with Alvaro: drop the AFTER variant.  It provides more than one way
to do the same thing, which isn't that exciting, and it's also going to
make it harder to document the performance issues.  Without that, you
can just say ADD BEFORE will make the enum slower, but plain ADD won't
(ignoring the issue of OID wraparound, which'll confuse matters in any
case).

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] WIP: extensible enums

2010-08-23 Thread Josh Berkus

 You're right. Strictly speaking we don't need it. But it doesn't hurt much
 to provide it for a degree of symmetry.

Swami Josh predicts that if we don't add AFTER now, we'll be adding it
in 2 years when enough people complain about it.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.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] WIP: extensible enums

2010-08-23 Thread Thom Brown
On 23 August 2010 10:35, Andrew Dunstan and...@dunslane.net wrote:

 Attached is a WIP patch that allows enums to be extended with additional
 labels arbitrarily. As previously discussed, it works by adding an explicit
 sort order column to pg_enum. It keeps track of whether the labels are
 correctly sorted by oid value, and if so uses that for comparison, so the
 possible performance impact on existing uses, and on almost all cases where
 a label is added at the end of the list, should be negligible.

 Open items include

   * some additional error checking required
   * missing documentation
   * pg_upgrade considerations


 To add a label at the end of the list, do:

  ALTER TYPE myenum ADD 'newlabel';

 To add a label somewhere else, do:

  ALTER TYPE myenum ADD 'newlabel' BEFORE 'existinglabel';

 or

  ALTER TYPE myenum ADD 'newlabel' AFTER 'existinglabel';


 I'm not wedded to the syntax. Let the bikeshedding begin.

 cheers

 andrew

When you write the supporting doc changes, you might want to add a
note in to mention that you cannot remove a label once it has been
added.

Will the modified enums remain intact after a dump/restore?

-- 
Thom Brown
Registered Linux user: #516935

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David Fetter
On Mon, Aug 23, 2010 at 11:49:41AM -0400, Alvaro Herrera wrote:
 Excerpts from Andrew Dunstan's message of lun ago 23 05:35:09 -0400 2010:
 
  To add a label at the end of the list, do:
  
ALTER TYPE myenum ADD 'newlabel';
  
  To add a label somewhere else, do:
  
ALTER TYPE myenum ADD 'newlabel' BEFORE 'existinglabel';
  
  or
  
ALTER TYPE myenum ADD 'newlabel' AFTER 'existinglabel';
 
 What do you need AFTER for?  Seems to me that BEFORE should be enough.
 (You already have the unadorned syntax for adding an item after the last
 one, which is the corner case that BEFORE alone doesn't cover).

Making things easier for the users is a good thing all by itself :)

+1 for including both BEFORE and AFTER.  Would it be worth it to allow
something like FIRST and LAST?

ALTER TYPE myenum ADD 'newlabel' FIRST;

ALTER TYPE myenum ADD 'newlabel' LAST;

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David Fetter
On Mon, Aug 23, 2010 at 01:54:40PM -0400, Tom Lane wrote:
 Andrew Dunstan and...@dunslane.net writes:
  On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote:
  What do you need AFTER for?  Seems to me that BEFORE should be
  enough.  (You already have the unadorned syntax for adding an
  item after the last one, which is the corner case that BEFORE
  alone doesn't cover).
 
  You're right. Strictly speaking we don't need it. But it doesn't
  hurt much to provide it for a degree of symmetry.
 
 I'm with Alvaro: drop the AFTER variant.  It provides more than one
 way to do the same thing, which isn't that exciting,

Not to you, maybe, but to users, it's really handy to have intuitive,
rather than strictly orthogonal, ways to do things.

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes:
 Swami Josh predicts that if we don't add AFTER now, we'll be adding it
 in 2 years when enough people complain about it.

If it's not there, no one will ever miss it.  You might as well argue
that there should be a way of creating a foreign key reference by
ALTER'ing the referenced table instead of the referencing table.
Sure, if the SQL committee was into symmetry, they might have provided
such a thing.  But they didn't and no one misses it.

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] WIP: extensible enums

2010-08-23 Thread Joseph Adams
On Mon, Aug 23, 2010 at 1:54 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Andrew Dunstan and...@dunslane.net writes:
 On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote:
 What do you need AFTER for?  Seems to me that BEFORE should be enough.
 (You already have the unadorned syntax for adding an item after the last
 one, which is the corner case that BEFORE alone doesn't cover).

 You're right. Strictly speaking we don't need it. But it doesn't hurt much
 to provide it for a degree of symmetry.

 I'm with Alvaro: drop the AFTER variant.  It provides more than one way
 to do the same thing, which isn't that exciting, and it's also going to
 make it harder to document the performance issues.  Without that, you
 can just say ADD BEFORE will make the enum slower, but plain ADD won't
 (ignoring the issue of OID wraparound, which'll confuse matters in any
 case).

But what if you want to insert an OID at the end?  You can't do it if
all you've got is BEFORE:

CREATE TYPE colors AS ENUM ('red', 'green', 'blue');

If I want it to become ('red', 'green', 'blue', 'orange'), what am I to do?

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Thom Brown
On 23 August 2010 19:25, Joseph Adams joeyadams3.14...@gmail.com wrote:
 On Mon, Aug 23, 2010 at 1:54 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Andrew Dunstan and...@dunslane.net writes:
 On Mon, August 23, 2010 11:49 am, Alvaro Herrera wrote:
 What do you need AFTER for?  Seems to me that BEFORE should be enough.
 (You already have the unadorned syntax for adding an item after the last
 one, which is the corner case that BEFORE alone doesn't cover).

 You're right. Strictly speaking we don't need it. But it doesn't hurt much
 to provide it for a degree of symmetry.

 I'm with Alvaro: drop the AFTER variant.  It provides more than one way
 to do the same thing, which isn't that exciting, and it's also going to
 make it harder to document the performance issues.  Without that, you
 can just say ADD BEFORE will make the enum slower, but plain ADD won't
 (ignoring the issue of OID wraparound, which'll confuse matters in any
 case).

 But what if you want to insert an OID at the end?  You can't do it if
 all you've got is BEFORE:

 CREATE TYPE colors AS ENUM ('red', 'green', 'blue');

 If I want it to become ('red', 'green', 'blue', 'orange'), what am I to do?


ALTER TYPE colors ADD 'orange';

-- 
Thom Brown
Registered Linux user: #516935

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


Re: [HACKERS] [BUGS] BUG #5305: Postgres service stops when closing Windows session

2010-08-23 Thread Robert Haas
On Mon, Aug 23, 2010 at 11:37 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Mon, Aug 23, 2010 at 17:09, Tom Lane t...@sss.pgh.pa.us wrote:
 I would be inclined to write this off as Windows randomness that's
 unfixable on our end.  We could recommend that people take a closer
 look at what AV software they have installed and maybe try some other
 one.

 It may well be, but we can at least attempt to mitigate it, no?

 I'm not excited about a mitigation approach that introduces new
 data-loss hazards of its very own.  That doesn't meet the Less Evil
 standard in my eyes.

 [ thinks for a bit... ]  Although maybe it'd be all right to piggyback
 on the dead-man-switch code that already exists in pmsignal.c.  If the
 child process hasn't got as far as doing MarkPostmasterChildActive,
 then in principle it should be okay to assume it hasn't touched shared
 memory.  This really is independent of what exit code it returned.

I'm confused.  That seems like it would be LESS safe than the proposed
approach of taking a mutex just before mapping shared memory.  There
is some finite amount of code that executes after shared memory is
mapped and before MarkPostmasterChildActive executes; the advantage of
the mutex is that it can be taken BEFORE shared memory is mapped.  On
the other hand, if you think it's safe enough, it would certainly be
nice to use an existing mechanism rather than inventing something
totally new.

I agree that the exit code is irrelevant.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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


Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Bruce Momjian
Alvaro Herrera wrote:
 Excerpts from Tom Lane's message of lun ago 23 12:40:32 -0400 2010:
  Alvaro Herrera alvhe...@commandprompt.com writes:
   Excerpts from Bruce Momjian's message of dom ago 22 12:51:47 -0400 2010:
   Do you have a suggestion?  Reorder the items?
  
   I'd add another para before that one saying that this value also
   affects pg_clog truncation.  I agree that putting pg_clog truncation as
   the first item here is not an improvement.  For most people, having
   those pg_clog files there or not is going to be a wash, compared to data
   size.
  
  I was going to suggest that the point about pg_clog should be in a
  separate paragraph *after* this one, since it seems like a secondary
  issue.  But anyway, I agree with putting this para back as it was and
  talking about clog in a separate para.
 
 Sorry, yes, I was also thinking after.  I don't know what made me
 write before but it wasn't clarity of thought.

OK, I have attached a proposed patch to improve this.  I moved the
pg_clog mention to a new paragraph and linked it to the reason the
default is relatively low.

Comments?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.307
diff -c -c -r1.307 config.sgml
*** doc/src/sgml/config.sgml23 Aug 2010 02:43:25 -  1.307
--- doc/src/sgml/config.sgml23 Aug 2010 18:55:02 -
***
*** 4150,4161 
 para
  Specifies the maximum age (in transactions) that a table's
  structnamepg_class/.structfieldrelfrozenxid/ field can
! attain before a commandVACUUM/ operation is forced to allow 
removal
! of old files from the filenamepg_clog/ subdirectory and prevent
! transaction ID wraparound within the table.  Note that the system
! will launch autovacuum processes to prevent wraparound even when
! autovacuum is otherwise disabled.
! The default is 200 million transactions.
  This parameter can only be set at server start, but the setting
  can be reduced for individual tables by
  changing storage parameters.
--- 4150,4165 
 para
  Specifies the maximum age (in transactions) that a table's
  structnamepg_class/.structfieldrelfrozenxid/ field can
! attain before a commandVACUUM/ freeze operation is forced
! to prevent transaction ID wraparound within the table.
! Note that the system will launch autovacuum processes to
! prevent wraparound even when autovacuum is otherwise disabled.
!/para
! 
!para
! Vacuum freeze also allows removal of old files from the
! filenamepg_clog/ subdirectory, which is why the default
! is a relatively low 200 million transactions.
  This parameter can only be set at server start, but the setting
  can be reduced for individual tables by
  changing storage parameters.

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Josh Berkus

 If it's not there, no one will ever miss it.  You might as well argue
 that there should be a way of creating a foreign key reference by
 ALTER'ing the referenced table instead of the referencing table.
 Sure, if the SQL committee was into symmetry, they might have provided
 such a thing.  But they didn't and no one misses it.

That's a very different situation, since the relationship is not
symmetrical, and it would take far more than a single keyword.  Analogy
fail.

And one of the reasons people don't miss it is because far too many
users don't use FKs in the first place. ;-(  The only reason why users
wouldn't notice the absence of AFTER (or, more likely, try it and then
ask on IRC for error message diagnosis) is because they're not using the
feature.  (In which case it doesn't matter how it operates)

Docs which say Add new enums BEFORE the enum you want to add them to,
and if you need to add an enum at the end, then add it without the
BEFORE keyword is unnecessarily confusing to users.  Saying Add new
enum values using the BEFORE or AFTER keyword before or after the
appropriate value is vastly easier to understand.

I really don't see the value in making a command substantially less
intuitive in order to avoid a single keyword, unless it affects areas of
Postgres outside of this particular command.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.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] WIP: extensible enums

2010-08-23 Thread Tom Lane
Thom Brown t...@linux.com writes:
 On 23 August 2010 19:25, Joseph Adams joeyadams3.14...@gmail.com wrote:
 But what if you want to insert an OID at the end?

 ALTER TYPE colors ADD 'orange';

Alternatively, if people are dead set on symmetry, what we should do
to simplify is drop *this* syntax, and just have the BEFORE and AFTER
variants.

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] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 OK, I have attached a proposed patch to improve this.  I moved the
 pg_clog mention to a new paragraph and linked it to the reason the
 default is relatively low.

The references to vacuum freeze are incorrect; autovacuum does NOT
do the equivalent of VACUUM FREEZE.  Please stop playing around with
the perfectly good existing wording.

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] [9.1] pg_stat_get_backend_server_addr

2010-08-23 Thread Peter Eisentraut
On tor, 2010-05-27 at 22:32 +0300, Peter Eisentraut wrote:
 I suggest that we add the functions pg_stat_get_backend_server_addr
 and pg_stat_get_backend_server_port, but don't expose them in
 pg_stat_activity.  (_server_port is really mostly for symmetry,
 because you can't currently bind to multiple ports.)

I think I'm not going to pursue this patch anymore.  There hasn't been
any enthusiasm from anyone else, and if necessary the information can be
carved out of netstat.


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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Heikki Linnakangas

On 23/08/10 22:06, Tom Lane wrote:

Thom Brownt...@linux.com  writes:

On 23 August 2010 19:25, Joseph Adamsjoeyadams3.14...@gmail.com  wrote:

But what if you want to insert an OID at the end?



ALTER TYPE colors ADD 'orange';


Alternatively, if people are dead set on symmetry, what we should do
to simplify is drop *this* syntax, and just have the BEFORE and AFTER
variants.


Then you need to know the last existing value to add a new one to the 
end. Seems awkward.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Alvaro Herrera
Excerpts from Bruce Momjian's message of lun ago 23 14:55:55 -0400 2010:

 OK, I have attached a proposed patch to improve this.  I moved the
 pg_clog mention to a new paragraph and linked it to the reason the
 default is relatively low.
 
 Comments?

I think the new para doesn't make much sense, in context.  Why does it
say freeze?  How can we expect users to understand how that is
related to this parameter?

 --- 4150,4165 
  para
   Specifies the maximum age (in transactions) that a table's
   structnamepg_class/.structfieldrelfrozenxid/ field can
 ! attain before a commandVACUUM/ freeze operation is forced
 ! to prevent transaction ID wraparound within the table.
 ! Note that the system will launch autovacuum processes to
 ! prevent wraparound even when autovacuum is otherwise disabled.
 !/para
 ! 
 !para
 ! Vacuum freeze also allows removal of old files from the
 ! filenamepg_clog/ subdirectory, which is why the default
 ! is a relatively low 200 million transactions.
   This parameter can only be set at server start, but the setting
   can be reduced for individual tables by
   changing storage parameters.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes:
 I really don't see the value in making a command substantially less
 intuitive in order to avoid a single keyword, unless it affects areas of
 Postgres outside of this particular command.

It's the three variants to do two things that I find unintuitive.

As I mentioned a minute ago, dropping the abbreviated syntax and
just having BEFORE and AFTER would be a good way of achieving
symmetry if you find that important.

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] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  OK, I have attached a proposed patch to improve this.  I moved the
  pg_clog mention to a new paragraph and linked it to the reason the
  default is relatively low.
 
 The references to vacuum freeze are incorrect; autovacuum does NOT
 do the equivalent of VACUUM FREEZE.  Please stop playing around with
 the perfectly good existing wording.

Uh, so VACUUM FREEZE unconditionally freezes all rows, while vacuum just
freezes rows who's xid is older than vacuum_freeze_min_age?  I saw that
in our current docs in reference to VACUUM FREEZE:

Selects aggressive freezing of tuples. Specifying FREEZE is
equivalent to performing VACUUM with the vacuum_freeze_min_age
parameter set to zero. The FREEZE option is deprecated and
will be removed in a future release; set the parameter instead.

Updated patch attached.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.307
diff -c -c -r1.307 config.sgml
*** doc/src/sgml/config.sgml	23 Aug 2010 02:43:25 -	1.307
--- doc/src/sgml/config.sgml	23 Aug 2010 18:55:02 -
***
*** 4150,4161 
 para
  Specifies the maximum age (in transactions) that a table's
  structnamepg_class/.structfieldrelfrozenxid/ field can
! attain before a commandVACUUM/ operation is forced to allow removal
! of old files from the filenamepg_clog/ subdirectory and prevent
! transaction ID wraparound within the table.  Note that the system
! will launch autovacuum processes to prevent wraparound even when
! autovacuum is otherwise disabled.
! The default is 200 million transactions.
  This parameter can only be set at server start, but the setting
  can be reduced for individual tables by
  changing storage parameters.
--- 4150,4165 
 para
  Specifies the maximum age (in transactions) that a table's
  structnamepg_class/.structfieldrelfrozenxid/ field can
! attain before a commandVACUUM/ operation is forced
! to prevent transaction ID wraparound within the table.
! Note that the system will launch autovacuum processes to
! prevent wraparound even when autovacuum is otherwise disabled.
!/para
! 
!para
! Vacuum also allows removal of old files from the
! filenamepg_clog/ subdirectory, which is why the default
! is a relatively low 200 million transactions.
  This parameter can only be set at server start, but the setting
  can be reduced for individual tables by
  changing storage parameters.

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


Re: [HACKERS] Return of the Solaris vacuum polling problem -- anyone remember this?

2010-08-23 Thread Bruce Momjian
Alvaro Herrera wrote:
 Excerpts from Bruce Momjian's message of lun ago 23 14:55:55 -0400 2010:
 
  OK, I have attached a proposed patch to improve this.  I moved the
  pg_clog mention to a new paragraph and linked it to the reason the
  default is relatively low.
  
  Comments?
 
 I think the new para doesn't make much sense, in context.  Why does it
 say freeze?  How can we expect users to understand how that is
 related to this parameter?

I have removed the freeze mention per Tom's comment and posted an
updated version that removes the 'freeze' wording.  Are there other
changes needed?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

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

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Robert Haas
On Mon, Aug 23, 2010 at 3:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Thom Brown t...@linux.com writes:
 On 23 August 2010 19:25, Joseph Adams joeyadams3.14...@gmail.com wrote:
 But what if you want to insert an OID at the end?

 ALTER TYPE colors ADD 'orange';

 Alternatively, if people are dead set on symmetry, what we should do
 to simplify is drop *this* syntax, and just have the BEFORE and AFTER
 variants.

FWIW, I think Andrew's originally proposed syntax is fine and useful,
and we should just go with it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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


Re: [HACKERS] [BUGS] BUG #5305: Postgres service stops when closing Windows session

2010-08-23 Thread Cristian Bittel
From the users point of view, this could be a Windows or AV issue, but just
stops Postgres service, does not affect or interfire on Windows stability or
AV stability, instead it affect your product. So if you can improve the
stability of the service (and data integrity at the most) it could be a
benefic for all.

I've found the same behavior on Postgres service when clossing MSTSC session
without any AV installed, and after some months of Postgres crashes,
administrators installed Kaspersky for Servers AV, and crashes are still
there.

 Cristian.


2010/8/23 Tom Lane t...@sss.pgh.pa.us

 Magnus Hagander mag...@hagander.net writes:
  On Mon, Aug 23, 2010 at 17:09, Tom Lane t...@sss.pgh.pa.us wrote:
  I would be inclined to write this off as Windows randomness that's
  unfixable on our end.  We could recommend that people take a closer
  look at what AV software they have installed and maybe try some other
  one.

  It may well be, but we can at least attempt to mitigate it, no?

 I'm not excited about a mitigation approach that introduces new
 data-loss hazards of its very own.  That doesn't meet the Less Evil
 standard in my eyes.

 [ thinks for a bit... ]  Although maybe it'd be all right to piggyback
 on the dead-man-switch code that already exists in pmsignal.c.  If the
 child process hasn't got as far as doing MarkPostmasterChildActive,
 then in principle it should be okay to assume it hasn't touched shared
 memory.  This really is independent of what exit code it returned.

regards, tom lane



Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Andrew Dunstan
On Mon, August 23, 2010 3:20 pm, Heikki Linnakangas wrote:
 On 23/08/10 22:06, Tom Lane wrote:
 Thom Brownt...@linux.com  writes:
 On 23 August 2010 19:25, Joseph Adamsjoeyadams3.14...@gmail.com
 wrote:
 But what if you want to insert an OID at the end?

 ALTER TYPE colors ADD 'orange';

 Alternatively, if people are dead set on symmetry, what we should do
 to simplify is drop *this* syntax, and just have the BEFORE and AFTER
 variants.

 Then you need to know the last existing value to add a new one to the
 end. Seems awkward.


I agree. This is a non-starter, I think. The most common case in my
experience is where the user doesn't care at all about the order, and just
wants to add a new label. We should make that as easy as possible,
especially since it's the most efficient.

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: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-23 Thread Heikki Linnakangas

On 20/08/10 17:28, Tom Lane wrote:

[ It's way past time to change the thread title ]

Heikki Linnakangasheikki.linnakan...@enterprisedb.com  writes:

On 20/08/10 16:24, Tom Lane wrote:

You keep on proposing solutions that only work for walsender :-(.



Well yes, the other places where we use pg_usleep() are not really a
problem as is.


Well, yes they are.  They cause unnecessary process wakeups and thereby
consume cycles even when the database is idle.  See for example a
longstanding complaint here:
https://bugzilla.redhat.com/show_bug.cgi?id=252129

If we're going to go to the trouble of having a mechanism like this,
I'd like it to fix that problem so I can close out that bug.


Hmm, if you want to put bgwriter and walwriter to deep sleep, then 
someone will need to wake them up when they have work to do. Currently 
they poll. Maybe they should just sleep longer, like 10 seconds, if 
there hasn't been any work to do in the last X wakeups.


We've been designing the new sleep facility so that the event that wakes 
up the sleep is sent from the signal handler in the same process, but it 
seems that all the potential users would actually want to be woken up 
from *another* process, so the signal handler seems like an unnecessary 
middleman. Particularly on Windows where signals are simulated with 
pipes and threads, while you could just send a Windows event directly 
from one process to another.


A common feature that all the users of this facility want is that once 
the event is sent, re-sending it is a fast no-op until re-enabled by the 
receiver. For example, if we need backends to wake up bgwriter after 
dirtying a buffer, you don't want to waste many cycles determining that 
bgwriter is already active and doesn't need to be woken up.


Let's call these latches. I'm thinking of something like this:

/* Similar to LWLockId */
typedef enum
{
  BgwriterLatch,
  WalwriterLatch,
  /* plus one for each walsender */
} LatchId;

/*
 * Wait for given latch to be set. Only one process can wait
 * for a given latch at a time.
 */
WaitLatch(LatchId latch, long timeout);

/*
 * Sets latch. Returns quickly if the latch is set already.
 */
SetLatch(LatchId latch);

/*
 * Clear the latch. Calling WaitLatch after this will sleep, unless
 * the latch is set again before the WaitLatch call.
 */
ResetLatch(LatchId latch);

There would be a boolean for each latch in shared memory, to indicate if 
the latch is armed, allowing quick return from SetLatch if the latch 
is already set. Plus a signal to wake up the waiting process (maybe use 
procsignal.c), and the self-pipe trick within the receiving process to 
make it race condition free. On Windows, the signal and the self-pipe 
trick are replaced with Windows events.


I'll try out this approach tomorrow..

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread David E. Wheeler
On Aug 23, 2010, at 12:20 PM, Tom Lane wrote:

 Josh Berkus j...@agliodbs.com writes:
 I really don't see the value in making a command substantially less
 intuitive in order to avoid a single keyword, unless it affects areas of
 Postgres outside of this particular command.
 
 It's the three variants to do two things that I find unintuitive.

I strongly suspect that you are in the minority on this one.

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


[HACKERS] Problem Using PQcancel in a Synchronous Query

2010-08-23 Thread Eric Simon
Hi,

I am using Perl (DBD::Pg) with Postgres.  DBD::Pg already fully supports
asynchronous queries, and the canceling of those queries, but I need to
cancel a synchronous query.  So to do that, I am trying to write
$sth-cancel() support for DBD::Pg.  Here's my example in Perl:

my $aborted = 0;
my $action = POSIX::SigAction-new(
sub {$sth-cancel; $aborted = 1},
POSIX::SigSet-new(SIGALRM)
);
my $oldaction = POSIX::SigAction-new;
POSIX::sigaction(SIGALRM,$action,$oldaction);

my $sth = $dbh-prepare('SELECT id FROM user_session FOR UPDATE');
alarm(10); # wait 10 seconds before time out
$sth-execute;
alarm(0); # cancel alarm (if execute worked fast)

POSIX::sigaction(SIGALRM,$oldaction); # restore original handler
warn warning: query timed out if $aborted;

Now that I've established some context, here's where I'm at: I've written
$sth-cancel() for DBD::Pg using PQcancel(), and it works (it returns the
status 57014: QUERY CANCELED).  The problem is that the $sth-execute call
(which resides between the two alarm() calls above) doesn't continue on, but
rather stays frozen, waiting for data.  Does PQcancel not communicate back
to the execute statement so that it unblocks?

-- 
Eric Simon
The IQ Group, Inc.


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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Josh Berkus
On 8/23/10 12:20 PM, Tom Lane wrote:
 Josh Berkus j...@agliodbs.com writes:
 I really don't see the value in making a command substantially less
 intuitive in order to avoid a single keyword, unless it affects areas of
 Postgres outside of this particular command.
 
 It's the three variants to do two things that I find unintuitive.

Actually, it's 3 different things:

1. BEFORE adds a value before the value cited.
2. AFTER adds a value after the value cited.
3. unqualified adds a value at the end.

The fact that AFTER allows you to add a value at the end is
circumstantial overlap.  While executing an AFTER, you wouldn't *know*
that you were adding it to the end, necessarily.

The other reason to have AFTER is that, in scripts, the user may not
have the before value handy due to context (i.e. dynamically building an
enum).

Anyway, this'll still be useful with BEFORE only.  I'm just convinced
that we'll end up adding AFTER in 9.2 or 9.3 after we get a bunch of
user complaints and questions.  So why not add it now?

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.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: [Glue] [HACKERS] Deadlock bug

2010-08-23 Thread Josh Berkus
Kevin,

 In the for what it's worth department, I tried out the current
 Serializable Snapshot Isolation (SSI) patch with this test case at
 the SERIALIZABLE transaction isolation level.  Rather than defining
 a foreign key, I ran the queries which an SSI implementation in a
 SERIALIZABLE-only environment would -- that didn't use FOR SHARE or
 FOR UPDATE.  Not surprisingly, the behavior was the same up to the
 second UPDATE on Process 2, at which point there was no deadlock. 
 Process 2 was able to commit, at which point Process 1 failed with:
  
 ERROR:  could not serialize access due to concurrent update

Does this happen immediately, not waiting 2 seconds for deadlock checking?

 If you have other examples of user-hostile behaviors you want to
 share, I can see how they would behave under an SSI implementation.
 I can almost guarantee that you won't see deadlocks, although you
 will likely see more overall rollbacks in many transaction mixes.

They'd be more variations of this same theme; transactions updating each
other's FKs.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.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] WIP: extensible enums

2010-08-23 Thread Bruce Momjian
Josh Berkus wrote:
 On 8/23/10 12:20 PM, Tom Lane wrote:
  Josh Berkus j...@agliodbs.com writes:
  I really don't see the value in making a command substantially less
  intuitive in order to avoid a single keyword, unless it affects areas of
  Postgres outside of this particular command.
  
  It's the three variants to do two things that I find unintuitive.
 
 Actually, it's 3 different things:
 
 1. BEFORE adds a value before the value cited.
 2. AFTER adds a value after the value cited.
 3. unqualified adds a value at the end.
 
 The fact that AFTER allows you to add a value at the end is
 circumstantial overlap.  While executing an AFTER, you wouldn't *know*
 that you were adding it to the end, necessarily.
 
 The other reason to have AFTER is that, in scripts, the user may not
 have the before value handy due to context (i.e. dynamically building an
 enum).
 
 Anyway, this'll still be useful with BEFORE only.  I'm just convinced
 that we'll end up adding AFTER in 9.2 or 9.3 after we get a bunch of
 user complaints and questions.  So why not add it now?

CREATE ENUM in PG 9.0 allows you to create an enum with no columns,
e.g.:

test= CREATE TYPE etest AS ENUM ();
CREATE TYPE

so I think we have to have the ability add an enum without a
before/after.  This ability was added for pg_upgrade.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

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

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


Re: [HACKERS] WIP: extensible enums

2010-08-23 Thread Bruce Momjian
Andrew Dunstan wrote:
 
 Attached is a WIP patch that allows enums to be extended with additional 
 labels arbitrarily. As previously discussed, it works by adding an 
 explicit sort order column to pg_enum. It keeps track of whether the 
 labels are correctly sorted by oid value, and if so uses that for 
 comparison, so the possible performance impact on existing uses, and on 
 almost all cases where a label is added at the end of the list, should 
 be negligible.
 
 Open items include
 
 * some additional error checking required
 * missing documentation
 * pg_upgrade considerations

I looked at the pg_upgrade ramifications of this change and it seems
some adjustments will have to be made.  Right now pg_upgrade creates an
empty enum type:

CREATE TYPE etest AS ENUM ();

and then it calls EnumValuesCreate() to create the enum labels. 
EnumValuesCreate() is called from within DefineEnum() where the enum
type is created, and that assumes the enums are always created initially
sorted.  That would not be true when pg_upgrade is calling
EnumValuesCreate() directly with oid assignment as part of an upgrade.

I think the cleanest solution would be to modify pg_dump.c so that it
creates an empty ENUM type like before, but uses the new ALTER TYPE
myenum ADD 'newlabel' syntax to add the enum labels (with oid assignment
like we do for CREATE TYPE, etc.)  The existing code had to hack to call
EnumValuesCreate() but with this new syntax it will no longer be
necessary. The call to EnumValuesCreate() for enums is the only time
pg_upgrade_support calls into a function rather than just assigning an
oid to a global variable, so it would be great to remove that last
direct function call usage.

Does this code handle the case where CREATE ENUM oid wraps around while
the enum label oids are being assigned?  Does our existing code handle
that case?

I also noticed you grab an oid for the new type using the oid counter
without trying to make it in sorted order.  Seems that would be possible
for adding enums to the end of the list, and might not be worth it.  A
quick hack might be to just try of an oid+1 from the last enum and see
if that causes a conflict with the pg_enum.oid index.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

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

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


Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-23 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 On 20/08/10 17:28, Tom Lane wrote:
 Well, yes they are.  They cause unnecessary process wakeups and thereby
 consume cycles even when the database is idle.  See for example a
 longstanding complaint here:
 https://bugzilla.redhat.com/show_bug.cgi?id=252129
 
 If we're going to go to the trouble of having a mechanism like this,
 I'd like it to fix that problem so I can close out that bug.

 Hmm, if you want to put bgwriter and walwriter to deep sleep, then 
 someone will need to wake them up when they have work to do. Currently 
 they poll. Maybe they should just sleep longer, like 10 seconds, if 
 there hasn't been any work to do in the last X wakeups.

I should probably clarify that I don't expect this patch to solve that
problem all by itself.  But ISTM that a guaranteed-interruptible version
of pg_usleep is a necessary prerequisite before we can even think about
dialing down the database's CPU consumption at idle.

 We've been designing the new sleep facility so that the event that wakes 
 up the sleep is sent from the signal handler in the same process, but it 
 seems that all the potential users would actually want to be woken up 
 from *another* process, so the signal handler seems like an unnecessary 
 middleman.

No, because there are also lots of cases where the signal is arriving
from a non-Postgres process; SIGTERM arriving from init being the killer
case that you simply don't get to propose an alternative design for.
More generally, I'm uninterested in decoupling cases like SIGHUP for
postgresql.conf change from the existing signal mechanism, because it's
just too useful to be able to trigger that externally.

 Particularly on Windows where signals are simulated with 
 pipes and threads, while you could just send a Windows event directly 
 from one process to another.

Windows of course has different constraints, and in particular it lacks
the constraint that we must be able to respond to system-defined
signals.  So I wouldn't object to the underlying implementation being
different for Windows.

 [ latch proposal ]

This seems reasonably clean as far as signal conditions generated
internally to Postgres go, but I remain unclear on how it helps for
response to actual signals.

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] WIP: extensible enums

2010-08-23 Thread Tom Lane
David E. Wheeler da...@kineticode.com writes:
 On Aug 23, 2010, at 12:20 PM, Tom Lane wrote:
 It's the three variants to do two things that I find unintuitive.

 I strongly suspect that you are in the minority on this one.

Yeah, seems like I'm losing the argument.  Oh well.

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] Problem Using PQcancel in a Synchronous Query

2010-08-23 Thread Tom Lane
Eric Simon esi...@theiqgroup.com writes:
 Now that I've established some context, here's where I'm at: I've written
 $sth-cancel() for DBD::Pg using PQcancel(), and it works (it returns the
 status 57014: QUERY CANCELED).  The problem is that the $sth-execute call
 (which resides between the two alarm() calls above) doesn't continue on, but
 rather stays frozen, waiting for data.  Does PQcancel not communicate back
 to the execute statement so that it unblocks?

Um ... PQcancel returns no such thing, only true or false.  I'm guessing
you've coded your signal handler in such a way that it eats the query
result message intended for the mainline execute code.  You should not
be calling anything except PQcancel itself in the signal handler.

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] WIP: extensible enums

2010-08-23 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 I also noticed you grab an oid for the new type using the oid counter
 without trying to make it in sorted order.  Seems that would be possible
 for adding enums to the end of the list, and might not be worth it.  A
 quick hack might be to just try of an oid+1 from the last enum and see
 if that causes a conflict with the pg_enum.oid index.

That wouldn't be race-condition-safe.

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: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)

2010-08-23 Thread Alvaro Herrera
Excerpts from Tom Lane's message of lun ago 23 19:44:02 -0400 2010:
 Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:

  [ latch proposal ]
 
 This seems reasonably clean as far as signal conditions generated
 internally to Postgres go, but I remain unclear on how it helps for
 response to actual signals.

This could probably replace the signalling between postmaster and
autovac launcher, as well.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


[HACKERS] Typing Records

2010-08-23 Thread David E. Wheeler
Hackers,

I've been trying to come up with a simpler way to iterate over a series of 
values in pgTAP tests than by creating a table, inserting rows, and then 
selecting from the table. The best I've come up with so far is:

CREATE TYPE vcmp AS ( lv semver, op text, rv semver);

SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
ROW('1.2.2',  '=', '1.2.2')::vcmp,
ROW('1.2.23', '=', '1.2.23')::vcmp
]);

Not bad, but I was hoping that I could cast all the rows at once, without the 
type, like so:

SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
ROW('1.2.2',  '=', '1.2.2'),
ROW('1.2.23', '=', '1.2.23')
]) AS f(lv semver, op text, rv semver);

But this gives me an error (9.0b4):

psql:t/types.pg:205: ERROR:  function return row and query-specified return row 
do not match
DETAIL:  Returned type unknown at ordinal position 1, but query expects semver.

Pity it doesn't default to casting to text there. So I tried to just cast the 
first row:

SELECT cmp_ok(lv, op, rv) FROM unnest(ARRAY[
ROW('1.2.2'::semver,  '='::text, '1.2.2'::semver),
ROW('1.2.23', '=', '1.2.23')
]) AS f(lv semver, op text, rv semver);

That's even worse!

psql:t/types.pg:205: ERROR:  invalid memory alloc request size 
18446744071604011012

Wha??

That seems like a bug.

Aside from that, might there be another way to do this without an explicit 
composite type? Maybe with VALUES() or something?

Thanks,

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] security hook on authorization

2010-08-23 Thread KaiGai Kohei
(2010/08/22 21:38), KaiGai Kohei wrote:
 (2010/08/22 0:20), Robert Haas wrote:
 On Aug 20, 2010, at 8:27 PM, KaiGai Koheikai...@kaigai.gr.jp wrote:
 (2010/08/20 23:34), Robert Haas wrote:
 2010/8/19 KaiGai Koheikai...@ak.jp.nec.com:
 I think our standard criteria for the inclusion of hooks is that you
 must demonstrate that the hook can be used to do something interesting
 that couldn't be done without the hook. So far I'm unconvinced.

 We cannot handle an error of labeled networking (getpeercon(3)),
 if we don't have any hook during client authorization stage.

 If and when a connection came from a host but we don't accept the
 delivered security label, or labeled networking is misconfigured,
 getpeercon(3) returns NULL. In this case, server cannot identify
 what label should be applied on the client, then, we should
 disconnect this connection due to the error on database login,
 not any access control decision.

 In similar case, psm_selinux.so disconnect the connection when
 it cannot identify what security label shall be assigned on the
 session, due to some reasons such as misconfigurations.

 Without any hooks at authorization stage (but it might be different
 place from this patch, of course), we need to delay the error
 handling by the time when SE-PostgreSQL module is invoked at first.
 But it is already connection established and user sends a query.
 It seems to me quite strange behavior.

 You mentioned that before. I'm not totally sure I buy it, and I think
 there are other applications that might benefit from a hook in this area.
 We need to think about trying to do this in a way that is as general as
 possible. So I'd like to see some analysis of other possible 
 applications.

 Yes, I also think this kind of authorization hook should benefit other
 applications, not only label based mac features.
 
 For example, something like 'last' command in operations system which
 records username and login-time. Stephen mentioned pam_tally that locks
 down certain accounts who failed authentication too much.
 Perhaps, PAM modules in operating system give us some hints about other
 possible applications.
 

I've checked some documentation files of pam modules in operating system
to think about other possible applications.

* pam_env.so
It allows to set/unset environment variables, perhaps, per users.
In PG, we may be able to assume a module which set/unset guc variables
depending on authenticated user?

* pam_faildelay.so
It enables to delay to disconnect when authentication was failed.
It prevents brute-force attack on passwords.

* pam_lastlog.so
It enables to display a line of information about the last login of
the user. In addition, the module maintains the /var/log/lastlog file.

* pam_selinux.so
It sets up the default security context for the next execed shell.
It is equivalent to set up a set of privileges of the authenticated
user.

* pam_tally.so
It maintains a count of attempted accesses, can reset count on success,
can deny access if too many attempts fail.


If and when we try to provide something similar features of them,
the pam_env.so, pam_lastlog.so and pam_selinux.so need to be called
on the code path of authentication succeeded only.
But the pam_faildelay.so needs to be called on authentication failed
path, and the pam_tally.so needs to be called on both paths because
it maintain a count of authentication failed and locks down certain
user accounts which failed too many.

In the current patch, I put the authorization hook on SetSessionUserId()
but it is only called when authentication succeeded path.

Here is only one place where we can put the authorization hook where
is called on both of authentication succeeded and failed.

The ClientAuthentication() has a big switch statement which branches
to each authentication methods, then status will be updated to
STATUS_OK or others.
How about the security hook just after the big switch statement but
before sending a response to the client, as follows?

  void
  ClientAuthentication(Port *port)
  {
  int status = STATUS_ERROR;
:
  /*
   * Now proceed to do the actual authentication check
   */
  switch (port-hba-auth_method)
  {
  case uaReject:
:
  }

+ if (ClientAuthenticationHook)
+ status = (*ClientAuthenticationHook)(port, status);

  if (status == STATUS_OK)
  sendAuthRequest(port, AUTH_REQ_OK);
  else
  auth_failed(port, status);

  /* Done with authentication, so we should turn off immediate interrupts */
  ImmediateInterruptOK = false;
  }


Thanks,
-- 
KaiGai Kohei kai...@ak.jp.nec.com

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


[HACKERS] INSERT and parentheses

2010-08-23 Thread igor polishchuk
Marko et al,
This is my first ever attempt of a patch review just for learning the
procedure. I'm not a postgres developer, so the review is partial and mostly
from the usability prospective.
The original message id of the patch is
4bd58db3.4070...@cs.helsinki.fihttp://archives.postgresql.org/pgsql-hackers/2010-04/msg01200.php


Submssion review
=

* Is he patch in context diff format?
Ys

* Dos it apply cleanly to the current CVS HEAD?
Applies cleanly to a source code snapshot

* Dos it include reasonable tests, necessary doc patches, etc?
I does not require a doc patch. A test is not included, but it looks pretty
trivial:

-- Prpare the test tables

drop table if exists foo;
drop table if exists boo;

crate table foo(
a nt,
b nt,
c nt);

crate table boo(
a nt,
b nt,
c nt);

insert into boo values (10,20,30);

-- Actual test

INSERT INTO foo(a,b,c) SELECT (a,b,c) FROM boo;
INSERT INTO foo(a,b,c) VALUES((0,1,2));

Usability Review
=

The patch provides a HINT for unclear error. This should clarify for a user
what exactly is wrong with the sql.
However, the actual HINT text provided with the patch is not very clear,
too.
The Stephen Frost's suggestion would add clarity:

errhint(insert appears to be a single column with a record-type rather than
multiple columns of non-composite type.),


Feature test
=

The feature works as advertised for the test provided above.
No failures or crashes.
No visible affect on performance