Re: [HACKERS] betatesting: ERROR: failed to build any 2-way joins on 9.2

[Pavel Stehule]

2012/8/15 Tom Lane :
> Pavel Stehule  writes:
>> My colleague found a issue on 9.2 - sorry for query formatting - this
>> query is generated from ours query engine
>
> Fixed, thanks for the report.

thank you

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] [COMMITTERS] pgsql: Revert "commit_delay" change; just add comment that we don't hav

[Tom Lane]

Peter Geoghegan  writes:
> On 14 August 2012 21:26, Bruce Momjian  wrote:
>> Revert "commit_delay" change; just add comment that we don't have
>> a microsecond specification.

> I think that if we eventually decide to change the name of
> commit_delay for 9.3 (you previously suggested that that might be
> revisited), it will be reasonable to have the new GUC in units of
> milliseconds.

Well, the reason why it's like that at all is the thought that values
of less than 1 millisecond might be useful.  Are we prepared to suppose
that that is not and never will be true?

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] betatesting: ERROR: failed to build any 2-way joins on 9.2

[Tom Lane]

Pavel Stehule  writes:
> My colleague found a issue on 9.2 - sorry for query formatting - this
> query is generated from ours query engine

Fixed, thanks for the report.

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] Re: [HACKERS] PL/Perl build problem: error: ‘OP_SETSTATE’ undeclared

[Peter Eisentraut]

On Tue, 2012-08-14 at 20:58 -0600, Alex Hunsaker wrote:
> I know i've used 5.14 in the past successfully.  Wat happens if you
> regenerate plperl_opmask.h? (rm plperl_opmask.h && make)

Yeah, it seems to have something to do with a perl upgrade happening
between builds.  It was fixed by building from scratch.



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

Re: [HACKERS] -Wformat-zero-length

[Peter Eisentraut]

On Tue, 2012-08-14 at 17:56 -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > On 8/10/12 7:48 PM, Dimitri Fontaine wrote:
> >> What about having single user mode talk fe/be protocol, and talk to it via 
> >> a UNIX pipe, with pg_upgrade starting the single user backend as a 
> >> subprocess?
> 
> > I think that's essentially equivalent to starting the server on a 
> > Unix-domain socket in a private directory.  But that has been rejected 
> > because it doesn't work on Windows.
> 
> > The question in my mind is, is there some other usable way on Windows 
> > for two unrelated processes to communicate over file descriptors in a 
> > private and secure way?
> 
> You're making this unnecessarily hard, because there is no need for the
> two processes to be unrelated.
> 
> The implementation I'm visualizing is that a would-be client (think psql
> or pg_dump, though the code would actually be in libpq) forks off a
> process that becomes a standalone backend, and then they communicate
> over a pair of pipes that were created before forking.  This is
> implementable on any platform that supports Postgres, because initdb
> already relies on equivalent capabilities.

Well, that would be an interesting feature, but it's debatable which
among this or just adding a new socket type (if available) is harder.



-- 
Sent 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 patch for consolidating misplaced-aggregate checks

[Peter Eisentraut]

On Tue, 2012-08-14 at 11:30 -0400, Alvaro Herrera wrote:
> And so on (there are several more).  Note that here we use "check
> constraint" without any capitalization.  However this doesn't
> translate
> too well as is; I mean, if I were to translate "check" into its
> equivalent spanish word, I'm sure to cause a great deal of confusion.
> So I've opted for putting the check word, verbatim, in quotes; for
> example:
> 
> msgid "check constraint \"%s\" already exists"
> msgstr "la restricción «check» «%s» ya existe"
> 
> However this is also a bit ugly because I now have two sets of quoted
> words -- check itself and then the constraint name. 

I can't really advise you with the language issue, but I think just
writing check without quotes would be fine here.  I also find it useful
on occasion to consult existing (non-PostgreSQL, non-MySQL) books about
databases in the respective language for typical translations.



-- 
Sent 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 patch for consolidating misplaced-aggregate checks

[Peter Eisentraut]

On Tue, 2012-08-14 at 12:04 -0400, Tom Lane wrote:
> Alvaro Herrera  writes:
> > Speaking of english words, I was wondering at "check" the other day.
> > For example, we have
> 
> > #: catalog/heap.c:2171
> > #, c-format
> > msgid "check constraint \"%s\" already exists"
> 
> > #: catalog/heap.c:2534
> > #, c-format
> > msgid "only table \"%s\" can be referenced in check constraint"
> 
> > And so on (there are several more).  Note that here we use "check
> > constraint" without any capitalization.
> 
> FWIW, I think I changed "check" to "CHECK" in a couple of messages
> recently, for exactly the reason that it seemed to be used in its
> keyword meaning rather than as plain English text.  Perhaps we
> should just go around and do that consistently.

I'm not in favor of that.  "Check constraint" is a database term that
exists outside of SQL, just like "primary key", for instance.  You
wouldn't write the latter in all upper case everywhere, I think.



-- 
Sent 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 patch for consolidating misplaced-aggregate checks

[Peter Eisentraut]

On Fri, 2012-08-10 at 17:57 +0100, Greg Stark wrote:
> On Thu, Aug 9, 2012 at 5:40 PM, Tom Lane  wrote:
> > Fair enough.  I was not sold on doing that either.  I would still like
> > to know if it's okay to use one string with %s for the cases where
> > there's not a good reason for the "context" to be more than just a
> > SQL keyword.
> 
> Given that the SQL keyword is going to be an English word I can't
> imagine how this could be a big deal for translators. It might not
> match gender or case or something but only if the reader is
> automatically mentally translating the keyword into their language and
> then applying that language's rules to it. At least to me it makes
> sense to refer to "VALUES" as a singular noun or "LIMIT" as a generic
> male noun even though "limitation" would be feminine (I had to look
> that one up though so perhaps I'm not the best person to judge).

In some languages the grammatical adjustments do not depend on the
gender but on other aspects of the word, such as the last letter.  So
it's possible to construct cases where this is a problem.

That said, you can usually work around this by translating something
like "cannot use aggregates with %s" along the lines of "cannot use
aggregates in context %s".



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

Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

[Kevin Grittner]

> "Kevin Grittner" wrote:
> Heikki Linnakangas wrote:
>> On 14.08.2012 14:25, Kevin Grittner wrote:

Attached is version 3.

>>> Oh, further testing this morning shows that while *queries* on
>>> the HS seem OK, streaming replication is now broken. I probably
>>> need to override transaction isolation on the recovery process.
>>> I'll take a look at that.
>>
>> Hmm, seems to work for me. Do you get an unexpected error or what?
>
> No, I wasn't getting errors in the clients or the logs, but I
> wasn't seeing data pop up on the replica when I expected. Perhaps I
> messed up my streaming replication configuration somehow.

Yeah, setup error. I accidentally dropped the primary_conninfo
setting from my recovery.conf file. Now that I've put that back, it
works just fine.

>> I didn't, I meant to point out that you can set
>> transaction_isolation just for the one transaction. But your
>> suggested hint is OK as well - you suggest to set it for the whole
>> session, which will also work around the problem. The "before the
>> first query in the transaction" isn't necessary in that case
>> though.

>> Yet another option is to suggest the "BEGIN ISOLATION LEVEL
>> REPEATABLE READ" syntax, instead of SET.
>
> I'm inclined toward hinting at a session override of the default.
> If you're typing away in psql, that's a lot less work. :-)

