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] 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 Pflugf...@phlo.org 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 stdio.h
#include unistd.h
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
On Mon, Jul 4, 2011 at 23:30, Andrew Dunstan and...@dunslane.net wrote: 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. Definitely agreed. Those were ugly hacks that were supposed to be temporary, but yeah, we all know what happens to temporary things :O -- 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] Online base backup from the hot-standby
What about using backupStartPoint to check whether this recovery started from the backup or not? No, postgres can check whether this recovery started from the backup or not, but can not check whether standby server or master (got backup from). Once recovery started, backupStartPoint is recorded to pg_control until recovery reaches backup end location, it is not related to any backup server. 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] Latch implementation that wakes on postmaster death on both win32 and Unix
On 5 July 2011 07:49, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: 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. Hmm. Why assume that the opposite problem doesn't exist? What if the kill-based test is faster than the read() on the pipe on some platform or under some confluence of events? I suggest that we agree on a standard for determining whether or not the postmaster is dead and stick to it - that's already the case on Windows. Since that standard cannot be the kill() based test, because that would make a postmaster death aware latch implementation impossible, it has to be the read() test proposed by Florian. -- 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] Online base backup from the hot-standby
2011/7/5 Jun Ishiduka ishizuka@po.ntts.co.jp: What about using backupStartPoint to check whether this recovery started from the backup or not? No, postgres can check whether this recovery started from the backup or not, but can not check whether standby server or master (got backup from). Oh, right. We cannot distinguish the following two cases just by using minRecoveryPoint and backupStartPoint. * The standby starts from the backup taken from the standby * The standby starts after it crashes during recovering from the backup taken from the master As you proposed, adding new field which stores the backup end location taken from minRecoveryPoint, into pg_control sounds good idea. 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] Inconsistency between postgresql.conf and docs
On Thu, Jun 30, 2011 at 1:34 AM, Josh Berkus j...@agliodbs.com wrote: My preference would be to have: # REPLICATION # - Master Settings - # these settings affect the master role in replication # they will be ignored on the standby ... settings ... # - Standby Settings - # these settings affect the standby role in replication # they will be ignored on the master ... settings ... That's how I've been setting up the file for my customers; it's fairly clear and understandable. Looks better than it's now. Anyway, if you change those, you would need to change also the config_group in guc.c. 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 Tue, Jul 5, 2011 at 4:34 AM, Fujii Masao masao.fu...@gmail.com wrote: On Mon, Jul 4, 2011 at 6:24 PM, Simon Riggs si...@2ndquadrant.com wrote: On Tue, Jun 14, 2011 at 6:08 AM, Fujii Masao masao.fu...@gmail.com 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! I agree to focus on the main problem first. I removed that. Attached is the updated version. Now for the rest of the review... I'd rather not include another chunk of code related to wal_keep_segments. The existing code in CreateCheckPoint() should be refactored so that we call the same code from both CreateCheckPoint() and CreateRestartPoint(). IMHO it's time to get rid of RECOVERYXLOG as an initial target for de-archived files. That made sense once, but now we have streaming it makes more sense for us to de-archive straight onto the correct file name and let the file be cleaned up later. So de-archiving it and then copying to the new location doesn't seem the right thing to do (especially not to copy rather than rename). RECOVERYXLOG allowed us to de-archive the file without removing a pre-existing file, so we must handle that still - the current patch would fail if a pre-existing WAL file were there. Those changes will make this code cleaner for the long term. I don't think we should simply shutdown a WALSender when we startup. That is indistinguishable from a failure, which is going to be very worrying if we do a switchover. Is there another way to do this? Or if not, at least a log message to explain it was normal that we requested this. It would be possible to have synchronous cascaded replication but that is probably another patch :-) -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parameterized aggregate subquery (was: Pull up aggregate subquery)
Hello Hitosh, list, Attached is revised version. I failed to attached the patch. I'm trying again. I'm currently unable to test, since on holiday. I'm happy to continue testing once returned but it may not be within the bounds of the current commitfest, sorry. 5) Regression tests are missing; I think at this point they'd aid in speeding up development/test cycles. I'm still thinking about it. I can add complex test but the concept of regression test focuses on small pieces of simple cases. I don't want take pg_regress much more than before. IMHO, at least one query where the new code is touched is a good idea. I get the same error with the version from june 9 with current git HEAD. Fixed. Some variable initializing was wrong. Thanks - will test when I am back from holiday and other duties. regards, Yeb
Re: [HACKERS] Parameterized aggregate subquery (was: Pull up aggregate subquery)
2011/7/5 Yeb Havinga yebhavi...@gmail.com: Hello Hitosh, list, Attached is revised version. I failed to attached the patch. I'm trying again. I'm currently unable to test, since on holiday. I'm happy to continue testing once returned but it may not be within the bounds of the current commitfest, sorry. Thanks for replying. Regarding the time remained until the end of this commitfest, I'd think we should mark this item Returned with Feedback if no objection appears. I will be very happy if Yeb tests my latest patch after he comes back. I'll make up my mind around regression test. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Review of VS 2010 support patches
Hi all
I've got through a review of the VS 2010 support patches. Between work
being busy and some interesting issues getting my 64-bit build
environment set up it took longer than anticipated. Sorry.
It looks good so far. I haven't had any reply to my email to Brar, so
there are a few details (like whether x64 builds were tested and how x64
required libraries were built) I could use, but what I've got done so
far seems fine.
Details follow.
PATCH FORMATTING
==
The patch (VS2010v7.patch) seems to mix significant changes with
whitespace fixes etc. These should be separated for clarity and ease of
future bisect testing etc. Any perltidy run should be done as a
separate patch, too. This is easy if you are using git, because you can
just commit each to your local tree then use git format-patch to produce
nice patches. If you have a local tree with a more complicated history,
you can use git rebase to tidy up the history before you format-patch.
The patches apply cleanly to git master as of
21f1e15aafb13ab2430e831a3da7d4d4f525d1ce .
pgflex.pl and pgbison.pl
=
pgflex.pl and pgbison.pl are a big improvement over the horrid batch
files, but are perhaps too little a translation. There's no need for the
big if(string) then (otherstring) stuff; it can be done much more
cleanly by storing a simple hash of paths to options and doing a file
extension substitution to generate the output filenames. The hash only
needs to be populated for files that get processed with non-default
options, so for pgflex all you need is:
%LEX_OPTS = { 'src\backend\parser\scan.c' - '-CF' };
I can send adjusted versions of pgflex.pl and pgbison.pl that
DOCUMENTATION
===
I didn't notice any documentation updates to reflect the fact that
Visual Studio 2010 is now supported. It'd be a good idea to change
install-windows-full.html (or the source of it, anyway) to mention VS
2010 support.
TEST RESULTS (x86)
=
I used a buildenv.pl and config.pl that's known to build under VS 2008
and pass vcregress check with an unpatched copy of git master. I built
with everything except uuid and tcl enabled; I'll see if I can add them
later.
The patches applied cleanly, and didn't break VS 2008 builds, which
continued to work fine after a clean dist and build. vcregress
check still passes.
Builds done with VS 2010 using the patches worked fine, and passed
vcregress check tests.
I should have plcheck and contribcheck results as soon as I've got
things rebuilt with uuid and tcl.
TEST RESULTS (x64)
=
I'm still working on 64-bit tests. I've finally found out how to get
64-bit compilation working under Visual Studio 2008 Express Edition (or,
rather, Microsoft Windows SDK for Windows 7 and .NET Framework 3.5 SP1)
so I'll be testing that shortly.
I'm not sure if I'll be able to get 64-bit copies of all the optional
libraries built, so it may be a more minimal build. It'll include at
least zlib, plperl and plpython 64-bit support, though. Information from
Briar about whether he built for 64-bit and if so how he got his
libraries built would help.
--
Craig Ringer
POST Newspapers
276 Onslow Rd, Shenton Park
Ph: 08 9381 3088 Fax: 08 9388 2258
ABN: 50 008 917 717
http://www.postnewspapers.com.au/
--
Sent 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, Jul 4, 2011 at 12:47 PM, Radosław Smogura rsmog...@softperience.eu wrote: I asked about crash reports becaus of at this time there was thread about crashing in live system. Yeah, I thought this was the result of that effort: commit dcb09b595f88a3bca6097a6acc17bf2ec935d55f Author: Magnus Hagander mag...@hagander.net Date: Sun Dec 19 16:45:28 2010 +0100 Support for collecting crash dumps on Windows Add support for collecting minidump style crash dumps on Windows, by setting up an exception handling filter. Crash dumps will be generated in PGDATA/crashdumps if the directory is created (the existance of the directory is used as on/off switch for the generation of the dumps). Craig Ringer and Magnus Hagander -- 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] Crash dumps
On Tue, Jul 5, 2011 at 15:02, Robert Haas robertmh...@gmail.com wrote: On Mon, Jul 4, 2011 at 12:47 PM, Radosław Smogura rsmog...@softperience.eu wrote: I asked about crash reports becaus of at this time there was thread about crashing in live system. Yeah, I thought this was the result of that effort: commit dcb09b595f88a3bca6097a6acc17bf2ec935d55f Author: Magnus Hagander mag...@hagander.net Date: Sun Dec 19 16:45:28 2010 +0100 Support for collecting crash dumps on Windows Add support for collecting minidump style crash dumps on Windows, by setting up an exception handling filter. Crash dumps will be generated in PGDATA/crashdumps if the directory is created (the existance of the directory is used as on/off switch for the generation of the dumps). Craig Ringer and Magnus Hagander That crash dump is basically the windows equivalent of a coredump, though. Just a different name... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #6083: psql script line numbers incorrectly count \copy data
On Mon, Jul 4, 2011 at 12:02 PM, Tom Lane t...@sss.pgh.pa.us wrote: Steve Haslam araq...@googlemail.com 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? I think this is pretty safe. -- 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] Crash dumps
On Tue, Jul 5, 2011 at 9:05 AM, Magnus Hagander mag...@hagander.net wrote: On Tue, Jul 5, 2011 at 15:02, Robert Haas robertmh...@gmail.com wrote: On Mon, Jul 4, 2011 at 12:47 PM, Radosław Smogura rsmog...@softperience.eu wrote: I asked about crash reports becaus of at this time there was thread about crashing in live system. Yeah, I thought this was the result of that effort: commit dcb09b595f88a3bca6097a6acc17bf2ec935d55f Author: Magnus Hagander mag...@hagander.net Date: Sun Dec 19 16:45:28 2010 +0100 Support for collecting crash dumps on Windows Add support for collecting minidump style crash dumps on Windows, by setting up an exception handling filter. Crash dumps will be generated in PGDATA/crashdumps if the directory is created (the existance of the directory is used as on/off switch for the generation of the dumps). Craig Ringer and Magnus Hagander That crash dump is basically the windows equivalent of a coredump, though. Just a different name... Do we need something else in addition to that? -- 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] beta3?
On Fri, Jul 1, 2011 at 6:06 PM, Josh Berkus j...@agliodbs.com wrote: That sounds reasonable to me. I'll be on vacation then, but (1) I'm not really involved in pushing the release out the door and (2) I should have Internet access if push comes to shove. We seem to still have some blockers ... I'm only seeing these two: * ALTER TABLE lock strength reduction patch is unsafe * btree_gist breaks some behaviors involving operators Simon fixed the first over the weekend (though I think there are a few loose ends, see separate email on that topic) and Tom proposed a fix for the second (which I am guessing he will implement). Any other reason we can't or shouldn't wrap on the 11th? -- 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] Small documentation issue
On Fri, Jul 1, 2011 at 3:15 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: In fdwhandler.sgml, chapter fdwhandler has only one subsection (fdw-routines). If there is only one subsection, no table of contents is generated in the chapter. That means that people who navigate to the chapter from the main table of contents will not see that there is a subsection. I know too little about the documentation building process, but is it possible to generate a table of contents even if there is only one subsection? Maybe we could just add a sentence to the end of the third paragraph with a pointer to the section that follows, so it would read like this: The callback functions are plain C functions and are not visible or callable at the SQL level. They are described in more detail in Section 50.1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in SQL/MED?
2011/7/1 Shigeru Hanada shigeru.han...@gmail.com: 2011/6/30 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from 花田 茂's message of jue jun 30 06:00:23 -0400 2011: I attached a patch which fixes file_fdw to check required option (filename) in its validator function. I think that such requirement should be checked again in PlanForeignScan(), as it had been so far. Note that this patch requires fdw.patch has been applied. I think it'd be good to keep the error check in fileGetOptions() just to ensure that we don't crash in case a catalog is messed up with, though I'd degrade it to elog(ERROR) from ereport. Thanks for the comments. Please find attached a patch. Now file_fdw validates filename in: * file_fdw_validator(), to catch lack of required option at DDL * fileGetOptions(), to avoid crash caused by corrupted catalog I used ereport for the former check, because maybe such error usually happens and is visible to users. This criteria was taken from the document Reporting Errors Within the Server. http://developer.postgresql.org/pgdocs/postgres/error-message-reporting.html Do we want to apply this to 9.1 before beta3? -- 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] Range Types, constructors, and the type system
On Fri, Jul 1, 2011 at 2:09 AM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2011-06-30 at 12:28 +0200, Florian Pflug wrote: Well, arrays are containers, and we need two values to construct a range, What about empty ranges? What about infinite ranges? It seems quite a bit more awkward to shoehorn ranges into an array than to use a real type (even if it's intermediate and otherwise useless). Hm, I guess. I'm sill no huge fan of RANGEINPUT, but if we prevent it from being used as a column type and from being used as an argument type, then I guess it's workable... Btw, what happened to the idea of making RANGE(...) a special syntactic construct instead of a normal function call? Did we discard that for its intrusiveness, or were there other reasons? It has not been discarded; as far as I'm concerned it's still on the table. The main advantage is that it doesn't require an intermediate type, and that requiring a cast (or some specification of the range type) might be a little more natural. The downside is that, well, it's new syntax, and there's a little inertia there. But if it's actually better, we should do it. If an intermediate type seems to be problematic, or if people think it's strange to require casting, then I think this is reasonable. I don't understand how the bespoke syntax avoids the need for a cast? -- 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] Libpq enhancement
On Tue, Jun 21, 2011 at 3:55 PM, Merlin Moncure mmonc...@gmail.com wrote: For update, it's a bit more complex - we don't have a replace into operator... Actually, we do. 9.1 supports data modifying CTE around which it's possible to rig a perfectly reasonable upsert...barring that, you could trivially do something similar in a hand rolled backend upsert function that takes a row or a set of rows (fed in as a composite array). I don't believe that any of the solutions we have today are guaranteed to behave correctly in the face of concurrent activity. Because of the way snapshot isolation works, you can try to update an existing record, find that there isn't one, and then fail when you go to insert because some other backend has meanwhile inserted one that isn't visible to your snapshot. Doing the operations in the other order is no better. I'm not saying this is the biggest problem in the entire world, but I do think it's a non-imaginary problem. -- 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] Small SSI issues
On Sun, Jun 19, 2011 at 7:17 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Heikki Linnakangas wrote: * The oldserxid code is broken for non-default BLCKSZ. o The warning will come either too early or too late o There is no safeguard against actually wrapping around the SLRU, just the warning o I'm suspicious of the OldSerXidPagePrecedesLogically() logic with 32k BLCKSZ. In that case, OLDSERXID_MAX_PAGE is a larger than necessary to cover the whole range of 2^32 transactions, so at high XIDs, say 2^32-1, doesn't it incorrectly think that low XIDs, say 10, are in the past, not the future? It took me a while to see these problems because somehow I had forgotten that SLRU_PAGES_PER_SEGMENT was a literal of 32 rather than being based on BLCKSZ. After I rediscovered that, your concern was clear enough. I think the attached patch addresses the problem with the OldSerXidPagePrecedesLogically() function, which strikes me as the most serious issue. Based on the calculation from the attached patch, it would be easy to adjust the warning threshold, but I'm wondering if we should just rip it out instead. As I mentioned in a recent post based on reviewing your concerns, with an 8KB BLCKSZ the SLRU system will start thinking we're into wraparound at one billion transactions, and refuse to truncate segment files until we get down to less than that, but we won't actually overwrite anything and cause SSI misbehaviors until we eat through two billion (2^31 really) transactions while holding open a single read-write transaction. At that point I think PostgreSQL has other defenses which come into play. With the attached patch I don't think we can have any such problems with a *larger* BLCKSZ, so the only point of the warning would be for a BLCKSZ of 4KB or less. Is it worth carrying the warning code (with an appropriate adjustment to the thresholds) or should we drop it? Is this still an open item? -- 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] [v9.2] SECURITY LABEL on shared database object
On Mon, Jun 13, 2011 at 2:33 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2011/6/13 Robert Haas robertmh...@gmail.com: On Mon, Jun 13, 2011 at 1:40 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: 2011/6/13 Robert Haas robertmh...@gmail.com: On Mon, Jun 13, 2011 at 12:24 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote: The attached patch is an update revision of security label support for shared database objects. I'm kind of unexcited about this whole idea. Adding a shared catalog for a feature that's only of interest to a small percentage of our user population seems unfortunate. Are there any other possible approaches to this problem? If unexcited about the new shared catalog, one possible idea is to add a new field to pg_database, pg_tablespace and pg_authid to store security labels? The reason why we had pg_seclabel is to avoid massive amount of modifications to system catalog. But only 3 catalogs to be modified to support security label on shared object. I guess maybe my real question here is - what do you plan to do with those security labels, from a security perspective? For example: roles. The user's security contect AIUI is passed over from the remote side; his DAC role doesn't even enter into it from a MAC perspective. Or does it? The current primary target of security label of shared object is to acquire control on databases. It performs as a basis to compute default security label of underlying objects, and I also plan to control when we open the connection like ACL_CONNECT. Right now, we assume any database has system_u:object_r:sepgsql_db_t:s0, then the default security label of schema objects are computed. (See sepgsql_schema_post_create in contrib/sepgsql/schema.c) Regarding to the pg_tablespace and pg_authid, I have a plan to control DDL statements on these objects, However, its priority is not higher than databases or other non-shared objects such as tables or procedures. Hmm, OK. I guess what I'm not sure about is - how much should we worry about the fact that this creates several more shared (and therefore nailed?) system catalogs? Anyone have an opinion on that? -- 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] testing nested case-when scoping
On Wed, Jun 15, 2011 at 7:43 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello Heikki, probably I found a bug in patch: CREATE FUNCTION fx(i integer) RETURNS integer LANGUAGE plpgsql AS $$begin raise notice '%', i; return i; end;$$; CREATE FUNCTION fx1(integer) RETURNS text LANGUAGE sql AS $_$ select case $1 when 1 then 'A' else 'B' end$_$; CREATE FUNCTION fx2(text) RETURNS text LANGUAGE sql AS $_$ select case $1 when 'A' then 'a' else 'b' end$_$; CREATE TABLE foo ( a integer ); COPY foo (a) FROM stdin; 1 0 \. postgres=# select fx2(fx1(fx(a))) from foo; NOTICE: 1 ERROR: invalid expression parameter reference (1 levels up, while stack is only 1 elements deep) I can't reproduce this. Perhaps it was fixed by one of the later commits? -- 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] [BUG] SSPI authentication fails on Windows when server parameter is localhost or domain name
On Fri, Jun 17, 2011 at 6:32 AM, Thom Brown t...@linux.com wrote:
On 15 June 2011 12:16, Dave Page dp...@pgadmin.org wrote:
On Wed, Jun 15, 2011 at 10:53 AM, Ahmed Shinwari
ahmed.shinw...@gmail.com wrote:
Hi All,
I faced a bug on Windows while connecting via SSPI authentication. I was
able to find the bug and have attached the patch. Details listed below;
Postgres Installer: Version 9.0.4
OS: Windows Server 2008 R2/Windows 7
Bug Description:
=
If database Server is running on Windows ('Server 2008 R2' or 'Windows 7')
with authentication mode SSPI and one try to connect from the same machine
via 'psql' with server parameter as 'localhost' or 'fully qualified domain
name', the database throws error;
I've been able to reproduce this issue, and the patch does indeed fix
it. One of our customers has also confirmed it fixed it for them.
I can confirm this affects versions back to 8.3.
Seems like we'd better try to get this committed before the next set
of minor releases (and ideally also before 9.1beta3).
--
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] Bug in SQL/MED?
Robert Haas wrote: I attached a patch which fixes file_fdw to check required option (filename) in its validator function. I think that such requirement should be checked again in PlanForeignScan(), as it had been so far. Note that this patch requires fdw.patch has been applied. Do we want to apply this to 9.1 before beta3? If possible, yes please. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] SECURITY LABEL on shared database object
Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011: Hmm, OK. I guess what I'm not sure about is - how much should we worry about the fact that this creates several more shared (and therefore nailed?) system catalogs? Anyone have an opinion on that? Several? That would worry me, given that we currently have a small number (eight currently). If it's just one more, I don't think it's such a big deal. I'm not sure what you mean by nailed though -- I mean, for example pg_shdescription is shared but not nailed in the rd_isnailed sense of the word, AFAICS. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Small SSI issues
Robert Haas robertmh...@gmail.com wrote: On Sun, Jun 19, 2011 at 7:17 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Heikki Linnakangas wrote: * The oldserxid code is broken for non-default BLCKSZ. o The warning will come either too early or too late o There is no safeguard against actually wrapping around the SLRU, just the warning o I'm suspicious of the OldSerXidPagePrecedesLogically() logic with 32k BLCKSZ. In that case, OLDSERXID_MAX_PAGE is a larger than necessary to cover the whole range of 2^32 transactions, so at high XIDs, say 2^32-1, doesn't it incorrectly think that low XIDs, say 10, are in the past, not the future? It took me a while to see these problems because somehow I had forgotten that SLRU_PAGES_PER_SEGMENT was a literal of 32 rather than being based on BLCKSZ. After I rediscovered that, your concern was clear enough. I think the attached patch addresses the problem with the OldSerXidPagePrecedesLogically() function, which strikes me as the most serious issue. Based on the calculation from the attached patch, it would be easy to adjust the warning threshold, but I'm wondering if we should just rip it out instead. As I mentioned in a recent post based on reviewing your concerns, with an 8KB BLCKSZ the SLRU system will start thinking we're into wraparound at one billion transactions, and refuse to truncate segment files until we get down to less than that, but we won't actually overwrite anything and cause SSI misbehaviors until we eat through two billion (2^31 really) transactions while holding open a single read-write transaction. At that point I think PostgreSQL has other defenses which come into play. With the attached patch I don't think we can have any such problems with a *larger* BLCKSZ, so the only point of the warning would be for a BLCKSZ of 4KB or less. Is it worth carrying the warning code (with an appropriate adjustment to the thresholds) or should we drop it? Is this still an open item? Yes, although I'm not clear on whether people feel it is one which needs to be fixed for 9.1 or left for 9.2. On a build with a BLCKSZ less than 8KB we would not get a warning before problems occurred, and we would have more serious problem involving potentially incorrect behavior. Tom questioned whether anyone actually builds with BLCKSZ less than 8KB, and I'm not altogether sure that SLRUs dealing with transaction IDs would behave correctly either. On block sizes larger than 8KB it will the warning if you burn through one billion transactions while holding one serializable read write transaction open, even though there won't be a problem. With the larger BLCKSZ values it may also generate log level messages about SLRU wraparound when that's not really a problem. The patch posted with the quoted message will prevent the misbehavior on smaller block sizes and the bogus log messages on larger block sizes. We'd need to change a couple more lines to get the warning to trigger at the appropriate time for different block sizes -- or we could just rip out the warning code. (I didn't post a patch for that because there wasn't a clear consensus about whether to fix it, rip it out, or leave it alone for 9.1.) -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
Hello. System: PostgreSQL v9.0 Windows XP SP3 SQL: COPY tablename TO STDOUT WITH (FORMAT binary) ERROR: syntax error at or near binary LINE 1: ...OPY tablename TO STDOUT WITH (FORMAT binary) ^ ** Error ** ERROR: syntax error at or near binary SQL state: 42601 Character: 55 But if I use 'FORMAT text' or 'FORMAT csv' all is OK. Suppose this happens because BINARY is not listed in unreserved_keyword neither in col_name_keyword parser parser rules, but listed in type_func_name_keyword instead. -- With best wishes, Pavel mailto:pa...@gf.microolap.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.2] SECURITY LABEL on shared database object
On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011: Hmm, OK. I guess what I'm not sure about is - how much should we worry about the fact that this creates several more shared (and therefore nailed?) system catalogs? Anyone have an opinion on that? Several? That would worry me, given that we currently have a small number (eight currently). If it's just one more, I don't think it's such a big deal. I'm not sure what you mean by nailed though -- I mean, for example pg_shdescription is shared but not nailed in the rd_isnailed sense of the word, AFAICS. Well, right now the patch has pg_shseclabel, and its index, plus a toast table and a toast index. Not sure why we want/need the toast table index there, but the patch has 'em as of now. As for whether it needs to be nailed, I'm not sure I understand what the rules are there. I *think* the rule is that anything that might need to be consulted before choosing a database must be nailed. If that's right, we might be able to get by without nailing it, as long as the label isn't needed during authentication (or its use can be postponed until after we've connected to a database). -- 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] [COMMITTERS] pgsql: Move Trigger and TriggerDesc structs out of rel.h into a new rel
Excerpts from Tom Lane's message of mar jul 05 10:47:03 -0400 2011:
Alvaro Herrera alvhe...@alvh.no-ip.org writes:
Move Trigger and TriggerDesc structs out of rel.h into a new reltrigger.h
This lets us stop including rel.h into execnodes.h, which is a widely
used header.
I'm confused why this patch added pg_am.h to predtest.c?
Because otherwise it fails to compile with this error:
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels -Wformat-security
-fno-strict-aliasing -fwrapv -g -I../../../../src/include
-I/pgsql/source/HEAD/src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o
predtest.o /pgsql/source/HEAD/src/backend/optimizer/util/predtest.c -MMD -MP
-MF .deps/predtest.Po
/pgsql/source/HEAD/src/backend/optimizer/util/predtest.c: In function
‘get_btree_test_op’:
/pgsql/source/HEAD/src/backend/optimizer/util/predtest.c:1661:32: error:
‘BTREE_AM_OID’ undeclared (first use in this function)
/pgsql/source/HEAD/src/backend/optimizer/util/predtest.c:1661:32: note: each
undeclared identifier is reported only once for each function it appears in
make: *** [predtest.o] Error 1
Since that symbol is defined in pg_am.h, I thought the most convenient
fix was to include just that file. I could, of course, have included
the whole of rel.h but that seemed a bit pointless. The relevant code
is actually dealing with opfamilies (pg_amop.h is already being
included):
/* Now search the opfamilies */
for (i = 0; i catlist-n_members; i++)
{
HeapTuple pred_tuple = catlist-members[i]-tuple;
Form_pg_amop pred_form = (Form_pg_amop) GETSTRUCT(pred_tuple);
HeapTuple clause_tuple;
/* Must be btree */
if (pred_form-amopmethod != BTREE_AM_OID)
continue;
(Of course, the reason this didn't fail previously is because rel.h
includes pg_am.h).
--
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
Excerpts from Pavel Golub's message of mar jul 05 10:52:06 -0400 2011: Hello. System: PostgreSQL v9.0 Windows XP SP3 SQL: COPY tablename TO STDOUT WITH (FORMAT binary) ERROR: syntax error at or near binary LINE 1: ...OPY tablename TO STDOUT WITH (FORMAT binary) ^ ** Error ** ERROR: syntax error at or near binary SQL state: 42601 Character: 55 But if I use 'FORMAT text' or 'FORMAT csv' all is OK. Suppose this happens because BINARY is not listed in unreserved_keyword neither in col_name_keyword parser parser rules, but listed in type_func_name_keyword instead. That seems pretty unfortunate. Of course, it works if you quote it: COPY tablename TO STDOUT WITH (FORMAT binary) I assume it's not in unreserved_keyword because it would cause a shift/reduce conflict elsewhere. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Small SSI issues
On Tue, Jul 5, 2011 at 10:51 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Is this still an open item? Yes, although I'm not clear on whether people feel it is one which needs to be fixed for 9.1 or left for 9.2. On a build with a BLCKSZ less than 8KB we would not get a warning before problems occurred, and we would have more serious problem involving potentially incorrect behavior. Tom questioned whether anyone actually builds with BLCKSZ less than 8KB, and I'm not altogether sure that SLRUs dealing with transaction IDs would behave correctly either. On block sizes larger than 8KB it will the warning if you burn through one billion transactions while holding one serializable read write transaction open, even though there won't be a problem. With the larger BLCKSZ values it may also generate log level messages about SLRU wraparound when that's not really a problem. Well, as long as we can verify that OLDSERXID_MAX_PAGE has the same value for BLCKSZ=8K before and after this patch, I don't see any real downside to applying it. If, hypothetically, it's buggy, it's only going to break things for non-default block sizes which are, by your description, not correct right now anyway. -- 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] [COMMITTERS] pgsql: Move Trigger and TriggerDesc structs out of rel.h into a new rel
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Tom Lane's message of mar jul 05 10:47:03 -0400 2011: I'm confused why this patch added pg_am.h to predtest.c? ... (Of course, the reason this didn't fail previously is because rel.h includes pg_am.h). Oh, duh. Nevermind. 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] Range Types, constructors, and the type system
On Tue, 2011-07-05 at 10:06 -0400, Robert Haas wrote: But if it's actually better, we should do it. If an intermediate type seems to be problematic, or if people think it's strange to require casting, then I think this is reasonable. I don't understand how the bespoke syntax avoids the need for a cast? It doesn't, it just avoids the need for an intermediate type. What I meant was that it might be strange to require a cast on the result of a function call, because we don't really do that anywhere else. Florian pointed out that it's common to require casting the ARRAY[] constructor, so that has more of a precedent. I'm not really sure how much that matters. I'm OK with the intermediate type, but Florian seems skeptical of that idea. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
Hello, Alvaro. You wrote: AH Excerpts from Pavel Golub's message of mar jul 05 10:52:06 -0400 2011: Hello. System: PostgreSQL v9.0 Windows XP SP3 SQL: COPY tablename TO STDOUT WITH (FORMAT binary) ERROR: syntax error at or near binary LINE 1: ...OPY tablename TO STDOUT WITH (FORMAT binary) ^ ** Error ** ERROR: syntax error at or near binary SQL state: 42601 Character: 55 But if I use 'FORMAT text' or 'FORMAT csv' all is OK. Suppose this happens because BINARY is not listed in unreserved_keyword neither in col_name_keyword parser parser rules, but listed in type_func_name_keyword instead. AH That seems pretty unfortunate. Of course, it works if you quote it: AH COPY tablename TO STDOUT WITH (FORMAT binary) AH I assume it's not in unreserved_keyword because it would cause a AH shift/reduce conflict elsewhere. Well, there are two ways: 1. Change documentation, so quoted or double quoted values are accepted 2. Fix parser -- With best wishes, Pavel mailto:pa...@gf.microolap.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: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
On Tue, Jul 5, 2011 at 11:06 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Pavel Golub's message of mar jul 05 10:52:06 -0400 2011: Hello. System: PostgreSQL v9.0 Windows XP SP3 SQL: COPY tablename TO STDOUT WITH (FORMAT binary) ERROR: syntax error at or near binary LINE 1: ...OPY tablename TO STDOUT WITH (FORMAT binary) ^ ** Error ** ERROR: syntax error at or near binary SQL state: 42601 Character: 55 But if I use 'FORMAT text' or 'FORMAT csv' all is OK. Suppose this happens because BINARY is not listed in unreserved_keyword neither in col_name_keyword parser parser rules, but listed in type_func_name_keyword instead. That seems pretty unfortunate. Of course, it works if you quote it: COPY tablename TO STDOUT WITH (FORMAT binary) I assume it's not in unreserved_keyword because it would cause a shift/reduce conflict elsewhere. Yeah. In particular, it conflicts with the ancient copy syntax which we still support for backwards compatibility with versions 7.3. We can fix the immediate problem with something like the attached. (a) Should we do that? (b) Should we back-patch it to 9.1 and 9.0? (c) Should we consider removing compatibility with the ancient copy syntax in 9.2, and de-reserving that keyword? (Given that the workaround is this simple, I'm inclined to say no, but could be persuaded otherwise.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company allow-copy-format-binary.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] %ENV warnings during builds
On 07/05/2011 02:30 AM, Brar Piening wrote: 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. Try attached instead. The bat wrappers just need to read: @echo off @perl src/tools/msvc/pgflex.pl %* and @echo off @perl src/tools/msvc/pgbison.pl %* cheers andrew pgflex.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] Range Types, constructors, and the type system
On Tue, Jul 5, 2011 at 11:11 AM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2011-07-05 at 10:06 -0400, Robert Haas wrote: But if it's actually better, we should do it. If an intermediate type seems to be problematic, or if people think it's strange to require casting, then I think this is reasonable. I don't understand how the bespoke syntax avoids the need for a cast? It doesn't, it just avoids the need for an intermediate type. What I meant was that it might be strange to require a cast on the result of a function call, because we don't really do that anywhere else. Florian pointed out that it's common to require casting the ARRAY[] constructor, so that has more of a precedent. I'm not really sure how much that matters. I'm OK with the intermediate type, but Florian seems skeptical of that idea. How about the idea of creating a family of four constructor functions for each new range type? The functions would be named after the range type, with _cc, _co, _oc, and _oo appended. So, then, instead of writing: RANGE(1,8,'c','o')::int8range ...or somesuch, you could just say: int8range_co(1,8) ...which is both more compact and less ugly, IMHO, and seems to circumvent all the type system problems as well. -- 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: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
Robert Haas robertmh...@gmail.com writes: On Tue, Jul 5, 2011 at 11:06 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: I assume it's not in unreserved_keyword because it would cause a shift/reduce conflict elsewhere. Yeah. In particular, it conflicts with the ancient copy syntax which we still support for backwards compatibility with versions 7.3. We can fix the immediate problem with something like the attached. (a) Should we do that? That seems like a horrid crock ... 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] Core Extensions relocation
On Sat, Jun 11, 2011 at 12:38 PM, Greg Smith g...@2ndquadrant.com wrote: Peter Eisentraut wrote: For the directory name, I'd prefer either src/extensions (since there is more than one), or if you want to go for short somehow, src/ext. (Hmm, I guess the installation subdirectory is also called extension. But it felt wrong on first reading anyway.) I jumped between those two a couple of times myself, settling on extension to match the installation location as you figured out. Assuming that name shouldn't change at this point, this seemed the best way to name the new directory, even though I agree it seems weird at first. What version did you branch this off? :) Long enough ago that apparently I've missed some major changes; Magnus already pointed out I needed to revisit how MODULEDIR was used. Looks like I need to rebuild the first patch in this series yet again, which shouldn't be too bad. The second time I did that, I made the commits atomic enough that the inevitable third one would be easy. Are you going to do this work for this CommitFest? -- 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: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
On 07/05/2011 11:23 AM, Robert Haas wrote: Yeah. In particular, it conflicts with the ancient copy syntax which we still support for backwards compatibility with versions 7.3. We can fix the immediate problem with something like the attached. (a) Should we do that? yes. (b) Should we back-patch it to 9.1 and 9.0? yes. (c) Should we consider removing compatibility with the ancient copy syntax in 9.2, and de-reserving that keyword? (Given that the workaround is this simple, I'm inclined to say no, but could be persuaded otherwise.) I'm inclined to say yes, but mainly because it's just old cruft. I don't expect to be able,say, to load a pre-7.3 dump into a modern Postgres. 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: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
On Tue, Jul 5, 2011 at 11:30 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 5, 2011 at 11:06 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: I assume it's not in unreserved_keyword because it would cause a shift/reduce conflict elsewhere. Yeah. In particular, it conflicts with the ancient copy syntax which we still support for backwards compatibility with versions 7.3. We can fix the immediate problem with something like the attached. (a) Should we do that? That seems like a horrid crock ... Do you have something else to propose? It's a crock we have used elsewhere, so there is some precedent. -- 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: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
Hello, Robert. You wrote: RH On Tue, Jul 5, 2011 at 11:06 AM, Alvaro Herrera RH alvhe...@commandprompt.com wrote: Excerpts from Pavel Golub's message of mar jul 05 10:52:06 -0400 2011: Hello. System: PostgreSQL v9.0 Windows XP SP3 SQL: COPY tablename TO STDOUT WITH (FORMAT binary) ERROR: syntax error at or near binary LINE 1: ...OPY tablename TO STDOUT WITH (FORMAT binary) ^ ** Error ** ERROR: syntax error at or near binary SQL state: 42601 Character: 55 But if I use 'FORMAT text' or 'FORMAT csv' all is OK. Suppose this happens because BINARY is not listed in unreserved_keyword neither in col_name_keyword parser parser rules, but listed in type_func_name_keyword instead. That seems pretty unfortunate. Of course, it works if you quote it: COPY tablename TO STDOUT WITH (FORMAT binary) I assume it's not in unreserved_keyword because it would cause a shift/reduce conflict elsewhere. RH Yeah. In particular, it conflicts with the ancient copy syntax which RH we still support for backwards compatibility with versions 7.3. We RH can fix the immediate problem with something like the attached. This patch is ugly. Sorry, Robert, but it's true. RH (a) Should we do that? RH (b) Should we back-patch it to 9.1 and 9.0? RH (c) Should we consider removing compatibility with the ancient copy RH syntax in 9.2, and de-reserving that keyword? (Given that the RH workaround is this simple, I'm inclined to say no, but could be RH persuaded otherwise.) +1 for this. Pre-7.3 syntax is dead in fact for many years. -- With best wishes, Pavel mailto:pa...@gf.microolap.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: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
On Tue, Jul 5, 2011 at 11:37 AM, Pavel Golub pa...@microolap.com wrote: RH Yeah. In particular, it conflicts with the ancient copy syntax which RH we still support for backwards compatibility with versions 7.3. We RH can fix the immediate problem with something like the attached. This patch is ugly. Sorry, Robert, but it's true. No hard feelings here. If you, as the reporter of the problem, don't feel that it's serious enough to warrant back-patching a fix, then I'm not going to insist. However, if we don't do what I've proposed here, then I think 8.4 and 9.0 and probably 9.1 are going to need to stay as they are, because... RH (c) Should we consider removing compatibility with the ancient copy RH syntax in 9.2, and de-reserving that keyword? (Given that the RH workaround is this simple, I'm inclined to say no, but could be RH persuaded otherwise.) +1 for this. Pre-7.3 syntax is dead in fact for many years. ...this is not something we're going to back-patch. -- 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] Range Types, constructors, and the type system
On Jul5, 2011, at 17:11 , Jeff Davis wrote: I'm OK with the intermediate type, but Florian seems skeptical of that idea. I'm starting to get used to it, though ;-) I do now believe that it can be made safe against accidental miss-use, it seem that I was overly anxious there. What I still don't like about it is that it feels like a workaround for a feature missing in our type system - the possibility of having function with a polymorphic return type, but no polymorphic arguments. I feel somewhat strongly about this, because it bit me when I tried to implement record_getfield() and record_setfield() to get and set a record's field based on it's name. However, placing the burden of solving that onto the range type patch doesn't seem fair. Plus, I've realized now that a RANGEINPUT type would allow us to easily support some things that otherwise seem hard. We could, for example, make the cast from RANGEINPUT to the individual range types an assignment cast (or even implicit), thereby removing the need for an explicit cast in a lot of common cases like insert into a table with a range column. 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
[HACKERS] capturing regression test core dump
Is there any way to get the regression tests to write a core dump file somewhere that I can get at it? I tried ulimit -c unlimited but can't find any core file lying around after I reproduce the crash. -- 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: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
Robert Haas robertmh...@gmail.com writes: ... However, if we don't do what I've proposed here, then I think 8.4 and 9.0 and probably 9.1 are going to need to stay as they are, because... RH (c) Should we consider removing compatibility with the ancient copy RH syntax in 9.2, and de-reserving that keyword? (Given that the RH workaround is this simple, I'm inclined to say no, but could be RH persuaded otherwise.) +1 for this. Pre-7.3 syntax is dead in fact for many years. ...this is not something we're going to back-patch. Given the lack of prior complaints, and the simplicity of the double-quote workaround, I feel little need to have a back-patchable fix. 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] testing nested case-when scoping
2011/7/5 Robert Haas robertmh...@gmail.com: On Wed, Jun 15, 2011 at 7:43 AM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello Heikki, probably I found a bug in patch: CREATE FUNCTION fx(i integer) RETURNS integer LANGUAGE plpgsql AS $$begin raise notice '%', i; return i; end;$$; CREATE FUNCTION fx1(integer) RETURNS text LANGUAGE sql AS $_$ select case $1 when 1 then 'A' else 'B' end$_$; CREATE FUNCTION fx2(text) RETURNS text LANGUAGE sql AS $_$ select case $1 when 'A' then 'a' else 'b' end$_$; CREATE TABLE foo ( a integer ); COPY foo (a) FROM stdin; 1 0 \. postgres=# select fx2(fx1(fx(a))) from foo; NOTICE: 1 ERROR: invalid expression parameter reference (1 levels up, while stack is only 1 elements deep) I can't reproduce this. Perhaps it was fixed by one of the later commits? I don't checked it again, because Tom rejected Heikki's design. Regards Pavel -- 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] capturing regression test core dump
On Jul5, 2011, at 17:49 , Robert Haas wrote: Is there any way to get the regression tests to write a core dump file somewhere that I can get at it? I tried ulimit -c unlimited but can't find any core file lying around after I reproduce the crash. In case you're on OSX, the core dumps there are written to /cores, not to the cwd of the process. 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] Small SSI issues
Robert Haas robertmh...@gmail.com wrote: Well, as long as we can verify that OLDSERXID_MAX_PAGE has the same value for BLCKSZ=8K before and after this patch, I don't see any real downside to applying it. If, hypothetically, it's buggy, it's only going to break things for non-default block sizes which are, by your description, not correct right now anyway. Outside of a code comment, the entire patch consists of replacing the definition of the OLDSERXID_MAX_PAGE macro. The old definition is: (SLRU_PAGES_PER_SEGMENT * 0x1 - 1) SLRU_PAGES_PER_SEGMENT is define to be 32. So this is: (32 * 0x1) - 1 = 2097151 The new definition is the min of the old one and a value based on BLCKSZ: (MaxTransactionId + 1) / OLDSERXID_ENTRIESPERPAGE - 1) Where entries per page is BLCKSZ / sizeof(uint64). For an 8K BLCKSZ this is: ((0x + 1) / 1024) - 1 = 4194303 So the macro is guaranteed to have the same value as it currently does for BLCKSZ of 16KB or lower. -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] capturing regression test core dump
Excerpts from Robert Haas's message of mar jul 05 11:49:48 -0400 2011: Is there any way to get the regression tests to write a core dump file somewhere that I can get at it? I tried ulimit -c unlimited but can't find any core file lying around after I reproduce the crash. Are you using a temp install? If so, the core files are going to end up there, and thus go away when that's deleted. You could change the core_pattern if you're on Linux (see core(5)) but it's probably easier to use make installcheck instead. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [BUGS] [HACKERS] COPY .... WITH (FORMAT binary) causes syntax error at or near binary
Hello, Robert. You wrote: RH On Tue, Jul 5, 2011 at 11:37 AM, Pavel Golub pa...@microolap.com wrote: RH Yeah. In particular, it conflicts with the ancient copy syntax which RH we still support for backwards compatibility with versions 7.3. We RH can fix the immediate problem with something like the attached. This patch is ugly. Sorry, Robert, but it's true. RH No hard feelings here. If you, as the reporter of the problem, don't RH feel that it's serious enough to warrant back-patching a fix, then I'm RH not going to insist. However, if we don't do what I've proposed here, RH then I think 8.4 and 9.0 and probably 9.1 are going to need to stay as RH they are, because... RH (c) Should we consider removing compatibility with the ancient copy RH syntax in 9.2, and de-reserving that keyword? (Given that the RH workaround is this simple, I'm inclined to say no, but could be RH persuaded otherwise.) +1 for this. Pre-7.3 syntax is dead in fact for many years. RH ...this is not something we're going to back-patch. Patches needed for 9.0 and 9.1 only, because this is new format comparing with 8.x -- With best wishes, Pavel mailto:pa...@gf.microolap.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] capturing regression test core dump
On Tue, Jul 5, 2011 at 11:57 AM, Florian Pflug f...@phlo.org wrote: On Jul5, 2011, at 17:49 , Robert Haas wrote: Is there any way to get the regression tests to write a core dump file somewhere that I can get at it? I tried ulimit -c unlimited but can't find any core file lying around after I reproduce the crash. In case you're on OSX, the core dumps there are written to /cores, not to the cwd of the process. Oh, ho! Thanks! -- 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] Range Types, constructors, and the type system
On Tue, Jul 5, 2011 at 10:26 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 5, 2011 at 11:11 AM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2011-07-05 at 10:06 -0400, Robert Haas wrote: But if it's actually better, we should do it. If an intermediate type seems to be problematic, or if people think it's strange to require casting, then I think this is reasonable. I don't understand how the bespoke syntax avoids the need for a cast? It doesn't, it just avoids the need for an intermediate type. What I meant was that it might be strange to require a cast on the result of a function call, because we don't really do that anywhere else. Florian pointed out that it's common to require casting the ARRAY[] constructor, so that has more of a precedent. I'm not really sure how much that matters. I'm OK with the intermediate type, but Florian seems skeptical of that idea. How about the idea of creating a family of four constructor functions for each new range type? The functions would be named after the range type, with _cc, _co, _oc, and _oo appended. So, then, instead of writing: RANGE(1,8,'c','o')::int8range ...or somesuch, you could just say: int8range_co(1,8) ...which is both more compact and less ugly, IMHO, and seems to circumvent all the type system problems as well. +1 on this (so you wouldn't even then directly cast to a range?) merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Range Types, constructors, and the type system
On Tue, Jul 5, 2011 at 12:23 PM, Merlin Moncure mmonc...@gmail.com wrote: On Tue, Jul 5, 2011 at 10:26 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 5, 2011 at 11:11 AM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2011-07-05 at 10:06 -0400, Robert Haas wrote: But if it's actually better, we should do it. If an intermediate type seems to be problematic, or if people think it's strange to require casting, then I think this is reasonable. I don't understand how the bespoke syntax avoids the need for a cast? It doesn't, it just avoids the need for an intermediate type. What I meant was that it might be strange to require a cast on the result of a function call, because we don't really do that anywhere else. Florian pointed out that it's common to require casting the ARRAY[] constructor, so that has more of a precedent. I'm not really sure how much that matters. I'm OK with the intermediate type, but Florian seems skeptical of that idea. How about the idea of creating a family of four constructor functions for each new range type? The functions would be named after the range type, with _cc, _co, _oc, and _oo appended. So, then, instead of writing: RANGE(1,8,'c','o')::int8range ...or somesuch, you could just say: int8range_co(1,8) ...which is both more compact and less ugly, IMHO, and seems to circumvent all the type system problems as well. +1 on this (so you wouldn't even then directly cast to a range?) You wouldn't need to, because these functions would be declared to return the range type. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Extra check in 9.0 exclusion constraint unintended consequences
In the 9.0 version of exclusion constraints, we added an extra check to ensure that, when searching for a conflict, a tuple at least found itself as a conflict. This extra check is not present in 9.1+. It was designed to help diagnose certain types of problems, and is fine for most use cases. A value is equal to itself (and therefore conflicts with itself), and a value overlaps with itself (and therefore conflicts with itself), which were the primary use cases. We removed the extra check in 9.1 because there are other operators for which that might not be true, like , but the use case is a little more obscure. However, values don't always overlap with themselves -- for instance the empty period (which was an oversight by me). So, Abel Abraham Camarillo Ojeda ran into a rather cryptic error message when he tried to do that: ERROR: failed to re-find tuple within index t_period_excl HINT: This may be because of a non-immutable index expression. I don't think we need to necessarily remove the extra check in 9.0, because the workaround is simple: add a WHERE clause to the constraint eliminating empty periods. Perhaps we could improve the error message and hint, and add a note in the documentation. Thoughts? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra check in 9.0 exclusion constraint unintended consequences
Hi: On Tue, Jul 5, 2011 at 11:26 AM, Jeff Davis pg...@j-davis.com wrote: In the 9.0 version of exclusion constraints, we added an extra check to ensure that, when searching for a conflict, a tuple at least found itself as a conflict. This extra check is not present in 9.1+. It was designed to help diagnose certain types of problems, and is fine for most use cases. A value is equal to itself (and therefore conflicts with itself), and a value overlaps with itself (and therefore conflicts with itself), which were the primary use cases. We removed the extra check in 9.1 because there are other operators for which that might not be true, like , but the use case is a little more obscure. However, values don't always overlap with themselves -- for instance the empty period (which was an oversight by me). So, Abel Abraham Camarillo Ojeda ran into a rather cryptic error message when he tried to do that: ERROR: failed to re-find tuple within index t_period_excl HINT: This may be because of a non-immutable index expression. I don't think we need to necessarily remove the extra check in 9.0, because the workaround is simple: add a WHERE clause to the constraint eliminating empty periods. Perhaps we could improve the error message and hint, and add a note in the documentation. That's what I'm doing now: using a where clause to workaround... it's easy, but I was still amazed about what that error message meant... Thanks. Thoughts? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] hint bit cache v6
On Thu, Jun 30, 2011 at 3:42 PM, Merlin Moncure mmonc...@gmail.com wrote: On Thu, Jun 30, 2011 at 11:44 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Jun 30, 2011 at 11:18 AM, Merlin Moncure mmonc...@gmail.com wrote: I think the basic problem is that the cache pages are so large. It's hard to make them smaller because that increases the cost of accessing the cache, as you already noted, but at the same time, making an eviction decision on a cache that holds 64K entries based on 100 data points seems like waving around a pretty large-caliber weapon in a fairly uncontrolled fashion. Maybe it works OK 90% of the time, but it's hard to avoid the niggling fear that someone might accidentally get shot. Right...it's essentially a rolling statistical sampling of transaction IDs based on past acceses. Going from say, 100 to 1000 will probably drop your errror margin quite a bit but I really wonder how benefit you can get from going past that. An interesting idea might be to forcibly cause a replacement every 100 tuples (or every 1000 tuples) and see if that causes a noticeable performance regression. If it doesn't, that's a good data point. To test this I disabled the cache check on top of HeapTupleSatisfiesMVCC and forced a cache entry in place of it with a bogus xid (so every tuple scanned was treated as a 'miss'. Scanning 1 million records in memory over and over went up a few percentage points -- converging on about 280ms instead of 270ms. This is with bumped to 1000 miss array: oprofile said: regular hinted tuple case: 120 10.2041 advance_aggregates 107 9.0986 heapgettup_pagemode 77 6.5476 ExecProject 74 6.2925 heapgetpage 72 6.1224 ExecProcNode 72 6.1224 ExecScan 69 5.8673 advance_transition_function 66 5.6122 heap_getnext 58 4.9320 HeapTupleSatisfiesMVCC hinted tuple with pathological cache entry: 111 8.9588 advance_aggregates 109 8.7974 heapgettup_pagemode 85 6.8604 ExecProject 81 6.5375 heapgetpage 77 6.2147 ExecScan 70 5.6497 ExecStoreTuple 70 5.6497 HeapTupleSatisfiesMVCC this was a fairly short profiling run but the numbers are fairly consistent. the replacement is fairly cheap...sorting 1000 ints doesn't take all that long. 100 is even faster. I think the sour spot for this whole idea is likely to be the case where you have a workload that is 90% read and 10% write, or something like that. On an all-read workload (like pgbench -S), you're hopefully going to converge to a state where all the hint-bits are set, once VACUUM kicks in. And on an all-write workload I think that you might lose the effect in the noise. Not sure if we have an easy way to set up such a workload with pgbench, though. it's trivial to test with pgbench -- just use the random number facility to generate an id for some table and update where random() .9. However this does not generate nearly enough 'misses' to exercise cache replacement in any meaningful way. My workstation vm can punch out about 5k transactions/sec. With only 10% getting a new xid and writing back a tuple to the table only 500 xids are getting generated/second. At that rate it takes quite a while to burn through the 256k transactions the cache ranges over. Also this case is not bound by scan performance anyways making profiling it a little silly -- HeapTupleSatisfiesMVCC is simply not as big of a bottleneck in OLTP workloads. Scan heavy loads is where this matters, and scan heavy loads naturally tend to generate less xids per tuple scanned. Just for the sake of argument, let's suppose we made an array with 64K elements, each one representing one 64K chunk of the XID space. Each element is a 4-byte unsigned integer, so the array takes up 256kB of memory... probably not good, but I'm just thinking out loud here. Every time we're asked about an XID, we bump the corresponding counter. Periodically (say, every 64K probes) we zip through all the counters and consider performing a cache replacement. We look at the not-already-cached page that has the highest counter value and compare it to the counter value for the cached pages. If it is higher and the difference exceeds some safety margin (to protect against hysteresis), then we do the replacement. I think something like this would allow you to have a lot more information available to make a direct apples-to-apples comparison at cache replacement time, as compared with what you have now. yeah -- what you've done here is basically two things: 1. by mapping static ranges you get to skip the sort (but not the scan) during the replacement, and 2. by vastly expanding the sampling size you eliminate thrashing via noise in the rolling sample. This comes at a significant memory cost and loss of some nimbleness in the cache (i'm not completely sure more aggressive replacement is 'bad')
Re: [HACKERS] Range Types, constructors, and the type system
On Tue, 2011-07-05 at 11:26 -0400, Robert Haas wrote: How about the idea of creating a family of four constructor functions for each new range type? The functions would be named after the range type, with _cc, _co, _oc, and _oo appended. So, then, instead of writing: RANGE(1,8,'c','o')::int8range It would be something like: range_co(1,8)::int8range (just so we're comparing apples to apples) The intermediate type proposal doesn't require that we move the c and o into the parameter list. int8range_co(1,8) ...which is both more compact and less ugly, IMHO, and seems to circumvent all the type system problems as well. I brought that up before: http://archives.postgresql.org/pgsql-hackers/2011-06/msg02046.php It certainly circumvents the polymorphic type problems, but the problem is that it adds up to quite a few permutations. Not only are there cc/co/oc/oo, but there are also variations for infinite bounds and empty ranges. So I think we're talking 10+ functions per range type rather than 4. Also, if someone has an idea for another constructor, like the one you mention above: range(1,8,'c','o') then they have to create it for every range type, and they can't anticipate new range types that someone might create. In other words, the constructors wouldn't benefit from the polymorphism. However, if we used an intermediate type, then they could create the above constructor and it would work for any range type automatically. I don't object to this idea, but we'll need to come up with a pretty exhaustive list of possibly-useful constructors. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SSI atomic commit
In reviewing the 2PC changes mentioned in a separate post, both Dan
and I realized that these were dependent on the assumption that
SSI's commitSeqNo is assigned in the order in which the transactions
became visible. There is a race condition such that this is not
necessarily true. It is a very narrow race condition, which would
come up very rarely in practice, but Murphy's Law being what it is,
I think we need to consider it a bug and fix it.
We considered a fix which would be contained within predicate.c code
and operate by making pessimistic assumptions, so that no false
negatives occurred. The reason we didn't go that way is that the
code would be very convoluted and fragile. The attached patch just
makes it atomic in a very direct way, and adjusts the predicate.c
code to use the right tests in the right places. We were careful
not to add any LW locking to the path that a normal transaction
without an XID is terminating, since there had obviously been
significant work put into keeping locks out of that code path.
Dan and I collaborated on this code over the holiday weekend, and
are in agreement that this is the right route. I know it's only six
days before beta3, but I'm hopeful that someone can review this for
commit in time to make it into that beta.
This patch applies over the top of the fix for the 2PC permutation
problems.
I apologize for having found this bug so late in the release cycle.
-Kevin
*** a/src/backend/access/transam/twophase.c
--- b/src/backend/access/transam/twophase.c
***
*** 1298,1314 FinishPreparedTransaction(const char *gid, bool isCommit)
* callbacks will release the locks the transaction held.
*/
if (isCommit)
RecordTransactionCommitPrepared(xid,
hdr-nsubxacts, children,
hdr-ncommitrels, commitrels,
hdr-ninvalmsgs, invalmsgs,
hdr-initfileinval);
else
RecordTransactionAbortPrepared(xid,
hdr-nsubxacts, children,
hdr-nabortrels, abortrels);
!
! ProcArrayRemove(gxact-proc, latestXid);
/*
* In case we fail while running the callbacks, mark the gxact invalid
so
--- 1298,1318
* callbacks will release the locks the transaction held.
*/
if (isCommit)
+ {
RecordTransactionCommitPrepared(xid,
hdr-nsubxacts, children,
hdr-ncommitrels, commitrels,
hdr-ninvalmsgs, invalmsgs,
hdr-initfileinval);
+ CommitRemove2PC(gxact-proc, latestXid);
+ }
else
+ {
RecordTransactionAbortPrepared(xid,
hdr-nsubxacts, children,
hdr-nabortrels, abortrels);
! ProcArrayRemove(gxact-proc, latestXid);
! }
/*
* In case we fail while running the callbacks, mark the gxact invalid
so
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
***
*** 1878,1884 CommitTransaction(void)
* must be done _before_ releasing locks we hold and _after_
* RecordTransactionCommit.
*/
! ProcArrayEndTransaction(MyProc, latestXid);
/*
* This is all post-commit cleanup. Note that if an error is raised
here,
--- 1878,1884
* must be done _before_ releasing locks we hold and _after_
* RecordTransactionCommit.
*/
! CommitEndTransaction(MyProc, latestXid);
/*
* This is all post-commit cleanup. Note that if an error is raised
here,
*** a/src/backend/storage/lmgr/predicate.c
--- b/src/backend/storage/lmgr/predicate.c
***
*** 120,125
--- 120,128
*- The same lock protects a target, all locks on that target, and
*the linked list of locks on the target..
*- When more than one is needed, acquire in ascending order.
+ - There are times when this lock must be held concurrently with
+ *ProcArrayLock (acquired in called functions, not
directly).
+ *In such cases this lock
Re: [HACKERS] [v9.2] SECURITY LABEL on shared database object
On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011: Hmm, OK. I guess what I'm not sure about is - how much should we worry about the fact that this creates several more shared (and therefore nailed?) system catalogs? Anyone have an opinion on that? Several? That would worry me, given that we currently have a small number (eight currently). If it's just one more, I don't think it's such a big deal. I'm not sure what you mean by nailed though -- I mean, for example pg_shdescription is shared but not nailed in the rd_isnailed sense of the word, AFAICS. Well, right now the patch has pg_shseclabel, and its index, plus a toast table and a toast index. Not sure why we want/need the toast table index there, but the patch has 'em as of now. As a common belief, TEXT is a variable length data type, so pg_shseclabel need to have its toast table. However, I don't expect the label field get represented as a reference to external pointer, because average length of security context is about 40-60 bytes much less than the threshold to launch toast_save_datum(). Do I need to remove these toast table index? As for whether it needs to be nailed, I'm not sure I understand what the rules are there. I *think* the rule is that anything that might need to be consulted before choosing a database must be nailed. If that's right, we might be able to get by without nailing it, as long as the label isn't needed during authentication (or its use can be postponed until after we've connected to a database). In SELinux, all we are doing in the authentication hook is to acquire security label of the client, without referencing any catalogs. I also plan to support permission checks on the selected database in the future, however, I believe its hook should be placed in CheckMyDatabase() according to the existing checks. Thanks, -- NEC Europe Ltd, SAP Global Competence Center KaiGai Kohei kohei.kai...@emea.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Range Types, constructors, and the type system
On Tue, Jul 5, 2011 at 12:54 PM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2011-07-05 at 11:26 -0400, Robert Haas wrote: How about the idea of creating a family of four constructor functions for each new range type? The functions would be named after the range type, with _cc, _co, _oc, and _oo appended. So, then, instead of writing: RANGE(1,8,'c','o')::int8range It would be something like: range_co(1,8)::int8range (just so we're comparing apples to apples) The intermediate type proposal doesn't require that we move the c and o into the parameter list. Well, you have to specify the bounds somewhere... int8range_co(1,8) ...which is both more compact and less ugly, IMHO, and seems to circumvent all the type system problems as well. I brought that up before: http://archives.postgresql.org/pgsql-hackers/2011-06/msg02046.php It certainly circumvents the polymorphic type problems, but the problem is that it adds up to quite a few permutations. Not only are there cc/co/oc/oo, but there are also variations for infinite bounds and empty ranges. So I think we're talking 10+ functions per range type rather than 4. OK, so let's pass the information on the bounds as a separate argument. Like this: int8range(1,8,'co') Then you can instead pass 'o' for open or 'i' for infinity (passing NULL for the corresponding argument position in that case). The third argument can be optional and default to 'cc'. For empty ranges I doubt we need a separate constructor function; presumably the representation of an empty range is some fixed string and users can just write 'empty'::int8range or similar. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] lazy vxid locks, v2
Here is an updated version of the lazy vxid locks patch [1], which applies over the latest reduce the overhead of frequent table locks[2] patch. [1] https://commitfest.postgresql.org/action/patch_view?id=585 [2] https://commitfest.postgresql.org/action/patch_view?id=572 -- 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] beta3?
Robert Haas robertmh...@gmail.com wrote: Any other reason we can't or shouldn't wrap on the 11th? There are two new SSI issues which Dan and I spent a lot of time on over the holiday weekend. I hope they can be pushed before the 11th. I have added them to the Wiki page. -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] [v9.2] Fix leaky-view problem, part 1
On Sun, Jul 03, 2011 at 11:33:38AM +0200, Kohei KaiGai wrote:
The attached patches are revised version.
The part-0 provides 'security_barrier' option for view definition, and
performs
as a common basis of part-1 and part-2 patches.
Syntax is extended as follows:
CREATE VIEW view_name [WITH (param [=value])] AS query;
We can also turn on/off this security_barrier setting by ALTER TABLE with
SET/RESET options.
The part-1 patch enforces the qualifiers originally located under the security
barrier view to be launched prior to ones supplied on upper level.
The differences from the previous version is this barrier become conditional,
not always. So, existing optimization will be applied without any changes
onto non-security-barrier views.
I tested various query trees I considered interesting, and this version had
sound semantics for all of them. I have one suggestion for CREATE OR REPLACE
VIEW semantics, plus various cosmetic comments.
These patches are unified diffs, rather than project-standard context diffs.
Part 0:
--- a/doc/src/sgml/ref/create_view.sgml
+++ b/doc/src/sgml/ref/create_view.sgml
@@ -22,6 +22,7 @@ PostgreSQL documentation
refsynopsisdiv
synopsis
CREATE [ OR REPLACE ] [ TEMP | TEMPORARY ] VIEW replaceable
class=PARAMETERname/replaceable [ ( replaceable
class=PARAMETERcolumn_name/replaceable [, ...] ) ]
+[ WITH ( parameter [= value] [, ... ] ) ]
This needs a bit more markup; see the corresponding case in create_table.sgml.
alter_view.sgml also needs an update. Incidentally, we should use ALTER VIEW
SET OPTION when referring to setting this for a view. ALTER TABLE SET OPTION
will also support views, since that's the general pattern for tablecmds.c type
checks, but that's largely an implementation detail.
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -97,7 +97,8 @@ isViewOnTempTable_walker(Node *node, void *context)
*-
*/
static Oid
-DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
+DefineVirtualRelation(const RangeVar *relation, List *tlist,
+ bool replace, List *options)
{
Oid viewOid,
namespaceId;
This hunk and the hunk for the function's caller get rejects due to another
recent signature change.
@@ -167,6 +168,8 @@ DefineVirtualRelation(const RangeVar *relation, List
*tlist, bool replace)
{
Relationrel;
TupleDesc descriptor;
+ List *atcmds = NIL;
+ AlterTableCmd *atcmd;
/*
* Yes. Get exclusive lock on the existing view ...
@@ -211,14 +214,11 @@ DefineVirtualRelation(const RangeVar *relation, List
*tlist, bool replace)
*/
if (list_length(attrList) rel-rd_att-natts)
{
- List *atcmds = NIL;
ListCell *c;
int skip = rel-rd_att-natts;
foreach(c, attrList)
{
- AlterTableCmd *atcmd;
-
if (skip 0)
{
skip--;
@@ -229,10 +229,24 @@ DefineVirtualRelation(const RangeVar *relation, List
*tlist, bool replace)
atcmd-def = (Node *) lfirst(c);
atcmds = lappend(atcmds, atcmd);
}
- AlterTableInternal(viewOid, atcmds, true);
}
/*
+ * If optional parameters are specified, we must set options
+ * using ALTER TABLE SET OPTION internally.
I think CREATE OR REPLACE VIEW should replace the option list, while ALTER
VIEW SET OPTION should retain its current behavior. That is, this should
leave the view with no options set:
create or replace view v0(n) with (security_barrier) as values (1), (2), (3),
(4);
select reloptions from pg_class where oid = 'v0'::regclass;
create or replace view v0(n) as values (4), (3), (2), (1);
select reloptions from pg_class where oid = 'v0'::regclass;
+ */
+ if (list_length(options) 0)
+ {
+ atcmd = makeNode(AlterTableCmd);
+ atcmd-subtype = AT_SetRelOptions;
+ atcmd-def = options;
This line produces a warning:
view.c: In function `DefineVirtualRelation':
view.c:240: warning: assignment from incompatible pointer type
+
+ atcmds = lappend(atcmds, atcmd);
+ }
+ if (atcmds != NIL)
+ AlterTableInternal(viewOid, atcmds, true);
+
+ /*
* Seems okay, so return the OID of the pre-existing view.
Re: [HACKERS] lazy vxid locks, v2
On Tue, Jul 5, 2011 at 1:13 PM, Robert Haas robertmh...@gmail.com wrote: Here is an updated version of the lazy vxid locks patch [1], which applies over the latest reduce the overhead of frequent table locks[2] patch. [1] https://commitfest.postgresql.org/action/patch_view?id=585 [2] https://commitfest.postgresql.org/action/patch_view?id=572 And then I forgot the attachment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company lazyvxid-v2.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] [v9.2] SECURITY LABEL on shared database object
Excerpts from Kohei Kaigai's message of mar jul 05 11:46:06 -0400 2011: On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011: Hmm, OK. I guess what I'm not sure about is - how much should we worry about the fact that this creates several more shared (and therefore nailed?) system catalogs? Anyone have an opinion on that? Several? That would worry me, given that we currently have a small number (eight currently). If it's just one more, I don't think it's such a big deal. I'm not sure what you mean by nailed though -- I mean, for example pg_shdescription is shared but not nailed in the rd_isnailed sense of the word, AFAICS. Well, right now the patch has pg_shseclabel, and its index, plus a toast table and a toast index. Not sure why we want/need the toast table index there, but the patch has 'em as of now. As a common belief, TEXT is a variable length data type, so pg_shseclabel need to have its toast table. However, I don't expect the label field get represented as a reference to external pointer, because average length of security context is about 40-60 bytes much less than the threshold to launch toast_save_datum(). Do I need to remove these toast table index? We don't have toast tables for pg_database and so on, for example, which means that datacl cannot go over a few hundred bytes long. I think it makes sense to not have toast tables for pg_shseclabel. Keep in mind that the label might be compressed before it's stored out of line, which gives you quite a bit of actual space. If a security context is over 5000 bytes in length I think you're in trouble :-) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI atomic commit
On 05.07.2011 20:03, Kevin Grittner wrote: In reviewing the 2PC changes mentioned in a separate post, both Dan and I realized that these were dependent on the assumption that SSI's commitSeqNo is assigned in the order in which the transactions became visible. There is a race condition such that this is not necessarily true. It is a very narrow race condition, which would come up very rarely in practice, but Murphy's Law being what it is, I think we need to consider it a bug and fix it. We considered a fix which would be contained within predicate.c code and operate by making pessimistic assumptions, so that no false negatives occurred. The reason we didn't go that way is that the code would be very convoluted and fragile. The attached patch just makes it atomic in a very direct way, and adjusts the predicate.c code to use the right tests in the right places. We were careful not to add any LW locking to the path that a normal transaction without an XID is terminating, since there had obviously been significant work put into keeping locks out of that code path. Hmm, I think it would be simpler to decide that instead of SerializableXactHashLock, you must hold ProcArrayLock to access LastSxactCommitSeqNo, and move the assignment of commitSeqNo to ProcArrayTransaction(). It's probably easiest to move LastSxactCommitSeqno to ShmemVariableCache too. There's a few places that would then need to acquire ProcArrayLock to read LastSxactCommitSeqno, but I feel it might still be much simpler that way. -- 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] SSI 2PC coverage
On 05.07.2011 20:06, Kevin Grittner wrote: [resending after gzip of test patch] In reviewing the recent fix to 2PC coverage in SSI, I found some cases which didn't seem to be covered. Dan bit the bullet and came up with an additional isolation test to rigorously cover all the permutations, to find *all* 2PC statement orderings which weren't working right. Because it was so big, he pared out tests which were redundant, in that they exercised the same code paths and pointed at the same issues. A patch to add this test is attached. Run against HEAD it shows the errors. It's kinda big, but I think it's worth having. Attached is also a patch to fix those, so that all permutations work. I think that needs some explanation, why only those SxactIsCommitted() tests need to be replaced with SxactIsPrepared()? What about the first SxactIsCommitted() test in OnConflict_CheckForSerializationFailure(), for instance? -- 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] SSI atomic commit
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Hmm, I think it would be simpler to decide that instead of SerializableXactHashLock, you must hold ProcArrayLock to access LastSxactCommitSeqNo, and move the assignment of commitSeqNo to ProcArrayTransaction(). It's probably easiest to move LastSxactCommitSeqno to ShmemVariableCache too. There's a few places that would then need to acquire ProcArrayLock to read LastSxactCommitSeqno, but I feel it might still be much simpler that way. We considered that. I think the biggest problem was that when there is no XID it wouldn't be covered by the lock on assignment. We couldn't see a good way to increment and assign the value without LW lock coverage, and we didn't want to add LW locking to that code path. If you can see a way around that issue, I agree it would be simpler. -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] Review of VS 2010 support patches
Original Message
Subject: Review of VS 2010 support patches
From: Craig Ringer cr...@postnewspapers.com.au
To: PG Hackers pgsql-hackers@postgresql.org, Brar Piening b...@gmx.de
Date: 05.07.2011 14:25
I haven't had any reply to my email to Brar, so there are a few
details (like whether x64 builds were tested and how x64 required
libraries were built) I could use, but what I've got done so far seems
fine.
I've replied on-list see:
http://archives.postgresql.org/pgsql-hackers/2011-07/msg00066.php
Seems like i've got fooled by reply to list being thunderbird's
default for mailing lists once more. Sorry for that one.
The patch (VS2010v7.patch) seems to mix significant changes with
whitespace fixes etc.
Current version (VS2010v8.patch) which I've submitted on-list about one
month ago has fixed this as per Tom Lane's comment.
See: http://archives.postgresql.org/message-id/4dedb6ee.9060...@gmx.de
pgflex.pl and pgbison.pl
=
pgflex.pl and pgbison.pl are a big improvement over the horrid batch
files, but are perhaps too little a translation. There's no need for
the big if(string) then (otherstring) stuff; it can be done much more
cleanly by storing a simple hash of paths to options and doing a file
extension substitution to generate the output filenames. The hash only
needs to be populated for files that get processed with non-default
options, so for pgflex all you need is:
%LEX_OPTS = { 'src\backend\parser\scan.c' - '-CF' };
I can send adjusted versions of pgflex.pl and pgbison.pl that
I think the approach Andrew Dunstan chose (parsing the Makefiles) is
even more flexible and future proof. We should probably be using his
versions.
See: http://archives.postgresql.org/pgsql-hackers/2011-07/msg00140.php
and http://archives.postgresql.org/pgsql-hackers/2011-07/msg00185.php
DOCUMENTATION
===
I didn't notice any documentation updates to reflect the fact that
Visual Studio 2010 is now supported. It'd be a good idea to change
install-windows-full.html (or the source of it, anyway) to mention VS
2010 support.
Yep - a clear leftover. I've never written any SGML but I'll try to come
up with something as soon as as I've got the doc build working on my system.
I'm not sure if I'll be able to get 64-bit copies of all the optional
libraries built, so it may be a more minimal build. It'll include at
least zlib, plperl and plpython 64-bit support, though. Information
from Briar about whether he built for 64-bit and if so how he got his
libraries built would help.
Actually my default builds are 64-bit builds as my PC is Win7 x64 and
I'm using 64-Bit versions for my PostgreSQL work.
As you noted, the availability of 64-bit libraries was the limiting
factor for more extensive testing but I haven't run into any Problems
with my default configuration (nothing but plperl) and some others I've
tried yet.
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] SSI atomic commit
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Hmm, I think it would be simpler to decide that instead of SerializableXactHashLock, you must hold ProcArrayLock to access LastSxactCommitSeqNo, and move the assignment of commitSeqNo to ProcArrayTransaction(). It's probably easiest to move LastSxactCommitSeqno to ShmemVariableCache too. There's a few places that would then need to acquire ProcArrayLock to read LastSxactCommitSeqno, but I feel it might still be much simpler that way. Yeah ... this patch creats the need to hold both SerializableXactHashLock and ProcArrayLock during transaction commit, which is a bit scary from a deadlock-risk perspective, and not pleasant from the concurrency standpoint either. It'd be better to push some functionality into the procarray code. 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] SSI 2PC coverage
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Attached is also a patch to fix those, so that all permutations work. I think that needs some explanation, why only those SxactIsCommitted() tests need to be replaced with SxactIsPrepared()? What about the first SxactIsCommitted() test in OnConflict_CheckForSerializationFailure(), for instance? Well, that's covered in the other patch. This one has the minimum required to get all the permutations of 2PC working correctly. It was looking at just such questions as you pose here that led us to the other patch. Neither macro has quite the right semantics without the lower level work in the atomic commit patch. -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] %ENV warnings during builds
Original Message Subject: Re: [HACKERS] %'ENV warnings during builds From: Andrew Dunstan and...@dunslane.net To: Brar Piening b...@gmx.de Date: 05.07.2011 17:25 Try attached instead. I can confirm that this version of pgflex.pl works as expected in my environment. 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] Bug in SQL/MED?
Robert Haas robertmh...@gmail.com writes: 2011/7/1 Shigeru Hanada shigeru.han...@gmail.com: I used ereport for the former check, because maybe such error usually happens and is visible to users. Â This criteria was taken from the document Reporting Errors Within the Server. http://developer.postgresql.org/pgdocs/postgres/error-message-reporting.html Do we want to apply this to 9.1 before beta3? The original patch in this thread seems pretty bogus to me, because it changes only one place of many in foreigncmds.c where PointerGetDatum(NULL) is used as the representation of an empty options list. If we're going to go over to the rule that pg_foreign_data_wrapper.fdwoptions can't be NULL, which is what this is effectively proposing, we need much more extensive changes than this. Also, since foreigncmds.c is sharing code with reloptions.c, wherein the PointerGetDatum(NULL) convention is also used, I'm concerned that we're going to have more bugs added than removed by changing the rule for just pg_foreign_data_wrapper.fdwoptions. I think it might be better to keep the convention that an empty options list is represented by null, and to say that if a validator wants to be called on such a list, it had better declare itself non-strict. At least we ought to think about that before redefining the catalog semantics at this late hour. 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] SSI atomic commit
Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: Hmm, I think it would be simpler to decide that instead of SerializableXactHashLock, you must hold ProcArrayLock to access LastSxactCommitSeqNo, and move the assignment of commitSeqNo to ProcArrayTransaction(). It's probably easiest to move LastSxactCommitSeqno to ShmemVariableCache too. There's a few places that would then need to acquire ProcArrayLock to read LastSxactCommitSeqno, but I feel it might still be much simpler that way. Yeah ... this patch creats the need to hold both SerializableXactHashLock and ProcArrayLock during transaction commit, which is a bit scary from a deadlock-risk perspective, If I remember correctly, we already do some calls to functions which acquire ProcArrayLock while holding SerializableXactHashLock, in order to acquire a snapshot with the right timing. Regardless of what we do on transaction completion, we either need to document that in the code comments, or do some major surgery at the last minute, which is always scary. and not pleasant from the concurrency standpoint either. On the bright side, both locks are not held at the same time unless the transaction isolation level is serializable. It'd be better to push some functionality into the procarray code. That's easily done if we don't mind taking out a ProcArrayLock during completion of a transaction which has no XID, if only long enough to increment a uint64 in shared memory, and then stash the value -- somewhere -- so that SSI code can find and use it. We thought it was best to avoid that so there wasn't any impact on non-serializable transactions. -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] Bug in SQL/MED?
I wrote:
I think it might be better to keep the convention that an empty options
list is represented by null, and to say that if a validator wants to be
called on such a list, it had better declare itself non-strict. At
least we ought to think about that before redefining the catalog
semantics at this late hour.
Another possibility that just occurred to me is to call the validator
like this:
if (OidIsValid(fdwvalidator))
{
Datumvalarg = result;
/* pass a null options list as an empty array */
if (DatumGetPointer(valarg) == NULL)
valarg = construct_empty_array(TEXTOID);
OidFunctionCall2(fdwvalidator, valarg, ObjectIdGetDatum(catalogId));
}
This would avoid messing with the semantics of empty options lists
throughout foreigncmds.c, and also avoid requiring validators to deal
with null arguments.
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] SSI atomic commit
On Tue, Jul 5, 2011 at 2:35 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: It'd be better to push some functionality into the procarray code. That's easily done if we don't mind taking out a ProcArrayLock during completion of a transaction which has no XID, if only long enough to increment a uint64 in shared memory, and then stash the value -- somewhere -- so that SSI code can find and use it. That sure sounds scary from a scalability perspective. If we can piggyback on an existing ProcArrayLock acquisition, fine, but additional ProcArrayLock acquisitions when SSI isn't even being used sound like a real bad idea to me. I doubt you'll notice much of a performance regression in the current code, but if/when we fix the lock manager bottlenecks that my fastlock and lazy vxid lock patches are intended to correct, then I suspect it's going to matter quite a bit. -- 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] SSI atomic commit
Robert Haas robertmh...@gmail.com writes: On Tue, Jul 5, 2011 at 2:35 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: That's easily done if we don't mind taking out a ProcArrayLock during completion of a transaction which has no XID, if only long enough to increment a uint64 in shared memory, and then stash the value -- somewhere -- so that SSI code can find and use it. That sure sounds scary from a scalability perspective. If we can piggyback on an existing ProcArrayLock acquisition, fine, but additional ProcArrayLock acquisitions when SSI isn't even being used sound like a real bad idea to me. Isn't SSI *already* forcing a new acquisition of an LWLock during commits of read-only transactions that aren't using SSI? Perhaps there's a bit less contention on SerializableXactHashLock than on ProcArrayLock, but it's not obvious that the current situation is a lot better than this would be. 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] SSI atomic commit
Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 5, 2011 at 2:35 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: It'd be better to push some functionality into the procarray code. That's easily done if we don't mind taking out a ProcArrayLock during completion of a transaction which has no XID, if only long enough to increment a uint64 in shared memory, and then stash the value -- somewhere -- so that SSI code can find and use it. That sure sounds scary from a scalability perspective. If we can piggyback on an existing ProcArrayLock acquisition, fine, but additional ProcArrayLock acquisitions when SSI isn't even being used sound like a real bad idea to me. I doubt you'll notice much of a performance regression in the current code, but if/when we fix the lock manager bottlenecks that my fastlock and lazy vxid lock patches are intended to correct, then I suspect it's going to matter quite a bit. Just to be clear, the patch as submitted does *not* acquire a ProcArrayLock lock for completion of a transaction which has no XID. What you quoted from me was explaining what would seem to be required (unless Dan and I missed something) to simplify the patch as Tom and Heikki proposed. We saw that approach and its simplicity, but shied away from it because of the locking issue and its potential performance impact. We took the route we did in this fix patch to avoid such issues. I'm not so sure that the impact on a load of many short read-only transactions would be so benign as things stand. It seemed to me that one of the reasons to avoid *getting* an XID was to avoid acquiring ProcArrayLock on transaction completion. The way we did this patch may indeed slow serializable transactions more than the alternative; but from the beginning I thought the ground rules were that SSI had to have no significant impact on those who didn't choose to use it. I suspect that most of the 9.2 work on SSI will be for improved performance (including in that, as I do, a reduction in the percentage of false positive serialization failures). Tuning this should go on that list. It may well benefit from using some of the techniques you've been working on. -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] SSI atomic commit
Tom Lane t...@sss.pgh.pa.us wrote: Isn't SSI *already* forcing a new acquisition of an LWLock during commits of read-only transactions that aren't using SSI? During COMMIT PREPARED there is one. We could avoid that by storing the transaction isolation level in the persistent data for a prepared statement, but that seems inappropriate for 9.1 at this point, and it's hard to be sure that would be a net win. Otherwise I don't *think* there's an extra LW lock for a non-serializable transaction (whether or not read-only). Do you see one I'm not remembering? -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] SSI atomic commit
On Tue, Jul 05, 2011 at 01:15:13PM -0500, Kevin Grittner wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Hmm, I think it would be simpler to decide that instead of SerializableXactHashLock, you must hold ProcArrayLock to access LastSxactCommitSeqNo, and move the assignment of commitSeqNo to ProcArrayTransaction(). It's probably easiest to move LastSxactCommitSeqno to ShmemVariableCache too. There's a few places that would then need to acquire ProcArrayLock to read LastSxactCommitSeqno, but I feel it might still be much simpler that way. We considered that. I think the biggest problem was that when there is no XID it wouldn't be covered by the lock on assignment. One other issue is that after the sequence number is assigned, it still needs to be stored in MySerializableXact-commitSeqNo. Modifying that does require taking SerializableXactHashLock. With the proposed patch, assigning the next commitSeqNo and storing it in MySerializableXact happen atomically. That makes it possible to say that a transaction that has a commitSeqNo must have committed before one that doesn't. If the two steps are separated, that isn't true: two transactions might get their commitSeqNos in one order and make them visible in the other. We should be able to deal with that, but it will make some of the commit ordering checks more complicated. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.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] Small documentation issue
Robert Haas robertmh...@gmail.com writes: On Fri, Jul 1, 2011 at 3:15 AM, Albe Laurenz laurenz.a...@wien.gv.at wrote: In fdwhandler.sgml, chapter fdwhandler has only one subsection (fdw-routines). If there is only one subsection, no table of contents is generated in the chapter. That means that people who navigate to the chapter from the main table of contents will not see that there is a subsection. I know too little about the documentation building process, but is it possible to generate a table of contents even if there is only one subsection? I don't know how to change the doc toolchain to do that either. But on reflection it seemed to me that this documentation was badly designed anyhow, because it mixed up description of the FDW's routines with actual introductory text. I split that into an introduction and a sect1 for the routines. Problem solved. Maybe we could just add a sentence to the end of the third paragraph with a pointer to the section that follows, so it would read like this: The callback functions are plain C functions and are not visible or callable at the SQL level. They are described in more detail in Section 50.1. Yeah, I did that too. 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
[HACKERS] Changing extension schema fails silently
Hi, I'm using the latest head and I created the file_fdw extension, then attempted to change its schema (ALTER EXTENSION file_fdw SET SCHEMA new_schema. No error was returned, but it remained in the same schema (according to pg_extension). I then dropped the extension and created it again specifying the new schema, and it is correctly assigned to that schema. Am I missing something, or does this fail silently? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 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
Re: [HACKERS] SSI 2PC coverage
On Tue, Jul 05, 2011 at 09:14:30PM +0300, Heikki Linnakangas wrote: I think that needs some explanation, why only those SxactIsCommitted() tests need to be replaced with SxactIsPrepared()? Here is the specific problem this patch addresses: If there's a dangerous structure T0 --- T1 --- T2, and T2 commits first, we need to abort something. If T2 commits before both conflicts appear, then it should be caught by OnConflict_CheckForSerializationFailure. If both conflicts appear before T2 commits, it should be caught by PreCommit_CheckForSerializationFailure. But that is actually run before T2 *prepares*. So the problem occurs if T2 is prepared but not committed when the second conflict is detected. OnConflict_CFSF deems that OK, because T2 hasn't committed yet. PreCommit_CFSF doesn't see a problem either, because the conflict didn't exist when it ran (before the transaction prepared) This patch fixes that by having OnConflict_CFSF declare a serialization failure in this circumstance if T2 is already prepared, not just if it's committed. Although it fixes the situation described above, I wasn't initially confident that there weren't other problematic cases. I wrote the attached test case to convince myself of that. Because it tests all possible sequences of conflict/prepare/commit that should lead to serialization failure, the fact that they do all fail (with this patch) indicates that these are the only checks that need to be changed. What about the first SxactIsCommitted() test in OnConflict_CheckForSerializationFailure(), for instance? That one only comes up if the SERIALIZABLEXACT for one of the transactions involved has been freed, and the RWConflict that points to it has been replaced by a flag. That only happens if writer has previously called ReleasePredicateLocks. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.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] Changing extension schema fails silently
On 5 July 2011 21:27, Thom Brown t...@linux.com wrote: Hi, I'm using the latest head and I created the file_fdw extension, then attempted to change its schema (ALTER EXTENSION file_fdw SET SCHEMA new_schema. No error was returned, but it remained in the same schema (according to pg_extension). I then dropped the extension and created it again specifying the new schema, and it is correctly assigned to that schema. Am I missing something, or does this fail silently? Correction, the objects which belong to the extension do switch schema, but the properties of the extension itself indicate the extension is in a different schema. So rather than not working at all, it seems that it's just forgotten to update the pg_extension catalog table. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 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
Re: [HACKERS] Changing extension schema fails silently
From the docs: Note that unlike most catalogs with a namespace column, extnamespace is not meant to imply that the extension belongs to that schema. Extension names are never schema-qualified. Rather, extnamespace indicates the schema that contains most or all of the extension's objects. If extrelocatable is true, then this schema must in fact contain all schema-qualifiable objects belonging to the extension. However, if you look at the source, the function AlterExtensionNamespace(List *names, const char *newschema) has this line: /* Now adjust pg_extension.extnamespace */ extForm-extnamespace = nspOid; So clearly the catalog column ought to have been updated. I can't recreate the problem here, and I too am working from git head on the master branch. -- 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] Moving the community git server
Sometime later this week, the community git server (git.postgresql.org) will be migrated to a new server in the same datacenter. This does *not* affect the master git server for the project, just the anonymous mirror and those that run standalone projects on it, such as pgAdmin. When this move is done, the IP will change to a different address in the same subnet, and the server will have a new SSH key. That means you will get a security warning if you use the ssh protocol. The fingerprint of the new server is going to be: cc:0c:27:40:d7:a2:98:f4:69:07:0c:b6:3a:05:19:71 - verify that this is the one when you get a warning. When the server has been migrated, the old one will be switched to read-only access through http and git protocols for a day or two. The ssh protocol will be completely turned off on the old server. If you get an error about ssh login, it may simply mean that your DNS hasn't yet updated. I'll post again here with more details when the actual move happens, but I wanted to get a notice out early since it will cause security warnings for anybody trying to access it after the move. -- 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] Changing extension schema fails silently
On 5 July 2011 22:31, Peter Geoghegan pe...@2ndquadrant.com wrote: From the docs: Note that unlike most catalogs with a namespace column, extnamespace is not meant to imply that the extension belongs to that schema. Extension names are never schema-qualified. Rather, extnamespace indicates the schema that contains most or all of the extension's objects. If extrelocatable is true, then this schema must in fact contain all schema-qualifiable objects belonging to the extension. However, if you look at the source, the function AlterExtensionNamespace(List *names, const char *newschema) has this line: /* Now adjust pg_extension.extnamespace */ extForm-extnamespace = nspOid; So clearly the catalog column ought to have been updated. I can't recreate the problem here, and I too am working from git head on the master branch. D'oh, I've discovered the problem. It's my copy of PgAdmin that was reporting the wrong name. Nothing to see here. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 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
Re: [HACKERS] Changing extension schema fails silently
Thom Brown t...@linux.com writes: Correction, the objects which belong to the extension do switch schema, but the properties of the extension itself indicate the extension is in a different schema. So rather than not working at all, it seems that it's just forgotten to update the pg_extension catalog table. Really? Works for me. 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] Changing extension schema fails silently
On 5 July 2011 23:00, Tom Lane t...@sss.pgh.pa.us wrote: Thom Brown t...@linux.com writes: Correction, the objects which belong to the extension do switch schema, but the properties of the extension itself indicate the extension is in a different schema. So rather than not working at all, it seems that it's just forgotten to update the pg_extension catalog table. Really? Works for me. It was a Thom fail. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 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
Re: [HACKERS] Bug in SQL/MED?
Shigeru Hanada shigeru.han...@gmail.com writes: Thanks for the comments. Please find attached a patch. Now file_fdw validates filename in: * file_fdw_validator(), to catch lack of required option at DDL * fileGetOptions(), to avoid crash caused by corrupted catalog Applied with small adjustments. 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] Bug in SQL/MED?
I wrote:
Another possibility that just occurred to me is to call the validator
like this:
if (OidIsValid(fdwvalidator))
{
Datumvalarg = result;
/* pass a null options list as an empty array */
if (DatumGetPointer(valarg) == NULL)
valarg = construct_empty_array(TEXTOID);
OidFunctionCall2(fdwvalidator, valarg, ObjectIdGetDatum(catalogId));
}
This would avoid messing with the semantics of empty options lists
throughout foreigncmds.c, and also avoid requiring validators to deal
with null arguments.
Not hearing any objections, I've fixed it that way.
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] Crash dumps
On 5/07/2011 9:05 PM, Magnus Hagander wrote: On Tue, Jul 5, 2011 at 15:02, Robert Haasrobertmh...@gmail.com wrote: On Mon, Jul 4, 2011 at 12:47 PM, Radosław Smogura rsmog...@softperience.eu wrote: I asked about crash reports becaus of at this time there was thread about crashing in live system. Yeah, I thought this was the result of that effort: [snip] That crash dump is basically the windows equivalent of a coredump, though. Just a different name... Yup, it's a cut-down core dump. In this case generated in-process by the crashing backend. It'd be nice to be able to generate the crash dump from out-of-process. Unfortunately, the automatic crash dump generation system on Windows doesn't appear to be available to system services running non-interactively. Not that I could see, anyway. As a result we had to trap the crashes within the crashing process and generate the dump from there. As previously stated, doing anything within a segfaulting process is unreliable, so it's not the best approach in the world. All I was saying in this thread is that it'd be nice to have a way for a crashing backend to request that another process capture diagnostic information from it before it exits with a fault, so it doesn't have to try to dump its self. As Tom said, though, anything like that is more likely to decrease the reliability of the overall system. You don't want a dead backend hanging around forever waiting for the postmaster to act on it, and you *really* don't want other backends still alive and potentially writing from shm that's in in who-knows-what state while the postmaster is busy fiddling with a crashed backend. So, overall, I think dump a simple core and die as quickly as possible is the best option. That's how it already works on UNIX, and all the win32 crash dump patches do is make it work on Windows too. -- Craig Ringer POST Newspapers 276 Onslow Rd, Shenton Park Ph: 08 9381 3088 Fax: 08 9388 2258 ABN: 50 008 917 717 http://www.postnewspapers.com.au/ -- Sent 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, Jul 5, 2011 at 8:08 PM, Simon Riggs si...@2ndquadrant.com wrote: Now for the rest of the review... Thanks! I'd rather not include another chunk of code related to wal_keep_segments. The existing code in CreateCheckPoint() should be refactored so that we call the same code from both CreateCheckPoint() and CreateRestartPoint(). This makes sense. Will do. IMHO it's time to get rid of RECOVERYXLOG as an initial target for de-archived files. That made sense once, but now we have streaming it makes more sense for us to de-archive straight onto the correct file name and let the file be cleaned up later. So de-archiving it and then copying to the new location doesn't seem the right thing to do (especially not to copy rather than rename). RECOVERYXLOG allowed us to de-archive the file without removing a pre-existing file, so we must handle that still - the current patch would fail if a pre-existing WAL file were there. You mean that we must keep a pre-existing file? If so, why do we need to do that? Since it's older than an archived file, it seems to be OK to overwrite it with an archived file. Or is there corner case that an archived file is older than a pre-existing one? If we don't need to keep a pre-existing file, I'll change the way to de-archive according to your suggestion, as follows; 1. Rename a pre-existing file to EXISTINGXLOG 2. De-archive the file onto the correct name 3. If the de-archived file is invalid (i.e., its size is not 16MB), remove it and rename EXISTINGXLOG to the correct name 4. If the de-archived file is valid, remove EXISTINGXLOG 5. Replay the file with the correct name Or 1. De-archive the file to RECOVERYXLOG 2. If RECOVERYXLOG is valid, remove a pre-existing one and rename RECOVERYXLOG to the correct name 3. Replay the file with the correct name Those changes will make this code cleaner for the long term. I don't think we should simply shutdown a WALSender when we startup. That is indistinguishable from a failure, which is going to be very worrying if we do a switchover. Is there another way to do this? Or if not, at least a log message to explain it was normal that we requested this. What about outputing something like the following message in that case? if (walsender receives SIGUSR2) ereport(LOG, terminating walsender process due to administrator command); It would be possible to have synchronous cascaded replication but that is probably another patch :-) Yeah, right. You'll try? ;) 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] Potential NULL dereference found in typecmds.c
Excerpts from Alvaro Herrera's message of lun jul 04 11:12:32 -0400 2011: 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. I forgot to close this: I applied Heikki's suggested fix yesterday. Thanks for the report. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] weird cast behavior in IN (values) clause
I just came across this:
alvherre=# select * from pg_class where oid::regclass in ('foo');
ERROR: invalid input syntax for type oid: foo
LÍNEA 1: select * from pg_class where oid::regclass in ('foo');
^
alvherre=# select * from pg_class where oid::regclass in ('foo', 'foo');
relname | relnamespace | reltype | reloftype | relowner | relam | relfilenode
| reltablespace | relpages | reltuples | reltoastrelid | reltoastidxid |
relhasindex | relisshared | relpersistence | relkind | relnatts | relchecks |
relhasoids | relhaspkey | relhasrules | relhastriggers | relhassubclass |
relfrozenxid | relacl | reloptions
-+--+-+---+--+---+-+---+--+---+---+---+-+-++-+--+---+++-+++--++
foo | 2200 | 16448 | 0 | 10 | 0 | 16446
| 0 |0 | 0 | 0 | 0 | t
| f | p | r |1 | 0 | f
| t | f | f | f | 720 |
|
(1 fila)
Not sure what to make of it.
(The reason I put the regclass cast in the oid instead of the other way
around is that I was trying a bunch of other tables, so it was
oid::regclass IN ('foo', 'bar', 'baz')
which is a lot easier to type than attaching a regclass cast to each
literal).
I am not sure why it would be valid to list two literals in the values
but not one.
--
Álvaro Herrera alvhe...@alvh.no-ip.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] weird cast behavior in IN (values) clause
Alvaro Herrera alvhe...@alvh.no-ip.org writes: I am not sure why it would be valid to list two literals in the values but not one. The discrepancy seems to be because transformAExprIn uses a different type resolution method when there's more than one non-Var in the RHS. Maybe we should apply select_common_type even when there's only one RHS non-Var, even though we don't want to use a ScalarArrayOpExpr? Curious that it's acted like this since 8.2 and nobody complained. 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] Range Types, constructors, and the type system
On Tue, 2011-07-05 at 13:06 -0400, Robert Haas wrote: On Tue, Jul 5, 2011 at 12:54 PM, Jeff Davis pg...@j-davis.com wrote: It would be something like: range_co(1,8)::int8range (just so we're comparing apples to apples) The intermediate type proposal doesn't require that we move the c and o into the parameter list. Well, you have to specify the bounds somewhere... That's true. In my example it's in the function name. OK, so let's pass the information on the bounds as a separate argument. Like this: int8range(1,8,'co') That has a lot going for it, in the sense that it avoids dealing with the type problems. Then you can instead pass 'o' for open or 'i' for infinity (passing NULL for the corresponding argument position in that case). The third argument can be optional and default to 'cc'. The fact that there can be a default for the third argument makes this quite a lot more appealing than I had originally thought (although I think 'co' is the generally-accepted default). There's some slight ugliness around the NULL/infinity business, but I think that I could be convinced. I'd like to avoid confusion between NULL and infinity if possible. Regards, Jeff Davis -- Sent 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 Wed, Jul 6, 2011 at 2:13 AM, Fujii Masao masao.fu...@gmail.com wrote: IMHO it's time to get rid of RECOVERYXLOG as an initial target for de-archived files. That made sense once, but now we have streaming it makes more sense for us to de-archive straight onto the correct file name and let the file be cleaned up later. So de-archiving it and then copying to the new location doesn't seem the right thing to do (especially not to copy rather than rename). RECOVERYXLOG allowed us to de-archive the file without removing a pre-existing file, so we must handle that still - the current patch would fail if a pre-existing WAL file were there. snip If we don't need to keep a pre-existing file, I'll change the way to de-archive according to your suggestion, as follows; 1. Rename a pre-existing file to EXISTINGXLOG 2. De-archive the file onto the correct name 3. If the de-archived file is invalid (i.e., its size is not 16MB), remove it and rename EXISTINGXLOG to the correct name 4. If the de-archived file is valid, remove EXISTINGXLOG 5. Replay the file with the correct name I'm laughing quite hard here... :-) Or 1. De-archive the file to RECOVERYXLOG 2. If RECOVERYXLOG is valid, remove a pre-existing one and rename RECOVERYXLOG to the correct name 3. Replay the file with the correct name Yes please, that makes sense. Those changes will make this code cleaner for the long term. I don't think we should simply shutdown a WALSender when we startup. That is indistinguishable from a failure, which is going to be very worrying if we do a switchover. Is there another way to do this? Or if not, at least a log message to explain it was normal that we requested this. What about outputing something like the following message in that case? if (walsender receives SIGUSR2) ereport(LOG, terminating walsender process due to administrator command); ...which doesn't explain the situation because we don't know why SIGUSR2 was sent. I was thinking of something like this LOG: requesting walsenders for cascading replication reconnect to update timeline but then I ask: Why not simply send a new message type saying new timeline id is X and that way we don't need to restart the connection at all? It would be possible to have synchronous cascaded replication but that is probably another patch :-) Yeah, right. You'll try? ;) I'll wait for a request... -- 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
