Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
On 05.07.2011 00:42, Florian Pflug wrote:
On Jul4, 2011, at 23:11 , Peter Geoghegan wrote:
On 4 July 2011 17:36, Florian Pflug wrote:
Btw, with the death-watch / life-sign / whatever infrastructure in place,
shouldn't PostmasterIsAlive() be using that instead of getppid() / kill(0)?
Hmm, maybe. That seems like a separate issue though, that can be
addressed with another patch. It does have the considerable
disadvantage of making Heikki's proposed assertion failure useless. Is
the implementation of PostmasterIsAlive() really a problem at the
moment?
I'm not sure that there is currently a guarantee that PostmasterIsAlive
will returns false immediately after select() indicates postmaster
death. If e.g. the postmaster's parent is still running (which happens
for example if you launch postgres via daemontools), the re-parenting of
backends to init might not happen until the postmaster zombie has been
vanquished by its parent's call of waitpid(). It's not entirely
inconceivable for getppid() to then return the (dead) postmaster's pid
until that waitpid() call has occurred.
Good point, and testing shows that that is exactly what happens at least
on Linux (see attached test program). So, as the code stands, the
children will go into a busy loop until the grandparent calls waitpid().
That's not good.
In that light, I agree we should replace kill() in PostmasterIsAlive()
with read() on the pipe. It would react faster than the kill()-based
test, which seems like a good thing. Or perhaps do both, and return
false if either test says the postmaster is dead.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
#include
#include
int main(int argc, char **argv)
{
pid_t parentpid,
childpid;
int pipefds[2];
if ((parentpid = fork()) == 0)
{
parentpid = getpid();
pipe(pipefds);
if ((childpid = fork()) == 0)
{
char c;
int rc;
close(pipefds[1]);
/* Wait for pipe to close */
printf ("read() returned %d\n", read(pipefds[0], &c, 1));
while (kill(parentpid, 0) == 0)
{
printf("kill() says the parent is still alive\n");
sleep(1);
}
printf("kill() confirms that the parent is dead\n");
}
else
{
sleep(3);
printf("parent exits now\n");
}
}
else
{
printf("grandparent is sleeping\n");
sleep(6);
printf("grandparent exits now\n");
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] %ENV warnings during builds
schrieb Andrew Dunstan: Hmm, I missed that you had done this. Here are two replacement perl scripts I knocked up, but haven't yet tested. One of the things about them is that they remove knowledge of particular .l and .y files. and instead get the required invocation options from the relevant Makefiles. I think that's a lot better than the horrid hackery we have in the batch files. Yep - they certainly look ways less messy than what I've created as a simple perl version of the batch files. But I get "Undefined subroutine &main::dirname called at src\tools\msvc\pgflex.pl line 36." if I try to build with them. I'll update my patch to remove my versions once they are fixed and commited. Meanwhile you might need to create (at least temporarily) .bat wrappers as the unpatched msvc build sytem expects them to be in place. I could also extract those parts from my patch but we should probably go the wohle way now to get it in shape and get it commited. Regards, Brar -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Online base backup from the hot-standby
2011/7/4 Jun Ishiduka : > >> When the standby restarts after it crashes during recovery, it always >> checks whether recovery has reached the backup end location by >> using minRecoveryPoint even though the standby doesn't start from >> the backup. This looks odd. > > Certainly. > > But, in this case, the state before recovery starts is lost. > Therefore, postgres can not see that the backup got from whether > standby server or master. > > What should? > Should use pg_control? > Ex. > * Add 'Where to get backup' to pg_control. (default 'none') > * When recovery starts, it checks it whether 'none'. > * When minRecoveryPoint equals 0/0, change 'master'. > * When minRecoveryPoint do not equals 0/0, change 'slave'. > * When it reached the end of recovery, change 'none' . What about using backupStartPoint to check whether this recovery started from the backup or not? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cascade replication
On Mon, Jul 4, 2011 at 6:24 PM, Simon Riggs wrote: > On Tue, Jun 14, 2011 at 6:08 AM, Fujii Masao wrote: > >>> The standby must not accept replication connection from that standby itself. >>> Otherwise, since any new WAL data would not appear in that standby, >>> replication cannot advance any more. As a safeguard against this, I >>> introduced >>> new ID to identify each instance. The walsender sends that ID as the fourth >>> field of the reply of IDENTIFY_SYSTEM, and then walreceiver checks whether >>> the IDs are the same between two servers. If they are the same, which means >>> that the standby is just connecting to that standby itself, so walreceiver >>> emits ERROR. > > Thanks for waiting for review. Thanks for the review! > This part of the patch is troubling me. I think you have identified an > important problem, but this solution doesn't work fully. > > If we allow standbys to connect to other standbys then we have > problems with standbys not being connected to master. This can occur > with a 1-step connection, as you point out, but it could also occur > with a 2-step, 3-step or more connection, where a circle of standbys > are all depending upon each other. Your solution only works for 1-step > connections. "Solving" that problem in a general sense might be more > dangerous than leaving it alone. I think we should think some more > about the issues there and approach them as a separate problem. > > I think we should remove that and just focus on the main problem, for > now. That will make it a simpler patch and easier to commit. I agree to focus on the main problem first. I removed that. Attached is the updated version. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** *** 1998,2004 SET ENABLE_SEQSCAN TO OFF; doesn't keep any extra segments for standby purposes, and the number of old WAL segments available to standby servers is a function of the location of the previous checkpoint and status of WAL ! archiving. This parameter has no effect on restartpoints. This parameter can only be set in the postgresql.conf file or on the server command line. --- 1998,2004 doesn't keep any extra segments for standby purposes, and the number of old WAL segments available to standby servers is a function of the location of the previous checkpoint and status of WAL ! archiving. This parameter can only be set in the postgresql.conf file or on the server command line. *** *** 2105,2111 SET ENABLE_SEQSCAN TO OFF; synchronous replication is enabled, individual transactions can be configured not to wait for replication by setting the parameter to ! local or off. This parameter can only be set in the postgresql.conf --- 2105,2112 synchronous replication is enabled, individual transactions can be configured not to wait for replication by setting the parameter to ! local or off. This parameter has no effect on ! cascade replication. This parameter can only be set in the postgresql.conf *** a/doc/src/sgml/high-availability.sgml --- b/doc/src/sgml/high-availability.sgml *** *** 877,884 primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass' --- 877,921 network delay, or that the standby is under heavy load. + + + +Cascade Replication + + Cascade Replication + + + Cascade replication feature allows the standby to accept the replication + connections and stream WAL records to another standbys. This is useful + for reducing the number of standbys connecting to the master and reducing + the overhead of the master, when you have many standbys. + + + The cascading standby sends not only WAL records received from the + master but also those restored from the archive. So even if the replication + connection in higher level is terminated, you can continue cascade replication. + + + Cascade replication is asynchronous. Note that synchronous replication + (see ) has no effect on cascade + replication. + + + Promoting the cascading standby terminates all the cascade replication + connections which it uses. This is because the timeline becomes different + between standbys, and they cannot continue replication any more. + + + To use cascade replication, set up the cascading standby so that it can + accept the replication connections, i.e., set max_wal_senders, + hot_standby and authentication option (see + and ). + Also set primary_conninfo in the cascaded standby to point + to the cascading stan
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
On Tue, Jul 5, 2011 at 1:36 AM, Florian Pflug wrote: > On Jul4, 2011, at 17:53 , Heikki Linnakangas wrote: >>> Under Linux, select() may report a socket file descriptor as "ready >>> for >>> reading", while nevertheless a subsequent read blocks. This could >>> for >>> example happen when data has arrived but upon examination has >>> wrong >>> checksum and is discarded. There may be other circumstances in which >>> a >>> file descriptor is spuriously reported as ready. Thus it may be >>> safer >>> to use O_NONBLOCK on sockets that should not block. >> >> So in theory, on Linux you might WaitLatch might sometimes incorrectly >> return WL_POSTMASTER_DEATH. None of the callers check for >> WL_POSTMASTER_DEATH return code, they call PostmasterIsAlive() before >> assuming the postmaster has died, so that won't affect correctness at the >> moment. I doubt that scenario can even happen in our case, select() on a >> pipe that is never written to. But maybe we should add add an assertion to >> WaitLatch to assert that if select() reports that the postmaster pipe has >> been closed, PostmasterIsAlive() also returns false. > > The correct solution would be to read() from the pipe after select() > returns, and only return WL_POSTMASTER_DEATCH if the read doesn't return > EAGAIN. To prevent that read() from blocking if the read event was indeed > spurious, O_NONBLOCK must be set on the pipe but that patch does that already. +1 The syslogger read() from the pipe after select(), then it thinks EOF has arrived and there is no longer write-side process if the return value of read() is just zero. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
Thanks for reviewing my patch!
On Mon, Jul 4, 2011 at 7:10 AM, Bernd Helmle wrote:
> +comment = 'data type for storing and manipulating JSON content'
>
> I'm not sure, if "manipulating" is a correct description. Maybe i missed it,
> but i didn't see functions to manipulate JSON strings directly, for example,
> adding an object to a JSON string, ...
Indeed. I intend to add manipulation functions in the future. The
words "and manipulating" shouldn't be there yet.
> -- Coding
> ...
> ... It is noticable that the parser seems to define
> its own is_space() and is_digit() functions, e.g.:
>
> +#define is_space(c) ((c) == '\t' || (c) == '\n' || (c) == '\r' || (c) == '
> ')
>
> Wouldn't it be more clean to use isspace() instead?
isspace() accepts '\f', '\v', and sometimes other characters as well,
depending on locale. Likewise, isdigit() accepts some non-ASCII
characters in addition to [0-9], depending on the locale and platform.
Thus, to avoid parsing a superset of the JSON spec, I can't use the
functions. I should add a comment with this explanation.
> This code block in function json_escape_unicode() looks suspicious:
>
> + /* Convert to UTF-8, if necessary. */
> + {
> + const char *orig = json;
> + json = (const char *)
> + pg_do_encoding_conversion((unsigned char *) json, length,
> + GetDatabaseEncoding(), PG_UTF8);
> + if (json != orig)
> + length = strlen(json);
> + }
>
> Seems it *always* wants to convert the string. Isn't there a "if" condition
> missing?
pg_do_encoding_conversion does this check already. Perhaps I should
rephrase the comment slightly:
+ /* Convert to UTF-8 (only if necessary). */
> There's a commented code fragment missing, which should be removed (see last
> two lines of the snippet below):
>
> +static unsigned int
> +read_hex16(const char *in)
> ...
> + Assert(is_hex_digit(c));
> +
> + if (c >= '0' && c <= '9')
> + tmp = c - '0';
> + else if (c >= 'A' && c <= 'F')
> + tmp = c - 'A' + 10;
> + else /* if (c >= 'a' && c <= 'f') */
> + tmp = c - 'a' + 10;
That comment is there for documentation purposes, to indicate the
range check that's skipped because we know c is a hex digit, and [a-f]
is the only thing it could be (and must be) at that line. Should it
still be removed?
> json.c introduces new appendStringInfo* functions, e.g.
> appendStringInfoEscape() and appendStringInfoUtf8(). Maybe it's better
> to name them to something different, since it may occur someday that the
> backend
> will introduce the same notion with a different meaning. They are static,
> but why not naming them into something like jsonAppendStringInfoUtf8() or
> similar?
I agree.
> -- Regression Tests
...
> It also tests UNICODE sequences and input encoding with other server
> encoding than UTF-8.
It tests with other *client* encodings than UTF-8, but not other
server encodings than UTF-8. Is it possible to write tests for
different server encodings?
-- Self-review
There's a minor bug in the normalization code:
> select $$ "\u0009" $$::json;
json
--
"\u0009"
(1 row)
This should produce "\t" instead.
I'm thinking that my expect_*/next_*pop_char/... parsing framework is
overkill, and that, for a syntax as simple as JSON's, visibly messy
parsing code like this:
if (s < e && (*s == 'E' || *s == 'e'))
{
s++;
if (s < e && (*s == '+' || *s == '-'))
s++;
if (!(s < e && is_digit(*s)))
return false;
do
s++;
while (s < e && is_digit(*s));
}
would be easier to maintain than the deceptively elegant-looking code
currently in place:
if (optional_char_cond(s, e, *s == 'E' || *s == 'e'))
{
optional_char_cond(s, e, *s == '+' || *s == '-');
skip1_pred(s, e, is_digit);
}
I don't plan on gutting the current macro-driven parser just yet.
When I do, the new parser will support building a parse tree (only
when needed), and the parsing functions will have signatures like
this:
static bool parse_value(const char **sp, const char *e, JsonNode **out);
static bool parse_string(const char **sp, const char *e, StringInfo
*out);
...
The current patch doesn't provide manipulation features where a parse
tree would come in handy. I'd rather hold off on rewriting the parser
until such features are added.
I'll try to submit a revised patch within the next couple days.
Thanks!
- Joey
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] keepalives_* parameters usefullness
On Tue, Jul 5, 2011 at 4:00 AM, Jaime Casanova wrote: > Simon Riggs writes: >> There's a list of preconditions as to when these settings take effect, >> otherwise they are a no-op. >> > > do we know what those preconditions are? The keepalives don't work at least on linux when the connection is terminated after sending a packet and before receiving TCP-level ACK. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
On 4 July 2011 22:42, Florian Pflug wrote: > If we do expect such event, we should close the hole instead of asserting. > If we don't, then what's the point of the assert. You can say the same thing about any assertion. I'm not going to attempt to close the hole because I don't believe that there is one. I would be happy to see your "read() from the pipe after select()" test asserted though. > BTW, do we currently retry the select() on EINTR (meaning a signal has > arrived)? If we don't, that'd be an additional source of spurious returns > from select. Why might it be? WaitLatch() is currently documented to potentially have its timeout invalidated by the process receiving a signal, which is the exact opposite problem. We do account for this within the archiver calling code though, and I remark upon it in a comment there. > I'm not sure that there is currently a guarantee that PostmasterIsAlive > will returns false immediately after select() indicates postmaster > death. If e.g. the postmaster's parent is still running (which happens > for example if you launch postgres via daemontools), the re-parenting of > backends to init might not happen until the postmaster zombie has been > vanquished by its parent's call of waitpid(). It's not entirely > inconceivable for getppid() to then return the (dead) postmaster's pid > until that waitpid() call has occurred. Yes, this did occur to me - it's hard to reason about what exactly happens here, and probably impossible to have the behaviour guaranteed across platforms, however unlikely it seems. I'd like to hear what Heikki has to say about asserting or otherwise verifying postmaster death in the case of apparent postmaster death wake-up. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Review of patch Bugfix for XPATH() if expression returns a scalar value
Hi Radosław, Do you still have objections, or did I manage to convince you? Also, aside from the question of whether to escape or not, did you find any issues with either the patch (like spurious whitespace, ...) or the code, or is the patch OK implementation-wise? best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
On Jul4, 2011, at 23:11 , Peter Geoghegan wrote: > On 4 July 2011 17:36, Florian Pflug wrote: >> On Jul4, 2011, at 17:53 , Heikki Linnakangas wrote: Under Linux, select() may report a socket file descriptor as "ready for reading", while nevertheless a subsequent read blocks. This could for example happen when data has arrived but upon examination has wrong checksum and is discarded. There may be other circumstances in which a file descriptor is spuriously reported as ready. Thus it may be safer to use O_NONBLOCK on sockets that should not block. >>> >>> So in theory, on Linux you might WaitLatch might sometimes incorrectly >>> return WL_POSTMASTER_DEATH. None of the callers check for >>> WL_POSTMASTER_DEATH return code, they call PostmasterIsAlive() before >>> assuming the postmaster has died, so that won't affect correctness at the >>> moment. I doubt that scenario can even happen in our case, select() on a >>> pipe that is never written to. But maybe we should add add an assertion to >>> WaitLatch to assert that if select() reports that the postmaster pipe has >>> been closed, PostmasterIsAlive() also returns false. >> >> The correct solution would be to read() from the pipe after select() >> returns, and only return WL_POSTMASTER_DEATCH if the read doesn't return >> EAGAIN. To prevent that read() from blocking if the read event was indeed >> spurious, O_NONBLOCK must be set on the pipe but that patch does that >> already. > > Let's have some perspective on this. We're talking about a highly > doubtful chance that latches may wake when they shouldn't. Yeah, as long as there's just a spurious wake up, sure. However, having WaitLatch() indicate a postmaster death in that case seems dangerous. Some caller, sooner or later, is bound to get it wrong, i.e. forget to re-check PostmasterIsAlive. > Maybe we should restore the return value of WaitLatch to its previous > format (so it doesn't return a bitmask)? That way, we don't report > that the Postmaster died, and therefore clients are required to call > PostmasterIsAlive() to be sure. That'd solve the issue too. > In any case, I'm in favour of the assertion. I don't really see the value in that assertion. It'd cause spurious assertion failures in the case of spurious events reported by select(). If we do expect such event, we should close the hole instead of asserting. If we don't, then what's the point of the assert. BTW, do we currently retry the select() on EINTR (meaning a signal has arrived)? If we don't, that'd be an additional source of spurious returns from select. >> Btw, with the death-watch / life-sign / whatever infrastructure in place, >> shouldn't PostmasterIsAlive() be using that instead of getppid() / kill(0)? > > Hmm, maybe. That seems like a separate issue though, that can be > addressed with another patch. It does have the considerable > disadvantage of making Heikki's proposed assertion failure useless. Is > the implementation of PostmasterIsAlive() really a problem at the > moment? I'm not sure that there is currently a guarantee that PostmasterIsAlive will returns false immediately after select() indicates postmaster death. If e.g. the postmaster's parent is still running (which happens for example if you launch postgres via daemontools), the re-parenting of backends to init might not happen until the postmaster zombie has been vanquished by its parent's call of waitpid(). It's not entirely inconceivable for getppid() to then return the (dead) postmaster's pid until that waitpid() call has occurred. But agreed, this is probably best handled by a separate patch. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] %ENV warnings during builds
On 07/03/2011 05:14 PM, Brar Piening wrote: schrieb Magnus Hagander: I think you've stumbled on just about all the bits of the MSVC build system we haven't perlized. Maybe we should complete that task, and turn clean.bat, pgbison.bat and pgflex.bat into pure one-line wrappers. (It was done for builddoc just a few weeks ago). Yeah, give nthat you can't get anything useful done without perl anyway, I don't see any argument for keeping them at this point. I've already stumbled into this while preparing the VS2010 support and came to the same conclusion. In my VS2010 support patch I've already created perl replacements for those two and removed the batch files completely. Certainly those two could also stay around as mere wrappers but in my opinion they only mess up the directory without adding any relevant benefit. Hmm, I missed that you had done this. Here are two replacement perl scripts I knocked up, but haven't yet tested. One of the things about them is that they remove knowledge of particular .l and .y files. and instead get the required invocation options from the relevant Makefiles. I think that's a lot better than the horrid hackery we have in the batch files. cheers andrew pgflex.pl Description: Perl program pgbison.pl Description: Perl program -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
On 4 July 2011 16:53, Heikki Linnakangas wrote: > Ok, here's a new patch, addressing the issues Fujii raised, and with a bunch > of stylistic changes of my own. Also, I committed a patch to remove > silent_mode, so the fork_process() changes are now gone. I'm going to sleep > over this and review once again tomorrow, and commit if it still looks good > to me and no-one else reports new issues. Looks good. > I don't like the names POSTMASTER_FD_WATCH and POSTMASTER_FD_OWN. At a quick > glance, it's not at all clear which is which. I couldn't come up with better > names, so for now I just added some comments to clarify that. I would find > WRITE/READ more clear, but to make sense of that you need to how the pipe is > used. Any suggestions or opinions on that? We could bikeshed about that until the cows come home, but we're not going to come up with names that make the purpose of each evident at a glance - it's too involved. If we could, we would have thought of them already. Besides, I've probably already written all the client code those macros will ever have. On 4 July 2011 17:36, Florian Pflug wrote: > On Jul4, 2011, at 17:53 , Heikki Linnakangas wrote: >>> Under Linux, select() may report a socket file descriptor as "ready >>> for >>> reading", while nevertheless a subsequent read blocks. This could >>> for >>> example happen when data has arrived but upon examination has >>> wrong >>> checksum and is discarded. There may be other circumstances in which >>> a >>> file descriptor is spuriously reported as ready. Thus it may be >>> safer >>> to use O_NONBLOCK on sockets that should not block. >> >> So in theory, on Linux you might WaitLatch might sometimes incorrectly >> return WL_POSTMASTER_DEATH. None of the callers check for >> WL_POSTMASTER_DEATH return code, they call PostmasterIsAlive() before >> assuming the postmaster has died, so that won't affect correctness at the >> moment. I doubt that scenario can even happen in our case, select() on a >> pipe that is never written to. But maybe we should add add an assertion to >> WaitLatch to assert that if select() reports that the postmaster pipe has >> been closed, PostmasterIsAlive() also returns false. > > The correct solution would be to read() from the pipe after select() > returns, and only return WL_POSTMASTER_DEATCH if the read doesn't return > EAGAIN. To prevent that read() from blocking if the read event was indeed > spurious, O_NONBLOCK must be set on the pipe but that patch does that already. Let's have some perspective on this. We're talking about a highly doubtful chance that latches may wake when they shouldn't. Latches are typically expected to wake up for a variety of reasons, and if that occurred in the archiver's case with my patch applied, I think we'd just go asleep again without anything happening. It seems likely that latch client code in general will never trip up on this, as long as its not exclusively relying on the waitLatch() return value to report pm death. Maybe we should restore the return value of WaitLatch to its previous format (so it doesn't return a bitmask)? That way, we don't report that the Postmaster died, and therefore clients are required to call PostmasterIsAlive() to be sure. In any case, I'm in favour of the assertion. > Btw, with the death-watch / life-sign / whatever infrastructure in place, > shouldn't PostmasterIsAlive() be using that instead of getppid() / kill(0)? Hmm, maybe. That seems like a separate issue though, that can be addressed with another patch. It does have the considerable disadvantage of making Heikki's proposed assertion failure useless. Is the implementation of PostmasterIsAlive() really a problem at the moment? -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Perl 5.8 requirement for compiling on windows?
On 04.07.2011 19:40, Marios Vodas wrote: On 4/7/2011 7:33 μμ, Heikki Linnakangas wrote: On 04.07.2011 19:30, Marios Vodas wrote: I want to ask why is there a requirement for 5.8 version of Perl to compile under windows? In this page of the documentation http://www.postgresql.org/docs/9.0/static/install-windows-full.html#AEN23848it sais: "Note: version 5.8 is required". I was able to compile PostgreSQL 9.0.4 using Active Perl 5.14 (only some warnings but the build was successful). Should I necessarily use 5.8? Surely that means 5.8 or later. Given that 5.8 is very old by now, perhaps we should just remove that sentence. > Yes, or at least change it by adding "or later". It is confusing without > it... Ok, I've fixed the docs to say "or later". I did it somewhat arbitrarily back to 8.3, as that's as far as "git cherry-pick" worked without conflicts. -- 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] non-superuser reserved connections? connection pools?
On Jul 4, 2011, at 10:09, Magnus Hagander wrote:
> On Mon, Jul 4, 2011 at 00:01, Michael Glaesemann wrote:
>> It would be nice to be able to set aside a few connections for
>> non-superusers, such as stats-monitoring connections. There's often no
>> reason to grant these users superuser privileges (they're just observers,
>> and security-definer functions can make anything visible that they may
>> need)), but at the same time you want them to be able to connect even when
>> the "normal" pool of slots may be full.
>> connection_pools={stats=3,superuser=10}
>> max_connections=100
>>
>> The connections allotted to "superuser" would have the same meaning as the
>> current superuser_reserved_connections GUC.
>>
>> Does this seem to be a useful feature to anyone else?
>
> Yeah, I'd definitely find it useful. Gives you a bit more flexibility
> than just using connection limiting on the individual user (though in
> your specific case here, that might work, if all your stats processes
> are with the same user).
Good point. It's another way to think of managing connections.
Michael Glaesemann
grzm seespotcode net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] avoid including rel.h in execnodes.h
Excerpts from Tom Lane's message of vie jul 01 18:20:50 -0400 2011: > Alvaro Herrera writes: > > I also included rel.h in spi.h, because it was previously indirectly > > included via execnodes.h and with this patch it would no longer be, > > which is a problem because it'd cause external code to fail to compile. > > If we think that not including rel.h unnecessarily is a good thing, then > that should surely apply to external code as well. So -1 for that bit. > It's not like we have not removed stuff from spi.h before. Thanks, committed with that change. -- Álvaro Herrera 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] [BUGS] BUG #6083: psql script line numbers incorrectly count \copy data
On Mon, Jul 04, 2011 at 12:02:12PM -0400, Tom Lane wrote: > "Steve Haslam" writes: > > ... Apparently, the data read from \copy is incrementing the > > script line number counter? > > Yeah, so it is. That is correct behavior for COPY FROM STDIN, but > not so much for copying from a separate file. > > The attached patch seems like an appropriate fix. However, I'm > unsure whether to apply it to released branches ... does anyone > think this might break somebody's application? No. Cheers, David. -- David Fetter 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] keepalives_* parameters usefullness
Simon Riggs writes: > On Mon, Jul 4, 2011 at 9:42 AM, Jaime Casanova wrote: > >> i even used getsockopt() to ensure TCP_KEEPIDLE was being setted and >> tried to set it myself with setsockopt() with the same results. > > There's a list of preconditions as to when these settings take effect, > otherwise they are a no-op. > do we know what those preconditions are? TFM just says: """ keepalives_idle Controls the number of seconds of inactivity after which TCP should send a keepalive message to the server. A value of zero uses the system default. This parameter is ignored for connections made via a Unix-domain socket, or if keepalives are disabled. It is only supported on systems where the TCP_KEEPIDLE or TCP_KEEPALIVE socket option is available, and on Windows; on other systems, it has no effect. """ which made me think that on my debian system it should work so maybe we want to document those preconditions, even a statement indicating that other factors can prevent this working as expected would be good -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL Soporte 24x7, desarrollo, capacitación y servicios -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Crash dumps
Tom Lane Monday 04 of July 2011 16:32:32 > Craig Ringer writes: > > Why not produce a tool that watches the datadir for core files and > > processes them? ... > > By and large, our attitude has been that Postgres shouldn't be crashing > often enough to make this sort of infrastructure worthwhile. Developer > time spent on it would be far better spent on fixing the bugs instead. > > > For that reason, it'd be handy if a backend could trap SIGSEGV and > > reliably tell the postmaster "I'm crashing!" so the postmaster could > > fork a helper to capture any additional info the backend needs to be > > alive for. ... > > Unfortunately, "reliably" and "segfault" don't go together. > > Yeah. I think there's no chance at all that we'd accept patches pointed > in this direction. They'd be more likely to decrease the system's > reliability than anything else. Aside from the difficulty of doing > anything at all reliably in an already-failing process, once we realize > that something is badly wrong it's important to kill off all other > backends ASAP. That reduces the window for any possible corruption of > shared memory to make it into on-disk state. So interposing a "helper" > to fool around with the failed process doesn't sound good at all. > > In practice I think you can generally get everything of interest > out of the core file, so it's not clear to me that there's any win > available from this line of thought anyhow. > > regards, tom lane I asked about crash reports becaus of at this time there was thread about crashing in live system. There is one win, I think, users will faster send crash report, then core dump (from many reasons, size, number of confidential information, etc). Such report may be quite usefull, for "reasonable" bug finding. It may be attached as contrib too, but I noticed and we agree that report generation should not affect speed of shoutdown. PostgreSQL will look better - server application that generates crash reports looks better then no generating. Just a bit of marketing ;) Regards, Radek. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl 5.8 requirement for compiling on windows?
Yes, or at least change it by adding "or later". It is confusing without it... Thank you for your response. On 4/7/2011 7:33 μμ, Heikki Linnakangas wrote: On 04.07.2011 19:30, Marios Vodas wrote: Hello, I want to ask why is there a requirement for 5.8 version of Perl to compile under windows? In this page of the documentation http://www.postgresql.org/docs/9.0/static/install-windows-full.html#AEN23848it sais: "Note: version 5.8 is required". I was able to compile PostgreSQL 9.0.4 using Active Perl 5.14 (only some warnings but the build was successful). Should I necessarily use 5.8? Surely that means 5.8 or later. Given that 5.8 is very old by now, perhaps we should just remove that sentence. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
On Jul4, 2011, at 17:53 , Heikki Linnakangas wrote: >> Under Linux, select() may report a socket file descriptor as "ready for >> reading", while nevertheless a subsequent read blocks. This could for >> example happen when data has arrived but upon examination has wrong >> checksum and is discarded. There may be other circumstances in which a >> file descriptor is spuriously reported as ready. Thus it may be safer >> to use O_NONBLOCK on sockets that should not block. > > So in theory, on Linux you might WaitLatch might sometimes incorrectly return > WL_POSTMASTER_DEATH. None of the callers check for WL_POSTMASTER_DEATH return > code, they call PostmasterIsAlive() before assuming the postmaster has died, > so that won't affect correctness at the moment. I doubt that scenario can > even happen in our case, select() on a pipe that is never written to. But > maybe we should add add an assertion to WaitLatch to assert that if select() > reports that the postmaster pipe has been closed, PostmasterIsAlive() also > returns false. The correct solution would be to read() from the pipe after select() returns, and only return WL_POSTMASTER_DEATCH if the read doesn't return EAGAIN. To prevent that read() from blocking if the read event was indeed spurious, O_NONBLOCK must be set on the pipe but that patch does that already. Btw, with the death-watch / life-sign / whatever infrastructure in place, shouldn't PostmasterIsAlive() be using that instead of getppid() / kill(0)? best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Perl 5.8 requirement for compiling on windows?
On 04.07.2011 19:30, Marios Vodas wrote: Hello, I want to ask why is there a requirement for 5.8 version of Perl to compile under windows? In this page of the documentation http://www.postgresql.org/docs/9.0/static/install-windows-full.html#AEN23848it sais: "Note: version 5.8 is required". I was able to compile PostgreSQL 9.0.4 using Active Perl 5.14 (only some warnings but the build was successful). Should I necessarily use 5.8? Surely that means 5.8 or later. Given that 5.8 is very old by now, perhaps we should just remove that sentence. -- 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] Perl 5.8 requirement for compiling on windows?
Hello, I want to ask why is there a requirement for 5.8 version of Perl to compile under windows? In this page of the documentation http://www.postgresql.org/docs/9.0/static/install-windows-full.html#AEN23848it sais: "Note: version 5.8 is required". I was able to compile PostgreSQL 9.0.4 using Active Perl 5.14 (only some warnings but the build was successful). Should I necessarily use 5.8? Thank you
Re: [HACKERS] [BUGS] BUG #6083: psql script line numbers incorrectly count \copy data
"Steve Haslam" writes: > ... Apparently, the data read from \copy > is incrementing the script line number counter? Yeah, so it is. That is correct behavior for COPY FROM STDIN, but not so much for copying from a separate file. The attached patch seems like an appropriate fix. However, I'm unsure whether to apply it to released branches ... does anyone think this might break somebody's application? regards, tom lane diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c index 5e69d29b6cbeab56aa0c85e85c3edce46d06efac..ebe5ee9ea551b858a4ad6119322405109d3212d8 100644 *** a/src/bin/psql/copy.c --- b/src/bin/psql/copy.c *** handleCopyIn(PGconn *conn, FILE *copystr *** 586,592 } } ! pset.lineno++; } } --- 586,593 } } ! if (copystream == pset.cur_cmd_source) ! pset.lineno++; } } -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Latch implementation that wakes on postmaster death on both win32 and Unix
Ok, here's a new patch, addressing the issues Fujii raised, and with a
bunch of stylistic changes of my own. Also, I committed a patch to
remove silent_mode, so the fork_process() changes are now gone. I'm
going to sleep over this and review once again tomorrow, and commit if
it still looks good to me and no-one else reports new issues.
There's two small issues left:
I don't like the names POSTMASTER_FD_WATCH and POSTMASTER_FD_OWN. At a
quick glance, it's not at all clear which is which. I couldn't come up
with better names, so for now I just added some comments to clarify
that. I would find WRITE/READ more clear, but to make sense of that you
need to how the pipe is used. Any suggestions or opinions on that?
The BUGS section of Linux man page for select(2) says:
Under Linux, select() may report a socket file descriptor as "ready for
reading", while nevertheless a subsequent read blocks. This could for
example happen when data has arrived but upon examination has wrong
checksum and is discarded. There may be other circumstances in which a
file descriptor is spuriously reported as ready. Thus it may be safer
to use O_NONBLOCK on sockets that should not block.
So in theory, on Linux you might WaitLatch might sometimes incorrectly
return WL_POSTMASTER_DEATH. None of the callers check for
WL_POSTMASTER_DEATH return code, they call PostmasterIsAlive() before
assuming the postmaster has died, so that won't affect correctness at
the moment. I doubt that scenario can even happen in our case, select()
on a pipe that is never written to. But maybe we should add add an
assertion to WaitLatch to assert that if select() reports that the
postmaster pipe has been closed, PostmasterIsAlive() also returns false.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***
*** 10165,10171 retry:
/*
* Wait for more WAL to arrive, or timeout to be reached
*/
! WaitLatch(&XLogCtl->recoveryWakeupLatch, 500L);
ResetLatch(&XLogCtl->recoveryWakeupLatch);
}
else
--- 10165,10171
/*
* Wait for more WAL to arrive, or timeout to be reached
*/
! WaitLatch(&XLogCtl->recoveryWakeupLatch, WL_LATCH_SET | WL_TIMEOUT, 500L);
ResetLatch(&XLogCtl->recoveryWakeupLatch);
}
else
*** a/src/backend/port/unix_latch.c
--- b/src/backend/port/unix_latch.c
***
*** 93,98
--- 93,99
#endif
#include "miscadmin.h"
+ #include "postmaster/postmaster.h"
#include "storage/latch.h"
#include "storage/shmem.h"
***
*** 179,209 DisownLatch(volatile Latch *latch)
* Wait for given latch to be set or until timeout is exceeded.
* If the latch is already set, the function returns immediately.
*
! * The 'timeout' is given in microseconds, and -1 means wait forever.
! * On some platforms, signals cause the timeout to be restarted, so beware
! * that the function can sleep for several times longer than the specified
! * timeout.
*
* The latch must be owned by the current process, ie. it must be a
* backend-local latch initialized with InitLatch, or a shared latch
* associated with the current process by calling OwnLatch.
*
! * Returns 'true' if the latch was set, or 'false' if timeout was reached.
*/
! bool
! WaitLatch(volatile Latch *latch, long timeout)
{
! return WaitLatchOrSocket(latch, PGINVALID_SOCKET, false, false, timeout) > 0;
}
/*
! * Like WaitLatch, but will also return when there's data available in
! * 'sock' for reading or writing. Returns 0 if timeout was reached,
! * 1 if the latch was set, 2 if the socket became readable or writable.
*/
int
! WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
! bool forWrite, long timeout)
{
struct timeval tv,
*tvp = NULL;
--- 180,211
* Wait for given latch to be set or until timeout is exceeded.
* If the latch is already set, the function returns immediately.
*
! * The 'timeout' is given in microseconds. It must be >= 0 if WL_TIMEOUT
! * event is given, otherwise it is ignored. On some platforms, signals cause
! * the timeout to be restarted, so beware that the function can sleep for
! * several times longer than the specified timeout.
*
* The latch must be owned by the current process, ie. it must be a
* backend-local latch initialized with InitLatch, or a shared latch
* associated with the current process by calling OwnLatch.
*
! * Returns bit field indicating which condition(s) caused the wake-up. Note
! * that if multiple wake-up conditions are true, there is no guarantee that
! * we return all of them in one call, but we will return at least one.
*/
! int
! WaitLatch(volatile Latch *latch, int wakeEvents, long timeout)
{
! return WaitLatchOrSocket(latch
[HACKERS] proper format for printing GetLastError()
About half of our code prints GetLastError() using %d after casting it to int (actually, about half of that half uses %i, another thing to sort out, perhaps), and the other half uses %lu without casting. I gather from online documentation that GetLastError() returns DWORD, which appears to be unsigned 32 bits. So using %lu appears to be more correct. Any arguments against standardizing on %lu? Secondly, it might also be good if we could standardize on printing actual message: error code %lu instead of just actual message: %lu Thirdly, why are we not trying to print a textual message? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Potential NULL dereference found in typecmds.c
Excerpts from Heikki Linnakangas's message of lun jul 04 09:14:11 -0400 2011:
> On 04.07.2011 16:07, Peter Geoghegan wrote:
> That error message is bogus anyway:
>
> > if (!found)
> > ereport(ERROR,
> > (errcode(ERRCODE_UNDEFINED_OBJECT),
> > errmsg("constraint \"%s\" of domain \"%s\" does not exist",
> > constrName, NameStr(con->conname;
>
> It passes con->conname as the name of the domain, which is just wrong.
> It should be TypeNameToString(typename) instead. The second error
> message that follows has the same bug.
Um, evidently this code has a problem. I'll fix.
--
Álvaro Herrera
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] Problem installing odbc and .Net drivers on Windows 7 64 Ultimate
On 4 July 2011 15:57, Michael Gould
wrote:
> I am getting the following error when I run the install from stackbuilder.
>
> Error trying to install file destination ${installdir} resolves to empty
> value.
>
>
>
> Does anyone know what might be causing this and how I can fix it.
This is the developer's mailing list. You should ask this question on
the pgsql-general mailing list.
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
--
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 installing odbc and .Net drivers on Windows 7 64 Ultimate
I am getting the following error when I run the install from stackbuilder.
Error trying to install file destination ${installdir} resolves to empty
value.
Does anyone know what might be causing this and how I can fix it.
Best Regards
Michael Gould
Re: [HACKERS] Crash dumps
Craig Ringer writes: > Why not produce a tool that watches the datadir for core files and > processes them? ... By and large, our attitude has been that Postgres shouldn't be crashing often enough to make this sort of infrastructure worthwhile. Developer time spent on it would be far better spent on fixing the bugs instead. > For that reason, it'd be handy if a backend could trap SIGSEGV and > reliably tell the postmaster "I'm crashing!" so the postmaster could > fork a helper to capture any additional info the backend needs to be > alive for. ... > Unfortunately, "reliably" and "segfault" don't go together. Yeah. I think there's no chance at all that we'd accept patches pointed in this direction. They'd be more likely to decrease the system's reliability than anything else. Aside from the difficulty of doing anything at all reliably in an already-failing process, once we realize that something is badly wrong it's important to kill off all other backends ASAP. That reduces the window for any possible corruption of shared memory to make it into on-disk state. So interposing a "helper" to fool around with the failed process doesn't sound good at all. In practice I think you can generally get everything of interest out of the core file, so it's not clear to me that there's any win available from this line of thought anyhow. 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] non-superuser reserved connections? connection pools?
On Mon, Jul 4, 2011 at 00:01, Michael Glaesemann wrote:
> It would be nice to be able to set aside a few connections for
> non-superusers, such as stats-monitoring connections. There's often no reason
> to grant these users superuser privileges (they're just observers, and
> security-definer functions can make anything visible that they may need)),
> but at the same time you want them to be able to connect even when the
> "normal" pool of slots may be full.
>
> I googled a bit, assuming there had been discussion of something similar in
> the past, but didn't find anything.
>
> I'm not sure what configuration would look like. Perhaps there's a
> generalized "connection pool" concept that's missing, extending the current
> "superuser" connection pool specified by the superuser_reserved_connections
> GUC something like:
>
> CREATE CONNECTION POOL stats WITH LIMIT 10; -- 40 connections allotted for
> the foo connection pool.
> ALTER ROLE nagios WITH CONNECTION POOL foo; -- the nagios role is allowed to
> take a connection from the foo connection pool.
>
> Right now, of course, connection limits are set in postgresql.conf and only
> alterable on restart, so perhaps there could be a connection_pools GUC,
> something along the lines of:
>
> connection_pools={stats=3,superuser=10}
> max_connections=100
>
> The connections allotted to "superuser" would have the same meaning as the
> current superuser_reserved_connections GUC.
>
> Does this seem to be a useful feature to anyone else?
Yeah, I'd definitely find it useful. Gives you a bit more flexibility
than just using connection limiting on the individual user (though in
your specific case here, that might work, if all your stats processes
are with the same user).
No comments on the implementation suggestions at this point ;)
--
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] Crash dumps
Information if backend crashed should go fast to master, to kill others as fast as possible. This what i thought is to use socket urgent data, but this require to span small thread in master (i think oob data may not be processed in secure way). From one hand processing core dump may be good, but from other hand those may take huge area. Using it in any case will require to build PostgreSQL with debugging symbols. Regards, Radoslaw Smogura (mobile) -Original Message- From: Craig Ringer Sent: 4 lipca 2011 13:57 To: Radosław Smogura Cc: PG Hackers Subject: Re: [HACKERS] Crash dumps On 4/07/2011 7:03 PM, Radosław Smogura wrote: > Actually this, what I was thinking about was, to add dumping of GUC, > etc. List of mappings came from when I tired to mmap PostgreSQL, and due > to many of errors, which sometimes occurred in unexpected places, I was > in need to add something that will be useful for me and easy to analyse > (I could simple find pointer, and then check which region failed). The > idea to try to evolve this come later. Why not produce a tool that watches the datadir for core files and processes them? Most but not all of the info you listed should be able to be extracted from a core file. Things like GUCs should be extractable with a bit of gdb scripting - and with much less chance of crashing than trying to read them from a possibly corrupt heap within a crashing backend. To capture any information not available from the core, you can enlist the postmaster's help. It gets notified when a child crashes and should be able to capture things like the memory and disk state. See void reaper(SIGNAL_ARGS) in postmaster.c and HandleChildCrash(...) . If nothing else, the postmaster could probably fork a "child crashed" helper to collect data, analyse the core file, email the report to the admin, etc. About the only issue there is that the postmaster relies on the exit status to trigger the reaper code. Once an exit status is available, the crashed process is gone, so the free memory will reflect the memory state after the backend dies, and shared memory's state will have moved on from how it was when the backend was alive. For that reason, it'd be handy if a backend could trap SIGSEGV and reliably tell the postmaster "I'm crashing!" so the postmaster could fork a helper to capture any additional info the backend needs to be alive for. Then the helper can gcore() the backend, or the backend can just clear the SIGSEGV handler and kill(11) its self to keep on crashing and generate a core. Unfortunately, "reliably" and "segfault" don't go together. You don't want a crashing postmaster writing to shared memory so it can't use shm to tell the postmaster it's dying. Signals are ... interesting ... at the best of times, but would probably still be the best bet. The postmaster could install a SIGUSR[whatever] or RT signal handler that takes a siginfo so it knows the pid of the signal sender. The crashing backend could signal the postmaster with an agreed signal to say "I'm crashing" and let the postmaster clean it up. The problem with this is that a lost signal (for any reason) would cause a zombie backend to hang around waiting to be killed by a postmaster that never heard it was crashing. BTW, the win32 crash dump handler would benefit from being able to use some of the same facilities. In particular, being able to tell the postmaster "Argh, ogod I'm crashing, fork something to dump my core!" rather than trying to self-dump would be great. It'd also allow the addition of extra info like GUC data, last few lines of logs etc to the minidump, something that the win32 crash dump handler cannot currently do safely. -- Craig Ringer -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Potential NULL dereference found in typecmds.c
On 04.07.2011 16:07, Peter Geoghegan wrote:
On 4 July 2011 13:53, Magnus Hagander wrote:
This code is no longer present in git head, *removed* by commit
426cafc. Not added by it. at least that's how I read the history...
However, it still looks to me like we could get to that code with
con=NULL - if the while loop is never executed. Perhaps this is a
can-never-happen situation? Alvaro?
Seems slightly academic IMHO. No code path should dereference an
invariably NULL or wild pointer.
That error message is bogus anyway:
if (!found)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("constraint \"%s\" of domain \"%s\" does not
exist",
constrName,
NameStr(con->conname;
It passes con->conname as the name of the domain, which is just wrong.
It should be TypeNameToString(typename) instead. The second error
message that follows has the same bug.
--
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] Potential NULL dereference found in typecmds.c
On 4 July 2011 13:53, Magnus Hagander wrote: > This code is no longer present in git head, *removed* by commit > 426cafc. Not added by it. at least that's how I read the history... > > However, it still looks to me like we could get to that code with > con=NULL - if the while loop is never executed. Perhaps this is a > can-never-happen situation? Alvaro? Seems slightly academic IMHO. No code path should dereference an invariably NULL or wild pointer. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Potential NULL dereference found in typecmds.c
On Sat, Jul 2, 2011 at 20:10, Michael Mueller wrote:
> Hi folks,
>
> Sentry found this error last night, and it looks serious enough to
> report. The error was introduced in commit 426cafc. Here's the code
> in question, starting at line 2096:
>
> if (!found)
> {
> con = NULL; /* keep compiler quiet */
> ereport(ERROR,
> (errcode(ERRCODE_UNDEFINED_OBJECT),
> errmsg("constraint \"%s\" of domain \"%s\" does not exist",
> constrName, NameStr(con->conname;
> }
>
> It sets 'con' to NULL and then in the next statement, dereferences it.
> I'm not sure if it's possible to reach this path, but if it is
> reachable it will cause a crash.
This code is no longer present in git head, *removed* by commit
426cafc. Not added by it. at least that's how I read the history...
However, it still looks to me like we could get to that code with
con=NULL - if the while loop is never executed. Perhaps this is a
can-never-happen situation? Alvaro?
--
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] per-column generic option
(2011/07/04 10:17), Shigeru Hanada wrote:
> (2011/07/03 18:50), Kohei KaiGai wrote:
>> I checked the per-column generic option patch.
>> Right now, I have nothing to comment on anymore.
>> So, it should be reviewed by committers.
>
> Thanks for the review!.
I would like to propose adding force_not_null support to file_fdw, as
the first use case of per-column FDW option. Attached patch, which
assumes that per_column_option_v3.patch has been applied, implements
force_not_null option as per-column FDW option.
Overview
This option is originally supported by COPY FROM command, so I think
it's reasonable to support it in file_fdw too. It would provides more
flexible parsing capability. In fact, this option has been supported
by the internal routines which are shared with COPY FROM, but currently
we don't have any way to specify it.
Difference between COPY
===
For COPY FROM, FORCE_NOT_NULL is specified as a list of column names
('*' is not allowed). For file_fdw, per-table FDW option can be used
like other options, but then file_fdw needs parser which can identify
valid column. I think it's too much work, so I prefer per-column FDW
option which accepts boolean value string. The value 'true' means that
the column doesn't be matched against NULL string, same as ones listed
for COPY FROM.
Example:
If you have created a foreign table with:
CREATE FOREIGN TABLE foo (
c1 int OPTIONS (force_not_null 'false'),
c2 text OPTIONS (force_not_null 'true')
) SERVER file OPTIONS (file '/path/to/file', format 'csv', null '');
values which are read from the file for c1 are matched against
null-representation-string '', but values for c2 are NOT. Empty strings
for c2 are stored as empty strings; they don't treated as NULL value.
Regards,
--
Shigeru Hanada
diff --git a/contrib/file_fdw/data/text.csv b/contrib/file_fdw/data/text.csv
index ...bd4c327 .
*** a/contrib/file_fdw/data/text.csv
--- b/contrib/file_fdw/data/text.csv
***
*** 0
--- 1,4
+ 123,123
+ abc,abc
+ NULL,NULL
+ ABC,ABC
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 466c015..caf9c86 100644
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***
*** 23,29
--- 23,31
#include "foreign/fdwapi.h"
#include "foreign/foreign.h"
#include "miscadmin.h"
+ #include "nodes/makefuncs.h"
#include "optimizer/cost.h"
+ #include "utils/syscache.h"
PG_MODULE_MAGIC;
*** static struct FileFdwOption valid_option
*** 56,71
{"escape", ForeignTableRelationId},
{"null", ForeignTableRelationId},
{"encoding", ForeignTableRelationId},
/*
* force_quote is not supported by file_fdw because it's for COPY TO.
*/
- /*
-* force_not_null is not supported by file_fdw. It would need a parser
-* for list of columns, not to mention a way to check the column list
-* against the table.
-*/
/* Sentinel */
{NULL, InvalidOid}
--- 58,69
{"escape", ForeignTableRelationId},
{"null", ForeignTableRelationId},
{"encoding", ForeignTableRelationId},
+ {"force_not_null", AttributeRelationId},/* specified as boolean
value */
/*
* force_quote is not supported by file_fdw because it's for COPY TO.
*/
/* Sentinel */
{NULL, InvalidOid}
*** static void fileGetOptions(Oid foreignta
*** 111,116
--- 109,115
static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel,
const char *filename,
Cost *startup_cost, Cost *total_cost);
+ static List * get_force_not_null(Oid relid);
/*
*** file_fdw_validator(PG_FUNCTION_ARGS)
*** 144,149
--- 143,149
List *options_list = untransformRelOptions(PG_GETARG_DATUM(0));
Oid catalog = PG_GETARG_OID(1);
char *filename = NULL;
+ char *force_not_null = NULL;
List *other_options = NIL;
ListCell *cell;
*** file_fdw_validator(PG_FUNCTION_ARGS)
*** 197,203
buf.data)));
}
! /* Separate out filename, since ProcessCopyOptions won't allow
it */
if (strcmp(def->defname, "filename") == 0)
{
if (filename)
--- 197,206
buf.data)));
}
! /*
!* Separate out filename and force_not_null, since
ProcessCopyOptions
!* won't allow them.
!*/
if (strcmp(def->defname, "filename") == 0)
{
if (filename)
*** file_fdw_validator(PG_FUNCTION_ARGS)
*** 206,211
--- 209,228 --
Re: [HACKERS] Crash dumps
On 4/07/2011 7:03 PM, Radosław Smogura wrote: Actually this, what I was thinking about was, to add dumping of GUC, etc. List of mappings came from when I tired to mmap PostgreSQL, and due to many of errors, which sometimes occurred in unexpected places, I was in need to add something that will be useful for me and easy to analyse (I could simple find pointer, and then check which region failed). The idea to try to evolve this come later. Why not produce a tool that watches the datadir for core files and processes them? Most but not all of the info you listed should be able to be extracted from a core file. Things like GUCs should be extractable with a bit of gdb scripting - and with much less chance of crashing than trying to read them from a possibly corrupt heap within a crashing backend. To capture any information not available from the core, you can enlist the postmaster's help. It gets notified when a child crashes and should be able to capture things like the memory and disk state. See void reaper(SIGNAL_ARGS) in postmaster.c and HandleChildCrash(...) . If nothing else, the postmaster could probably fork a "child crashed" helper to collect data, analyse the core file, email the report to the admin, etc. About the only issue there is that the postmaster relies on the exit status to trigger the reaper code. Once an exit status is available, the crashed process is gone, so the free memory will reflect the memory state after the backend dies, and shared memory's state will have moved on from how it was when the backend was alive. For that reason, it'd be handy if a backend could trap SIGSEGV and reliably tell the postmaster "I'm crashing!" so the postmaster could fork a helper to capture any additional info the backend needs to be alive for. Then the helper can gcore() the backend, or the backend can just clear the SIGSEGV handler and kill(11) its self to keep on crashing and generate a core. Unfortunately, "reliably" and "segfault" don't go together. You don't want a crashing postmaster writing to shared memory so it can't use shm to tell the postmaster it's dying. Signals are ... interesting ... at the best of times, but would probably still be the best bet. The postmaster could install a SIGUSR[whatever] or RT signal handler that takes a siginfo so it knows the pid of the signal sender. The crashing backend could signal the postmaster with an agreed signal to say "I'm crashing" and let the postmaster clean it up. The problem with this is that a lost signal (for any reason) would cause a zombie backend to hang around waiting to be killed by a postmaster that never heard it was crashing. BTW, the win32 crash dump handler would benefit from being able to use some of the same facilities. In particular, being able to tell the postmaster "Argh, ogod I'm crashing, fork something to dump my core!" rather than trying to self-dump would be great. It'd also allow the addition of extra info like GUC data, last few lines of logs etc to the minidump, something that the win32 crash dump handler cannot currently do safely. -- Craig Ringer -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Initial Review: JSON contrib modul was: Re: [HACKERS] Another swing at JSON
--On 18. Juni 2011 12:29:38 +0200 Bernd Helmle wrote:
Similar problems occur with a couple other modules I tried (hstore,
intarray).
Hmm, works for me. Seems you have messed up your installation in some way
(build against current -HEAD but running against a 9.1?).
I'm going to review in the next couple of days again.
A bit later than expected, but here is my summary on the patch:
-- Patch Status
The patch applies cleanly. Since it's primarily targeted as an addition to
contrib/, it doesn't touch the backend source tree (besides documentation TOC
entries), but adds a new subdirectory in contrib/json. The patch is in context
diff as requested.
-- Functionality
The patch as is provides a basic implementation for a JSON datatype. It
includes normalization and validation of JSON strings. It adds the datatype
implementation as an extension.
The implementation focus on the datatype functionality only, there is no
additional support for special operators. Thus, i think the comments in the
control file (and docs) promises actually more than the contrib module will
deliver:
+comment = 'data type for storing and manipulating JSON content'
I'm not sure, if "manipulating" is a correct description. Maybe i missed it,
but i didn't see functions to manipulate JSON strings directly, for example,
adding an object to a JSON string, ...
-- Coding
The JSON datatype is implemented as a variable length character type, thus
allows very large JSON strings to be stored. It transparently decides when to
escape unicode code points depending on the selected client- and
server-encoding.
Validation is done through its own JSON parser. The parser itself is a
recursive descent parser. It is noticable that the parser seems to define its
own is_space() and is_digit() functions, e.g.:
+#define is_space(c) ((c) == '\t' || (c) == '\n' || (c) == '\r' || (c) == ' ')
Wouldn't it be more clean to use isspace() instead?
This code block in function json_escape_unicode() looks suspicious:
+ /* Convert to UTF-8, if necessary. */
+ {
+ const char *orig = json;
+ json = (const char *)
+ pg_do_encoding_conversion((unsigned char *) json, length,
+ GetDatabaseEncoding(), PG_UTF8);
+ if (json != orig)
+ length = strlen(json);
+ }
Seems it *always* wants to convert the string. Isn't there a "if" condition
missing?
There's a commented code fragment missing, which should be removed (see last
two lines of the snippet below):
+static unsigned int
+read_hex16(const char *in)
+{
+ unsigned int i;
+ unsigned int tmp;
+ unsigned int ret = 0;
+
+ for (i = 0; i < 4; i++)
+ {
+ char c = *in++;
+
+ Assert(is_hex_digit(c));
+
+ if (c >= '0' && c <= '9')
+ tmp = c - '0';
+ else if (c >= 'A' && c <= 'F')
+ tmp = c - 'A' + 10;
+ else /* if (c >= 'a' && c <= 'f') */
+ tmp = c - 'a' + 10;
json.c introduces new appendStringInfo* functions, e.g.
appendStringInfoEscape() and appendStringInfoUtf8(). Maybe it's better
to name them to something different, since it may occur someday that the backend
will introduce the same notion with a different meaning. They are static,
but why not naming them into something like jsonAppendStringInfoUtf8() or
similar?
-- Regression Tests
The patch includes a fairly complete regression test suite, which covers much
of the formatting, validating and input functionality of the JSON datatype. It
also tests UNICODE sequences and input encoding with other server encoding than
UTF-8.
--
Thanks
Bernd
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Crash dumps
On Mon, 04 Jul 2011 12:58:46 +0800, Craig Ringer wrote: On 15/06/2011 2:37 AM, Radosław Smogura wrote: Hello, Because, I work a little bit on streaming protocol and from time to time I have crashes. I want ask if you wont crash reporting (this is one of minors products from mmap playing) those what I have there is mmaped areas, and call stacks, and some other stuff. Core files already contain all that, don't they? They omit shared memory segments by default on most platforms, but should otherwise be quite complete. The usual approach on UNIXes and linux is to use the built-in OS features to generate a core dump of a crashing process then analyze it after the fact. That way the crash is over as fast as possible and you can get services back up and running before spending the time, CPU and I/O required to analyze the core dump. Actually this, what I was thinking about was, to add dumping of GUC, etc. List of mappings came from when I tired to mmap PostgreSQL, and due to many of errors, which sometimes occurred in unexpected places, I was in need to add something that will be useful for me and easy to analyse (I could simple find pointer, and then check which region failed). The idea to try to evolve this come later. I think report should looks like: This is crash report of PostgreSQL database, generated on Here is list of GUC variables: Here is list of files: Here is backtrace: Here is detailed backtrace: Here is list of m-mappings (you may get what library are linked in) Here is your free memory Here is your disk usage Here is your custom addition This based reports works for Linux with gdb, but there is some pluggable architecture, which connects with segfault Which process does the debugging? Does the crashing process fork() a copy of gdb to debug its self? One thing I've been interested in is giving the postmaster (or more likely a helper for the postmaster) the ability to handle "backend is crashing" messages, attach a debugger to the crashing backend and generate a dump and/or backtrace. This might be workable in cases where in-process debugging can't be done due to a smashed stack, full heap causing malloc() failure, etc. Currently I do everything in segfault handler (no fork), but I like the idea of fork (in segfault), this may resolve some problems. Regards, Radosław Smogura -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cascade replication
On Tue, Jun 14, 2011 at 6:08 AM, Fujii Masao wrote: >> The standby must not accept replication connection from that standby itself. >> Otherwise, since any new WAL data would not appear in that standby, >> replication cannot advance any more. As a safeguard against this, I >> introduced >> new ID to identify each instance. The walsender sends that ID as the fourth >> field of the reply of IDENTIFY_SYSTEM, and then walreceiver checks whether >> the IDs are the same between two servers. If they are the same, which means >> that the standby is just connecting to that standby itself, so walreceiver >> emits ERROR. Thanks for waiting for review. This part of the patch is troubling me. I think you have identified an important problem, but this solution doesn't work fully. If we allow standbys to connect to other standbys then we have problems with standbys not being connected to master. This can occur with a 1-step connection, as you point out, but it could also occur with a 2-step, 3-step or more connection, where a circle of standbys are all depending upon each other. Your solution only works for 1-step connections. "Solving" that problem in a general sense might be more dangerous than leaving it alone. I think we should think some more about the issues there and approach them as a separate problem. I think we should remove that and just focus on the main problem, for now. That will make it a simpler patch and easier to commit. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] flex on win64 - workaround for "flex: fatal internal error, exec failed"
On 4/07/2011 2:36 PM, Brar Piening wrote:
As you might also want to use git for developing with postgres its
possible that all you need is already in place.
As I use msysgit my usual way of dealing with flex and bison
dependencies is putting "$ENV{PATH}=$ENV{PATH} . ';C:\Program Files
(x86)\Git\bin';" into my buildenv.pl
Good thought. I hadn't realized msysgit bundled flex and bison - that's
really handy. I generally use git from a `vsvars' cmd.exe shell not bash
simply because some tools don't play well with bash, and I only have git
on the PATH so I didn't realize flex was bundled. Handy!
I wonder if the docs should be recommending using msys's flex and bison
(via msysgit or mingw) rather than GnuWin32? Pointing to a matching m4,
flex and bison that're kept up to date and easy to install might be a
good thing.
--
Craig Ringer
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Online base backup from the hot-standby
> When the standby restarts after it crashes during recovery, it always > checks whether recovery has reached the backup end location by > using minRecoveryPoint even though the standby doesn't start from > the backup. This looks odd. Certainly. But, in this case, the state before recovery starts is lost. Therefore, postgres can not see that the backup got from whether standby server or master. What should? Should use pg_control? Ex. * Add 'Where to get backup' to pg_control. (default 'none') * When recovery starts, it checks it whether 'none'. * When minRecoveryPoint equals 0/0, change 'master'. * When minRecoveryPoint do not equals 0/0, change 'slave'. * When it reached the end of recovery, change 'none' . > - XLogRecPtrIsInvalid(ControlFile->backupStartPoint)) > + (XLogRecPtrIsInvalid(ControlFile->backupStartPoint) || > + reachedControlMinRecoveryPoint == true)) > The flag 'reachedControlMinRecoveryPoint' is really required? When recovery > reaches minRecoveryPoint, ControlFile->backupStartPoint is reset to zero. So > we can check whether recovery has reached minRecoveryPoint or not by only > doing XLogRecPtrIsInvalid(ControlFile->backupStartPoint). No? Yes. 'reachedControlMinRecoveryPoint' is unnecessary. > We should check if recovery has reached minRecoveryPoint before calling > CheckRecoveryConsistency() after reading new WAL record? Otherwise, > even if recovery has reached minRecoveryPoint, the standby cannot think > that it's in consistent state until it reads new WAL record. This is a same sequence with a case of backup end location. It should be no changed. > + if > (XLByteLT(ControlFile->minRecoveryPoint, EndRecPtr)) > + > ControlFile->minRecoveryPoint = EndRecPtr; > Why does ControlFile->minRecoveryPoint need to be set to EndRecPtr? Yes. I delete it. Jun Ishizuka NTT Software Corporation TEL：045-317-7018 E-Mail: ishizuka@po.ntts.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] keepalives_* parameters usefullness
On Mon, Jul 4, 2011 at 9:42 AM, Jaime Casanova wrote: > i even used getsockopt() to ensure TCP_KEEPIDLE was being setted and > tried to set it myself with setsockopt() with the same results. There's a list of preconditions as to when these settings take effect, otherwise they are a no-op. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] keepalives_* parameters usefullness
Hi,
AFAIU TFM if i set keepalives_* parameters in a conninfo they set the
internal counters to these values so if i execute:
"""
conn = PQconnectdb("host=192.168.204.10 keepalives=1 keepalives_idle=45
keepalives_interval=5 keepalives_count=5");
"""
that means that the client's connection to the server in 192.168.204.10
will wait 45 seconds after the last packet sent to the server and then
will sent a probe every 5 seconds, 5 times...
if all of the above is right then if in the client i execute a PQexec() when
the connection has already drop it should detect the failure after 1
minute 10 seconds. is that right or am i misunderstanding this?
the reason i ask is that when i use that exact conninfo it detects the
failure condition after 15 minutes always... well, actually it waits the
same time even if i doesn't set anything...
i even used getsockopt() to ensure TCP_KEEPIDLE was being setted and
tried to set it myself with setsockopt() with the same results.
BTW, this paper (http://es.scribd.com/doc/2586622/tcpkeepalivehowto) on
section "4.2. The setsockopt function call" page 9 says that to set
TCP_KEEPIDLE, TCP_KEEPINTVL and TCP_KEEPCNT we should use level SOL_TCP
but on src/interfaces/libpq/fe-connect.c we use IPPROTO_TCP instead.
any leads?
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL
Soporte 24x7, desarrollo, capacitación y servicios
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Full GUID support
Dave, This is wonderful news. Best Regards Michael Gould "Dave Page" wrote: > Should be in 9.0.5/9.1b3 > > On Sunday, July 3, 2011, Michael Gould > wrote: > > Does this look to be something that will surface around for 9.1 > > > > Sent from Samsung mobile > > > > Dave Page wrote: > > > >>On Sunday, July 3, 2011, Michael Gould > >> wrote: > >>> Peter, > >>> > >>> I don't believe that the library that the contrib > module runs with can run > >>> on Window 64 bit servers or even Windows 7 64 bit. > That is problem as most > >>> shops are using 64 bit OS and if Window the contrib module is > >>going to fail. > >>> Taking the responsibility to handle this internally > means that you can > >>> write your own implementation not based on a libary > that can't handle > >>> Windows 64 bit. > >> > >>The next release of the installers will, now that we have a 64 Windows > >>port of ossp-uuid. > >> > >>Even If that weren't the case, integrating the type wouldn't fix the > >>problem anyway, unless you're suggesting we implement our own UUID > >>generator (which isn't nearly as straightforward as it might seem, as > >>I understand it).. > >> > >>-- > >>Dave Page > >>Blog: http://pgsnake.blogspot.com > >>Twitter: @pgsnake > >> > >>EnterpriseDB UK: 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 > > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: 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 > -- Michael Gould, Managing Partner Intermodal Software Solutions, LLC 904.226.0978 904.592.5250 fax -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Potential NULL dereference found in typecmds.c
Hi folks,
Sentry found this error last night, and it looks serious enough to
report. The error was introduced in commit 426cafc. Here's the code
in question, starting at line 2096:
if (!found)
{
con = NULL; /* keep compiler quiet */
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("constraint \"%s\" of domain \"%s\" does not exist",
constrName, NameStr(con->conname;
}
It sets 'con' to NULL and then in the next statement, dereferences it.
I'm not sure if it's possible to reach this path, but if it is
reachable it will cause a crash.
Best Regards,
Mike
--
Mike Mueller
Phone: (401) 405-1525
Email: mmuel...@vigilantsw.com
http://www.vigilantsw.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