I tinkered with the messages (including correcting a reverse-logic
bug in which was displayed under what circumstances) and tweaked a
comment. I struggled with how to capitalize and quote. Let me know
what you think.

> Since the existing behavior is so bad, I'm inclined to think
> this merits backpatching to 9.1. Thoughts on that?

 Yes, we have to somehow fix the crash and the assertion failure
 on 9.1.
>>>
>>> Should the check_transaction_read_only() stuff be back-patched to
>>> 9.1, too? So far as we know, that's fragile, not broken, right?
>>> Could the fix you envision there cause a behavioral change that
>>> could break anything that users might have in place?
>>
>> Good question. I don't see how it could cause a behavioral change,
>> but I've been wrong before.
>
> If we don't know of any actual existing bugs I'm inclined to not
> back-patch that part to 9.1, although it makes sense for 9.2 since
> we shouldn't be risking breakage of any production systems. I'm
> really cautious about giving anybody any excuse not to apply a
> minor update.

How about we fix the serializable versus HS & Windows bugs in one
patch, and then look at the other as a separate patch? If that's OK,
I think this is ready, unless my message text can be improved. (And
I will have a shot at my first back-patching)

-Kevin




hotstandby-serializable-3.patch
Description: Binary data

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

Re: [HACKERS] sha1, sha2 functions into core?

[Bruce Momjian]

Is there a TODO here?

---

On Wed, Aug 10, 2011 at 09:43:18PM +0300, Peter Eisentraut wrote:
> On ons, 2011-08-10 at 19:29 +0100, Dave Page wrote:
> > On Wed, Aug 10, 2011 at 7:06 PM, Peter Eisentraut  wrote:
> > > I would like to see whether there is support for adding sha1 and sha2
> > > functions into the core.  These are obviously well-known and widely used
> > > functions, but currently the only way to get them is either through
> > > pgcrypto or one of the PLs.  We could say that's OK, but then we do
> > > support md5 in core, which then encourages people to use that, when they
> > > really shouldn't use that for new applications.
> > 
> > Slightly different, but related - I've seen complaints that we only
> > use md5 for password storage/transmission, which is apparently not
> > acceptable under some government security standards. In the most
> > recent case, they wanted to be able to use sha256 for password storage
> > (transmission isn't really an issue where SSL can be used of course).
> 
> Yeah, that's one of those things.  These days, using md5 for anything
> raises red flags, so it would be better to slowly move some alternatives
> into place.
> 
> > If we're ready to move more hashing functions into core, then it seems
> > reasonable to add more options for password storage to help those who
> > need to meet mandated standards.
> 
> Yes, that would be good.
> 
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

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

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


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

Re: [HACKERS] -Wformat-zero-length

[Bruce Momjian]

On Tue, Aug 14, 2012 at 06:53:49PM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Tue, Aug 14, 2012 at 05:56:39PM -0400, Tom Lane wrote:
> >> The implementation I'm visualizing is that a would-be client (think psql
> >> or pg_dump, though the code would actually be in libpq) forks off a
> >> process that becomes a standalone backend, and then they communicate
> >> over a pair of pipes that were created before forking.  This is
> >> implementable on any platform that supports Postgres, because initdb
> >> already relies on equivalent capabilities.
> 
> > I think the big question is whether we need to modify every binary that
> > pg_upgrade executes to understand this pipe communication method.
> 
> I think we can fix it once in libpq and we're done.  It'd be driven
> by some new connection-string option, and the clients as such would
> never need to know that they're not talking to a regular postmaster.

OK, I like the idea if we can make it work.  I am unclear how we would
pass this connection thing.  Peter's idea of putting the socket in the
current directory is great, except for Windows support.  Maybe we
should just implement this for non-Windows.

FYI, we only use new binaries, so we would only need the support in the
new libpq client libraries.  However, we can't backpatch this into the
old servers, so I am concerned it will not be possible to implement
this if it requires server changes.

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

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


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

[HACKERS] Re: [HACKERS] PL/Perl build problem: error: ‘OP_SETSTATE’ undeclared

[Alex Hunsaker]

On Sun, Aug 12, 2012 at 9:57 PM, Peter Eisentraut  wrote:

> It appears that a recent Perl version (I have 5.14.2) has eliminated
> OP_SETSTATE, which causes the current PostgreSQL build to fail:
>
> plperl.c: In function ‘_PG_init’:
> plperl.c:442:5645: error: ‘OP_SETSTATE’ undeclared (first use in this
> function)
> plperl.c:442:5645: note: each undeclared identifier is reported only once
> for each function it appears in
>

Hrm, Thats strange, PLPERL_SET_OPMASK() is generated by plperl_opmask.pl--
that should use whatever OP's your current perl has defined (They come from
the "use Opcode" at the top and unless you somehow installed a different
Opcode module than what your perl has...).

Im running a non threaded 5.16.0 and I don't see OP_SETSTATE anywhere:
$ pwd
/home/alex/src/postgresql/src/pl/plperl
$ grep -RI 'OP_SETSTATE' *
$

I know i've used 5.14 in the past successfully.  Wat happens if you
regenerate plperl_opmask.h? (rm plperl_opmask.h && make)

Re: [HACKERS] TRUE/FALSE vs true/false

[Peter Eisentraut]

On Tue, 2012-08-14 at 17:36 -0400, Bruce Momjian wrote:
> On Tue, Aug 14, 2012 at 05:34:02PM -0400, Tom Lane wrote:
> > Bruce Momjian  writes:
> > > On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
> > >> On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
> > >>> I meant a mass "sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g'" run
> > >>> so all the ~200 occurrences of both "TRUE" and "FALSE" get
> > >>> converted so the whole source tree is consistent.
> > 
> > >> I would be in favor of that.
> > 
> > > I have implemented this with the patch at:
> > >   http://momjian.us/expire/true_diff.txt
> > 
> > Does this really do anything for us that will justify the extra
> > back-patching pain it will cause?  I don't see that it's improving
> > code readability any.
> 
> I think it is more of a consistency issue.  There were multiple people
> who wanted this change.  Of course, some of those people don't backport
> stuff.

I guess you could argue this particular case without end, but I think we
should be open to these kinds of changes.  They will make future code
easier to deal with and confuse new developers less (when to use which,
do they mean different things, etc.).

If back-patching really becomes a problem, we might want to look a
little deeper into git options.  I think the Linux kernel people do
these kinds of cleanups more often, so there is probably some better
support for it.



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

Re: [HACKERS] superusers are members of all roles?

[Andrew Dunstan]

On 08/14/2012 05:03 PM, Michael Braun wrote:

Hi,

I've just recently upgraded to postgrsql 9.1 and also hit bug #5763.
Having +group not match all superusers is essential to be able to assign
different authentication backends to different superusers with needing
to edit configuration files on the radius host system. E.g. to be able
to authenticate some against ldap services and some against the password
stored in the database, so the superusers can opt into the central
authentication system if they want to. With the old postgresql version,
all user managers would only need postgresql tcp access, no access to
files or similar.

Could the different behaviour (superusers matching all/not all group
entries in hba.conf) perhaps become a configuration item?




This is a feature in the upcoming 9.2. IIRC the consensus was not to 
backport it. There is no point in making it a configuration item, 
really, since the workaround for the old behaviour would be to add the 
superusers explicitly to the required groups. If you're interested and 
want to apply it to your own build, it's pretty much a one line patch: 
See 



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] -Wformat-zero-length

[Tom Lane]

Bruce Momjian  writes:
> On Tue, Aug 14, 2012 at 05:56:39PM -0400, Tom Lane wrote:
>> The implementation I'm visualizing is that a would-be client (think psql
>> or pg_dump, though the code would actually be in libpq) forks off a
>> process that becomes a standalone backend, and then they communicate
>> over a pair of pipes that were created before forking.  This is
>> implementable on any platform that supports Postgres, because initdb
>> already relies on equivalent capabilities.

> I think the big question is whether we need to modify every binary that
> pg_upgrade executes to underestand this pipe communication method.

I think we can fix it once in libpq and we're done.  It'd be driven
by some new connection-string option, and the clients as such would
never need to know that they're not talking to a regular postmaster.

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] small issue with host names in hba

[Bruce Momjian]

I assume we didn't feel any action was necessary on this issue.

---

On Thu, Aug 11, 2011 at 01:50:02PM -0400, Tom Lane wrote:
> Robert Haas  writes:
> > On Tue, Aug 9, 2011 at 2:16 PM, Peter Eisentraut  wrote:
> >> But I'm a little confused by what this code is really trying
> >> to accomplish: ...
> 
> > I think the intended behavior of NI_NUMERICHOST is to suppress the
> > name lookup, and return the text format *even if* the name lookup
> > would have worked.  So the intention of this code may be to ensure
> > that we convert the the sockaddr to some sort of string even if we
> > can't resolve the hostname - i.e. if the first call fails, try again
> > with NI_NUMERICHOST added to make sure that we didn't fail solely due
> > to some kind of DNS hiccup.  I suspect that at some time this was
> > defending against an actual hazard but I don't know whether it's still
> > a problem on modern operating systems.
> 
> POSIX v7 says
> 
>   If the node's name cannot be located, the numeric form of the
>   address contained in the socket address structure pointed to by
>   the sa argument is returned instead of its name.
> 
> If you read a bit further, apparently this is just supposed to be the
> default behavior if neither NI_NUMERICHOST nor NI_NAMEREQD is set (in
> the first case, it doesn't try to locate a node name, and in the second,
> it fails if it can't locate a node name).  But certainly the dance in
> postmaster.c is not necessary if you believe the spec.
> 
> I believe that the existing coding is meant to cope with the behavior of
> our stub version of getnameinfo(), which simply fails outright if
> NI_NUMERICHOST isn't set.  However, if we just removed that test and
> behaved as per spec (return a numeric address anyway), then we could
> eliminate the second call in postmaster.c.
> 
> >> The fix would appear to be using the NI_NAMEREQD flag to getnameinfo.
> 
> > If you want to do that, you're going to have to fix the version of
> > getnameinfo() in src/port/getaddrinfo.c, which apparently doesn't
> > support that flag.
> 
> Well, it's not just that it "doesn't support that flag".  It's
> fundamentally incapable of returning anything but a numeric address,
> and therefore the only "support" it could offer would be to fail.  So
> that approach seems like a dead end.
> 
> I don't really think that there's anything to fix here with respect to
> Peter's original concern, but it might be worth getting rid of the
> double call in postmaster.c.
> 
>   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

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

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


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

Re: [HACKERS] superusers are members of all roles?

[Michael Braun]

Hi,

I've just recently upgraded to postgrsql 9.1 and also hit bug #5763.
Having +group not match all superusers is essential to be able to assign
different authentication backends to different superusers with needing
to edit configuration files on the radius host system. E.g. to be able
to authenticate some against ldap services and some against the password
stored in the database, so the superusers can opt into the central
authentication system if they want to. With the old postgresql version,
all user managers would only need postgresql tcp access, no access to
files or similar.

Could the different behaviour (superusers matching all/not all group
entries in hba.conf) perhaps become a configuration item?

Regards,
 M. Braun


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

Re: [HACKERS] -Wformat-zero-length

[Bruce Momjian]

On Tue, Aug 14, 2012 at 05:56:39PM -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > On 8/10/12 7:48 PM, Dimitri Fontaine wrote:
> >> What about having single user mode talk fe/be protocol, and talk to it via 
> >> a UNIX pipe, with pg_upgrade starting the single user backend as a 
> >> subprocess?
> 
> > I think that's essentially equivalent to starting the server on a 
> > Unix-domain socket in a private directory.  But that has been rejected 
> > because it doesn't work on Windows.
> 
> > The question in my mind is, is there some other usable way on Windows 
> > for two unrelated processes to communicate over file descriptors in a 
> > private and secure way?
> 
> You're making this unnecessarily hard, because there is no need for the
> two processes to be unrelated.
> 
> The implementation I'm visualizing is that a would-be client (think psql
> or pg_dump, though the code would actually be in libpq) forks off a
> process that becomes a standalone backend, and then they communicate
> over a pair of pipes that were created before forking.  This is
> implementable on any platform that supports Postgres, because initdb
> already relies on equivalent capabilities.

I think the big question is whether we need to modify every binary that
pg_upgrade executes to underestand this pipe communication method.

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

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


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

Re: [HACKERS] -Wformat-zero-length

[Tom Lane]

Peter Eisentraut  writes:
> On 8/10/12 7:48 PM, Dimitri Fontaine wrote:
>> What about having single user mode talk fe/be protocol, and talk to it via a 
>> UNIX pipe, with pg_upgrade starting the single user backend as a subprocess?

> I think that's essentially equivalent to starting the server on a 
> Unix-domain socket in a private directory.  But that has been rejected 
> because it doesn't work on Windows.

> The question in my mind is, is there some other usable way on Windows 
> for two unrelated processes to communicate over file descriptors in a 
> private and secure way?

You're making this unnecessarily hard, because there is no need for the
two processes to be unrelated.

The implementation I'm visualizing is that a would-be client (think psql
or pg_dump, though the code would actually be in libpq) forks off a
process that becomes a standalone backend, and then they communicate
over a pair of pipes that were created before forking.  This is
implementable on any platform that supports Postgres, because initdb
already relies on equivalent capabilities.

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] macports and brew postgresql --universal builds

[Peter Eisentraut]

On 8/10/12 7:12 PM, Doug Coleman wrote:

What it looks like is the first line of each section is pattern matching.

If __LP64__ is defined, then it's a 64-bit architecture, and we want
to use the top part of the if statement. The #defines they target seem
to be all of the ones that are different on 32bit platforms.

I agree that you would want to do this in the configure script somehow.


That's not going to work.  The configure script can only test one target 
at a time.  You can probably get away with it on many simple programs, 
but when a configure script checks size and alignment of types, the 
whole concept of universal builds is at odds with the approach Autoconf 
takes.  OK, so you get away with it by patching up a few sites, but 
you're effectively overriding the configure checks with hardcoded results.


I think the only sane approach in general is to build the entire project 
twice and merge the resulting binaries together.



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

Re: [HACKERS] -Wformat-zero-length

[Peter Eisentraut]

On 8/10/12 7:48 PM, Dimitri Fontaine wrote:



Another thing worth considering is to have pg_upgrade init, stop and
start clusters as necessary instead of requesting the user to do it.
I think this is two less steps.


Then you'd need to expose the entire pg_ctl shutdown mode logic through 
pg_upgrade, which might not make things simpler.


What about having single user mode talk fe/be protocol, and talk to it via a 
UNIX pipe, with pg_upgrade starting the single user backend as a subprocess?


I think that's essentially equivalent to starting the server on a 
Unix-domain socket in a private directory.  But that has been rejected 
because it doesn't work on Windows.


The question in my mind is, is there some other usable way on Windows 
for two unrelated processes to communicate over file descriptors in a 
private and secure way?




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

Re: [HACKERS] SIGFPE handler is naive

[Tom Lane]

Noah Misch  writes:
> On Tue, Aug 14, 2012 at 08:40:06AM -0400, Robert Haas wrote:
>> On Tue, Aug 14, 2012 at 6:50 AM, Greg Stark  wrote:
>>> It is possible to check if the signal was synchronous or was sent from
>>> an external process. You can check siginfo->si_pid to see who sent you
>>> the signal. I'm not sure checking that and handling it at
>>> check_for_interrupts if it's asynchronous is the best solution or not
>>> though.

>> If that's portable it might be an option, but I doubt that it is.

> I suspect it is portable.

Single Unix Spec V2 says (on the sigaction man page)

The si_code member contains a code identifying the cause of the
signal. If the value of si_code is less than or equal to 0, then the
signal was generated by a process and si_pid and si_uid respectively
indicate the process ID and the real user ID of the sender.

I'm not sure I would trust checking si_pid alone; it would definitely
fail on my old HPUX box, where I see that field is union'ed with si_addr
and so will read as garbage for a locally-sourced SIGFPE.  But it might
be that checking si_code alone would work reliably.

I think that rejecting an externally sourced SIGFPE might be worth doing
if we can distinguish that reliably.

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] GetSnapshotData() comments

[Bruce Momjian]

Did these comment updates ever get addressed?

---

On Fri, Aug  5, 2011 at 11:02:24AM -0400, Robert Haas wrote:
> I think that the first sentence, in the following comment within
> GetSnapshotData(), is not quite right, because at the time this is
> executed, we already hold ProcArrayLock, which is the only lock that
> gets grabbed here:
> 
> /*
>  * If we're in recovery then snapshot data comes from a different 
> place,
>  * so decide which route we take before grab the lock. It is
> possible for
>  * recovery to end before we finish taking snapshot, and for newly
>  * assigned transaction ids to be added to the procarray. Xmax cannot
>  * change while we hold ProcArrayLock, so those newly added 
> transaction
>  * ids would be filtered away, so we need not be concerned about them.
>  */
> snapshot->takenDuringRecovery = RecoveryInProgress();
> 
> Having thought it over for a bit, I believe that the code is correct
> and it's only the comment that needs to be updated.  If we were to set
> snapshot->takenDuringRecovery before acquiring ProcArrayLock, then the
> following sequence of events might occur:
> 
> T1: Enter GetSnapshotData().  Set snapshot->takenDuringRecovery = true.
> Recovery ends
> T2: Begin, get an XID.
> T3: Begin, get an XID.
> T3: Commit, advancing latestCompletedXid.
> T1: Acquire ProcArrayLock and use the "in recovery" path, missing T2's
> XID (because it's not in the subxip[] array).
> T1: Do some stuff with the snapshot... not seeing T2's XID...
> T2: Commit
> T1: Do some stuff with the snapshot... now seeing T2's XID...
> 
> I think if we just delete the first sentence of the comment, the rest
> is all correct.
> 
> A little further down, there is this comment:
> 
> /*
>  * Spin over procArray checking xid, xmin, and
> subxids.  The goal is
>  * to gather all active xids, find the lowest xmin,
> and try to record
>  * subxids. During recovery no xids will be assigned,
> so all normal
>  * backends can be ignored, nor are there any VACUUMs
> running. All
>  * prepared transaction xids are held in
> KnownAssignedXids, so these
>  * will be seen without needing to loop through procs here.
>  */
> 
> ...but this code is only executed when recovery is not in progress.
> So I feel like everything after "try to record subxids" should either
> be removed, or relocated to the following "else" block, where the
> recovery path is discussed in detail.
> 
> -- 
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

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

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


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

Re: [HACKERS] TRUE/FALSE vs true/false

[Bruce Momjian]

On Tue, Aug 14, 2012 at 05:34:02PM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
> >> On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
> >>> I meant a mass "sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g'" run
> >>> so all the ~200 occurrences of both "TRUE" and "FALSE" get
> >>> converted so the whole source tree is consistent.
> 
> >> I would be in favor of that.
> 
> > I have implemented this with the patch at:
> > http://momjian.us/expire/true_diff.txt
> 
> Does this really do anything for us that will justify the extra
> back-patching pain it will cause?  I don't see that it's improving
> code readability any.

I think it is more of a consistency issue.  There were multiple people
who wanted this change.  Of course, some of those people don't backport
stuff.

Other comments?

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

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


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

Re: [HACKERS] TRUE/FALSE vs true/false

[Tom Lane]

Bruce Momjian  writes:
> On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
>> On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
>>> I meant a mass "sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g'" run
>>> so all the ~200 occurrences of both "TRUE" and "FALSE" get
>>> converted so the whole source tree is consistent.

>> I would be in favor of that.

> I have implemented this with the patch at:
>   http://momjian.us/expire/true_diff.txt

Does this really do anything for us that will justify the extra
back-patching pain it will cause?  I don't see that it's improving
code readability any.

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] default_isolation_level='serializable' crashes on Windows

[Kevin Grittner]

Heikki Linnakangas  wrote:
> On 14.08.2012 14:25, Kevin Grittner wrote:
>> Heikki Linnakangas  wrote:
>>> On 14.08.2012 08:23, Kevin Grittner wrote:
 
>> Oh, further testing this morning shows that while *queries* on
>> the HS seem OK, streaming replication is now broken.  I probably
>> need to override transaction isolation on the recovery process. 
>> I'll take a look at that.
> 
> Hmm, seems to work for me. Do you get an unexpected error or what?
 
No, I wasn't getting errors in the clients or the logs, but I wasn't
seeing data pop up on the replica when I expected.  Perhaps I messed
up my streaming replication configuration somehow.  Do I understand
that you are now seeing correction of both bugs and no new
misbehaviors in your tests?
 
>>> Now that the error is thrown when the first snapshot is taken,
>>> rather than at the SET command, I think the error message needs
>>> some work:
>>>
>>> postgres=# select 123;
>>> ERROR: cannot use serializable mode in a hot standby
>>> HINT: You can use REPEATABLE READ instead.
>>>
>>> If the isolation level came from default_transaction_isolation,
>>> set in postgresql.conf file, it might take the user a while to
>>> figure that out. Perhaps something like this:
>>>
>>> ERROR: cannot use serializable mode in a hot standby
>>> DETAIL: default_transaction_isolation was set to 'serializable'
>>> in the config file.
>>> HINT: You can use "SET transaction_isolation = 'repeatable
>>> read'" before the first query in the transaction to override it.
>>
>> Did you mean?:
>>
>> HINT: You can use "SET default_transaction_isolation =
>> 'repeatable read'" before the first query in the transaction to
>> override it.
 
> I didn't, I meant to point out that you can set
> transaction_isolation just for the one transaction. But your
> suggested hint is OK as well - you suggest to set it for the whole
> session, which will also work around the problem. The "before the
> first query in the transaction" isn't necessary in that case
> though.
 
Yeah, I figured that out before posting version 2 of the patch.  I
should have said something.
 
> Yet another option is to suggest the "BEGIN ISOLATION LEVEL
> REPEATABLE READ" syntax, instead of SET.
 
I'm inclined toward hinting at a session override of the default. 
If you're typing away in psql, that's a lot less work.  :-)  That's
what I included in version 2 of the patch, but only if the default
was serializable.  I did say to set it "before a transaction"
because a quick test showed that setting the default in a
transaction didn't keep that transaction from getting the error if
you proceeded to run a SELECT statement.
 
 Since the existing behavior is so bad, I'm inclined to think
 this merits backpatching to 9.1. Thoughts on that?
>>>
>>> Yes, we have to somehow fix the crash and the assertion failure
>>> on 9.1.
>>
>> Should the check_transaction_read_only() stuff be back-patched to
>> 9.1, too?  So far as we know, that's fragile, not broken, right?
>> Could the fix you envision there cause a behavioral change that
>> could break anything that users might have in place?
> 
> Good question. I don't see how it could cause a behavioral change,
> but I've been wrong before.
 
If we don't know of any actual existing bugs I'm inclined to not
back-patch that part to 9.1, although it makes sense for 9.2 since
we shouldn't be risking breakage of any production systems.  I'm
really cautious about giving anybody any excuse not to apply a minor
update.
 
-Kevin


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

Re: [HACKERS] SIGFPE handler is naive

[Noah Misch]

On Tue, Aug 14, 2012 at 08:40:06AM -0400, Robert Haas wrote:
> On Tue, Aug 14, 2012 at 6:50 AM, Greg Stark  wrote:
> > It is possible to check if the signal was synchronous or was sent from
> > an external process. You can check siginfo->si_pid to see who sent you
> > the signal. I'm not sure checking that and handling it at
> > check_for_interrupts if it's asynchronous is the best solution or not
> > though.
> 
> If that's portable it might be an option, but I doubt that it is.

I suspect it is portable.  Nonetheless, kill() is not the only SIGFPE source
that ought to produce a PANIC.  Library code might trigger the signal, at
which point we cannot assume that elog(ERROR) will leave an acceptable system
state.  To call this fixed, we need a whitelist of safe sources, not a
blacklist of bogus sources.

That said, I agree that the effort and risk may be out of proportion.


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

Re: [HACKERS] TRUE/FALSE vs true/false

[Bruce Momjian]

On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
> On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
> > 2011-08-04 14:32 keltezéssel, Robert Haas írta:
> > > 2011/8/4 Boszormenyi Zoltan :
> > >> Shouldn't these get fixed to be consistent?
> > > I believe I already did.  See commit 
> > > 85b436f7b1f06a6ffa8d2f29b03d6e440de18784.
> > 
> > I meant a mass "sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g'" run
> > so all the ~200 occurrences of both "TRUE" and "FALSE" get
> > converted so the whole source tree is consistent.
> 
> I would be in favor of that.

I have implemented this with the patch at:

http://momjian.us/expire/true_diff.txt

I did not modify the Win32 code which traditionally uses uppercase for
TRUE/FALSE.

It would be applied only to HEAD.

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

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


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

Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

[Heikki Linnakangas]

On 14.08.2012 14:25, Kevin Grittner wrote:

Heikki Linnakangas  wrote:

On 14.08.2012 08:23, Kevin Grittner wrote:



OK, attached is a first cut to confirm that the approach looks
sane to everyone; I *think* it is along the lines that we reached
consensus. After moving the check to the point where we get a
serializable snapshot, it was still behaving badly. It took a bit,
but forcing the snapshot acquisition in InitPostgres to use 'read
committed' instead of the default isolation level got reasonable
behavior in my initial tests. It sure looks a lot better to *me*
than what was happening before.


Oh, further testing this morning shows that while *queries* on the HS
seem OK, streaming replication is now broken.  I probably need to
override transaction isolation on the recovery process.  I'll take a
look at that.


Hmm, seems to work for me. Do you get an unexpected error or what?


Now that the error is thrown when the first snapshot is taken,
rather than at the SET command, I think the error message needs
some work:

postgres=# select 123;
ERROR: cannot use serializable mode in a hot standby
HINT: You can use REPEATABLE READ instead.

If the isolation level came from default_transaction_isolation, set
in postgresql.conf file, it might take the user a while to figure
that out. Perhaps something like this:

ERROR: cannot use serializable mode in a hot standby
DETAIL: default_transaction_isolation was set to 'serializable' in
the config file.
HINT: You can use "SET transaction_isolation = 'repeatable read'"
before the first query in the transaction to override it.


Did you mean?:

HINT: You can use "SET default_transaction_isolation = 'repeatable
read'" before the first query in the transaction to override it.

Otherwise, I agree and will do.


I didn't, I meant to point out that you can set transaction_isolation 
just for the one transaction. But your suggested hint is OK as well - 
you suggest to set it for the whole session, which will also work around 
the problem. The "before the first query in the transaction" isn't 
necessary in that case though.


Yet another option is to suggest the "BEGIN ISOLATION LEVEL REPEATABLE 
READ" syntax, instead of SET.



Since the existing behavior is so bad, I'm inclined to think this
merits backpatching to 9.1. Thoughts on that?


Yes, we have to somehow fix the crash and the assertion failure on
9.1.


Should the check_transaction_read_only() stuff be back-patched to
9.1, too?  So far as we know, that's fragile, not broken, right?
Could the fix you envision there cause a behavioral change that could
break anything that users might have in place?


Good question. I don't see how it could cause a behavioral change, but 
I've been wrong before.


--
  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] 9.2 Cascading replication after slave promotion

[Josh Berkus]

> Yeah, I think there's more people that agree with this use-case than you
> seem to think..  That said, I appreciate that it's not a trivial thing
> to support cleanly.

Not trivial, no, but not major either.  Really what needs to happen is
for the timeline change record to get transmitted over the WAL stream.

Hmmm.  You know, I bet I could get stream-only remastering working in an
unsafe way just by disabling the timeline checks.  Time to test ...

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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

Re: [HACKERS] WIP patch for consolidating misplaced-aggregate checks

[Tom Lane]

Alvaro Herrera  writes:
> Speaking of english words, I was wondering at "check" the other day.
> For example, we have

> #: catalog/heap.c:2171
> #, c-format
> msgid "check constraint \"%s\" already exists"

> #: catalog/heap.c:2534
> #, c-format
> msgid "only table \"%s\" can be referenced in check constraint"

> And so on (there are several more).  Note that here we use "check
> constraint" without any capitalization.

FWIW, I think I changed "check" to "CHECK" in a couple of messages
recently, for exactly the reason that it seemed to be used in its
keyword meaning rather than as plain English text.  Perhaps we
should just go around and do that consistently.

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] Statistics and selectivity estimation for ranges

[Heikki Linnakangas]

On 14.08.2012 09:45, Alexander Korotkov wrote:

After fixing few more bugs, I've a version with much more reasonable
accuracy.


Great! One little thing just occurred to me:

You're relying on the regular scalar selectivity estimators for the <<, 
>>, &< and &> operators. That seems bogus, in particular for << and &<, 
because ineq_histogram_selectivity then performs a binary search of the 
histogram using those operators. << and &< compare the *upper* bound of 
the value in table against the lower bound of constant, but the 
histogram is constructed using regular < operator, which sorts the 
entries by lower bound. I think the estimates you now get for those 
operators are quite bogus if there is a non-trivial amount of overlap 
between ranges. For example:


postgres=# create table range_test as
select int4range(-a, a) as r from generate_series(1,100) a; analyze 
range_test;

SELECT 100
ANALYZE
postgres=# EXPLAIN ANALYZE SELECT * FROM range_test WHERE r <<
int4range(20, 21);
QUERY PLAN 




---
 Seq Scan on range_test  (cost=0.00..17906.00 rows=100 width=14) 
(actual time=0.

060..1340.147 rows=20 loops=1)
   Filter: (r << '[20,21)'::int4range)
   Rows Removed by Filter: 80
 Total runtime: 1371.865 ms
(4 rows)

It would be quite easy to provide reasonable estimates for those 
operators, if we had a separate histogram of upper bounds. I also note 
that the estimation of overlap selectivity could be implemented using 
separate histograms of lower bounds and upper bounds, without requiring 
a histogram of range lengths, because a && b == NOT (a << b OR a >> b). 
I'm not sure if the estimates we'd get that way would be better or worse 
than your current method, but I think it would be easier to understand.


I don't think the &< and &> operators could be implemented in terms of a 
lower and upper bound histogram, though, so you'd still need the current 
length histogram method for that.


The code in that traverses the lower bound and length histograms in 
lockstep looks quite scary. Any ideas on how to simplify that? My first 
thought is that there should be helper function that gets a range length 
as argument, and returns the fraction of tuples with length >= argument. 
It would do the lookup in the length histogram to find the right 
histogram bin, and do the linear interpolation within the bin. You're 
assuming that length is independent of lower/upper bound, so you 
shouldn't need any other parameters than range length for that estimation.


You could then loop through only the lower bounds, and call the helper 
function for each bin to get the fraction of ranges long enough in that 
bin, instead dealing with both histograms in the same loop. I think a 
helper function like that might simplify those scary loops 
significantly, but I wasn't sure if there's some more intelligence in 
the way you combine values from the length and lower bound histograms 
that you couldn't do with such a helper function.


--
  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 patch for consolidating misplaced-aggregate checks

[Alvaro Herrera]

Excerpts from Greg Stark's message of vie ago 10 12:57:25 -0400 2012:
> On Thu, Aug 9, 2012 at 5:40 PM, Tom Lane  wrote:
> > Fair enough.  I was not sold on doing that either.  I would still like
> > to know if it's okay to use one string with %s for the cases where
> > there's not a good reason for the "context" to be more than just a
> > SQL keyword.
> 
> Given that the SQL keyword is going to be an English word I can't
> imagine how this could be a big deal for translators. It might not
> match gender or case or something but only if the reader is
> automatically mentally translating the keyword into their language and
> then applying that language's rules to it. At least to me it makes
> sense to refer to "VALUES" as a singular noun or "LIMIT" as a generic
> male noun even though "limitation" would be feminine (I had to look
> that one up though so perhaps I'm not the best person to judge).

Speaking of english words, I was wondering at "check" the other day.
For example, we have

#: catalog/heap.c:2171
#, c-format
msgid "check constraint \"%s\" already exists"

#: catalog/heap.c:2534
#, c-format
msgid "only table \"%s\" can be referenced in check constraint"

And so on (there are several more).  Note that here we use "check
constraint" without any capitalization.  However this doesn't translate
too well as is; I mean, if I were to translate "check" into its
equivalent spanish word, I'm sure to cause a great deal of confusion.
So I've opted for putting the check word, verbatim, in quotes; for
example:

msgid "check constraint \"%s\" already exists"
msgstr "la restricción «check» «%s» ya existe"

However this is also a bit ugly because I now have two sets of quoted
words -- check itself and then the constraint name.

If we were to have CHECK in uppercase, this would be easy:

msgid "check constraint \"%s\" already exists"
msgstr "la restricción CHECK «%s» ya existe"

Maybe I should just do that -- uppercase the keyword instead of sticking
it in quotes.


(As for the gender of limit, in spanish, you'd probably think of "el
límite" which is masculine.  But in general, I agree with you: I think
it makes sense to keep the key word in english in the error message.)

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] SIGFPE handler is naive

[Tom Lane]

Noah Misch  writes:
> On Tue, Aug 14, 2012 at 08:38:44AM -0400, Robert Haas wrote:
>> On Mon, Aug 13, 2012 at 11:52 PM, Tom Lane  wrote:
>>> That would depend on how many places there are where SIGFPE is expected.
>>> Are we sure there are any?  Maybe we should just remove the handler and
>>> let SIGFPE be treated as a core dump.

>> No clue.  According to Wikipedia, it is commonly caused by dividing by
>> zero, or by dividing by INT_MIN by -1, resulting in a quotient out of
>> range for the type.  I'd be willing to bet that we have got all the
>> division-by-zero cases patched up just because we try pretty hard to
>> emit the right error message for such cases, but I'm a lot less
>> certain that things like INT_MIN/-1 can't happen anywhere.

> [local] test=# select -9223372036854775808/-1;
> ERROR:  floating-point exception

On reflection I think we should just leave this alone.  If we try to
tighten it up, what we will mainly accomplish is to make the system
less robust, not more, because any place we miss then represents an
easily reproducible DOS attack.

If somebody's dumb enough to think that SIGFPE'ing a backend represents
a supported way of canceling a query, that's his problem not ours.
We don't need to expend large amounts of effort on being sure we slap
his hand.

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] SIGFPE handler is naive

[Robert Haas]

On Tue, Aug 14, 2012 at 8:55 AM, k...@rice.edu  wrote:
> On Mon, Aug 13, 2012 at 11:52:06PM -0400, Tom Lane wrote:
>> Robert Haas  writes:
>> > On Mon, Aug 13, 2012 at 10:14 PM, Noah Misch  wrote:
>> >> Overall, though, I think it best to plug this.  We could set a flag before
>> >> each operation, like evaluation of SQL arithmetic, for which SIGFPE is 
>> >> normal.
>>
>> > Yeah, that's what I thought of, too.  It seems like it'd be a lot of
>> > work to get there, though.
>>
>> That would depend on how many places there are where SIGFPE is expected.
>> Are we sure there are any?  Maybe we should just remove the handler and
>> let SIGFPE be treated as a core dump.
>
> Wouldn't any user level divide-by-zero code cause a SIGFPE?

If it's written in C, sure.  If it's written in SQL, no, because we
check for that inside int4div et all.

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


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

Re: [HACKERS] SIGFPE handler is naive

[Noah Misch]

On Tue, Aug 14, 2012 at 08:38:44AM -0400, Robert Haas wrote:
> On Mon, Aug 13, 2012 at 11:52 PM, Tom Lane  wrote:
> > That would depend on how many places there are where SIGFPE is expected.
> > Are we sure there are any?  Maybe we should just remove the handler and
> > let SIGFPE be treated as a core dump.
> 
> No clue.  According to Wikipedia, it is commonly caused by dividing by
> zero, or by dividing by INT_MIN by -1, resulting in a quotient out of
> range for the type.  I'd be willing to bet that we have got all the
> division-by-zero cases patched up just because we try pretty hard to
> emit the right error message for such cases, but I'm a lot less
> certain that things like INT_MIN/-1 can't happen anywhere.

[local] test=# select -9223372036854775808/-1;
ERROR:  floating-point exception


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

Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

[Kevin Grittner]

Heikki Linnakangas  wrote:
 
> we have to somehow fix the crash and the assertion failure on 9.1.
 
Here's a revision with some changes based on your feedback.  I have
to go to my "day job" now, and I was unable to find the right place
to fix the streaming replication problem.  I fear I won't be able to
get this sorted out before the wrap of back-branch releases this
evening, so if you feel it is urgent enough to need to get into
that, feel free to finish it; otherwise I'll keep at it tonight.
 
-Kevin




hotstandby-serializable-2.patch
Description: Binary data

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

Re: [HACKERS] SIGFPE handler is naive

[k...@rice.edu]

On Mon, Aug 13, 2012 at 11:52:06PM -0400, Tom Lane wrote:
> Robert Haas  writes:
> > On Mon, Aug 13, 2012 at 10:14 PM, Noah Misch  wrote:
> >> Overall, though, I think it best to plug this.  We could set a flag before
> >> each operation, like evaluation of SQL arithmetic, for which SIGFPE is 
> >> normal.
> 
> > Yeah, that's what I thought of, too.  It seems like it'd be a lot of
> > work to get there, though.
> 
> That would depend on how many places there are where SIGFPE is expected.
> Are we sure there are any?  Maybe we should just remove the handler and
> let SIGFPE be treated as a core dump.
> 
>   regards, tom lane
> 

Wouldn't any user level divide-by-zero code cause a SIGFPE? 

Regards,
Ken


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

Re: [HACKERS] SIGFPE handler is naive

[Robert Haas]

On Tue, Aug 14, 2012 at 6:50 AM, Greg Stark  wrote:
> It is possible to check if the signal was synchronous or was sent from
> an external process. You can check siginfo->si_pid to see who sent you
> the signal. I'm not sure checking that and handling it at
> check_for_interrupts if it's asynchronous is the best solution or not
> though.

If that's portable it might be an option, but I doubt that it is.

> I'm a bit confused. Didn't Tom do the laborious process of checking
> the whole source tree for situations where there's shared memory
> cleanup to be done in and arrange for it to happen? That was the
> blocking factor to get pg_cancel_backend() to work. Is the problem
> that the sigfpe handler doesn't invoke atexit() handlers?

No, the problem is that SIGFPE throws an error *from the signal
handler* rather than waiting for ProcessInterrupts().

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


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

Re: [HACKERS] SIGFPE handler is naive

[Robert Haas]

On Mon, Aug 13, 2012 at 11:52 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Aug 13, 2012 at 10:14 PM, Noah Misch  wrote:
>>> Overall, though, I think it best to plug this.  We could set a flag before
>>> each operation, like evaluation of SQL arithmetic, for which SIGFPE is 
>>> normal.
>
>> Yeah, that's what I thought of, too.  It seems like it'd be a lot of
>> work to get there, though.
>
> That would depend on how many places there are where SIGFPE is expected.
> Are we sure there are any?  Maybe we should just remove the handler and
> let SIGFPE be treated as a core dump.

No clue.  According to Wikipedia, it is commonly caused by dividing by
zero, or by dividing by INT_MIN by -1, resulting in a quotient out of
range for the type.  I'd be willing to bet that we have got all the
division-by-zero cases patched up just because we try pretty hard to
emit the right error message for such cases, but I'm a lot less
certain that things like INT_MIN/-1 can't happen anywhere.

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


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

Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

[Kevin Grittner]

Heikki Linnakangas  wrote:
> On 14.08.2012 08:23, Kevin Grittner wrote:
 
>> OK, attached is a first cut to confirm that the approach looks
>> sane to everyone; I *think* it is along the lines that we reached
>> consensus. After moving the check to the point where we get a
>> serializable snapshot, it was still behaving badly. It took a bit,
>> but forcing the snapshot acquisition in InitPostgres to use 'read
>> committed' instead of the default isolation level got reasonable
>> behavior in my initial tests. It sure looks a lot better to *me*
>> than what was happening before.
 
Oh, further testing this morning shows that while *queries* on the HS
seem OK, streaming replication is now broken.  I probably need to
override transaction isolation on the recovery process.  I'll take a
look at that.
 
> A comment in InitPostgres would be nice, explaining why we want a
> read committed snapshot there.
 
OK.
 
> This fixes both the assertion failure and the Windows crash, which
> is good.
 
I wasn't sure whether it would help with the Windows crash or not. 
I'm not set up to build under Windows, so I'm glad you gave that a
check.
 
> Now that the error is thrown when the first snapshot is taken,
> rather than at the SET command, I think the error message needs
> some work:
> 
> postgres=# select 123;
> ERROR: cannot use serializable mode in a hot standby
> HINT: You can use REPEATABLE READ instead.
> 
> If the isolation level came from default_transaction_isolation, set
> in postgresql.conf file, it might take the user a while to figure
> that out. Perhaps something like this:
> 
> ERROR: cannot use serializable mode in a hot standby
> DETAIL: default_transaction_isolation was set to 'serializable' in
> the config file.
> HINT: You can use "SET transaction_isolation = 'repeatable read'"
> before the first query in the transaction to override it.
 
Did you mean?:
 
HINT: You can use "SET default_transaction_isolation = 'repeatable
read'" before the first query in the transaction to override it.
 
Otherwise, I agree and will do.
 
> This still leaves the RecoveryInProgress() call in
> check_transaction_read_only(), which isn't a problem at the moment,
> but seems fragile. I think we should still add the
> IsTransactionState() check in there, so that it works without
> shared memory access. If we're not in a transaction yet
> (TRANS_DEFAULT), setting transaction_read_only has no effect, as
> it's overwritten at the beginning of a transaction. If we're in one
> of the transitory states, TRANS_START or
> TRANS_COMMIT/ABORT/PREPARE, I'm not sure what should happen, but it
> should not be possible for user code to run and change
> transaction_read_only in those states.
 
I'll take a look.
 
>> Since the existing behavior is so bad, I'm inclined to think this
>> merits backpatching to 9.1. Thoughts on that?
> 
> Yes, we have to somehow fix the crash and the assertion failure on
> 9.1.
 
Should the check_transaction_read_only() stuff be back-patched to
9.1, too?  So far as we know, that's fragile, not broken, right? 
Could the fix you envision there cause a behavioral change that could
break anything that users might have in place?
 
-Kevin


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

Re: [HACKERS] SIGFPE handler is naive

[Greg Stark]

It is possible to check if the signal was synchronous or was sent from
an external process. You can check siginfo->si_pid to see who sent you
the signal. I'm not sure checking that and handling it at
check_for_interrupts if it's asynchronous is the best solution or not
though.

I'm a bit confused. Didn't Tom do the laborious process of checking
the whole source tree for situations where there's shared memory
cleanup to be done in and arrange for it to happen? That was the
blocking factor to get pg_cancel_backend() to work. Is the problem
that the sigfpe handler doesn't invoke atexit() handlers?

-- 
greg


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

Re: [HACKERS] SIGFPE handler is naive

[Nils Goroll]

Should we do something to plug this, and if so, what?  If not, should
we document the danger?


I am not sure if I really understood the intention of the question correctly, 
but if the question was if pg should try to work around misuse of signals, then 
my answer would be a definite no.


IMHO, the signal handler should check if the signal was received for good 
reasons (as proposed by Noah) and handle it appropriately, but otherwise ignore it.


Nils


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

Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

[Heikki Linnakangas]

On 14.08.2012 08:23, Kevin Grittner wrote:

OK, attached is a first cut to confirm that the approach looks sane
to everyone; I *think* it is along the lines that we reached
consensus.  After moving the check to the point where we get a
serializable snapshot, it was still behaving badly.  It took a bit,
but forcing the snapshot acquisition in InitPostgres to use 'read
committed' instead of the default isolation level got reasonable
behavior in my initial tests.  It sure looks a lot better to *me*
than what was happening before.


A comment in InitPostgres would be nice, explaining why we want a read 
committed snapshot there.



Besides confirming that this is the solution people want, this has
not been tested well enough yet to be ready for commit.  It's getting
late, though, and I'm fading.  If the overall approach looks good
I'll beat up on it some more tomorrow.


Thanks! This fixes both the assertion failure and the Windows crash, 
which is good.


Now that the error is thrown when the first snapshot is taken, rather 
than at the SET command, I think the error message needs some work:


postgres=# select 123;
ERROR:  cannot use serializable mode in a hot standby
HINT:  You can use REPEATABLE READ instead.

If the isolation level came from default_transaction_isolation, set in 
postgresql.conf file, it might take the user a while to figure that out. 
Perhaps something like  this:


ERROR:  cannot use serializable mode in a hot standby
DETAIL: default_transaction_isolation was set to 'serializable' in the 
config file.
HINT:   You can use "SET transaction_isolation = 'repeatable read'" 
before the first query in the transaction to override it.



This still leaves the RecoveryInProgress() call in 
check_transaction_read_only(), which isn't a problem at the moment, but 
seems fragile. I think we should still add the IsTransactionState() 
check in there, so that it works without shared memory access. If we're 
not in a transaction yet (TRANS_DEFAULT), setting transaction_read_only 
has no effect, as it's overwritten at the beginning of a transaction. If 
we're in one of the transitory states, TRANS_START or 
TRANS_COMMIT/ABORT/PREPARE, I'm not sure what should happen, but it 
should not be possible for user code to run and change 
transaction_read_only in those states.



Since the existing behavior is so bad, I'm inclined to think this
merits backpatching to 9.1.  Thoughts on that?


Yes, we have to somehow fix the crash and the assertion failure on 9.1.

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

[HACKERS] betatesting: ERROR: failed to build any 2-way joins on 9.2

[Pavel Stehule]

Hello

My colleague found a issue on 9.2 - sorry for query formatting - this
query is generated from ours query engine

testdb=# \i planbug.sql
DROP TABLE
DROP TABLE
DROP TABLE
DROP TABLE
DROP TABLE
CREATE TABLE
CREATE TABLE
CREATE TABLE
CREATE TABLE
CREATE TABLE
psql:planbug.sql:66: ERROR:  failed to build any 2-way joins
LOCATION:  join_search_one_level, joinrels.c:199

it works on 9.1


planbug.sql
Description: Binary data

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

[HACKERS] patch: shared session variables

[Pavel Stehule]

Hello

patch that implements "shared" client/server session variables

Regards

Pavel Stehule


shared_variables-01.diff
Description: Binary data

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