Re: [HACKERS] Problem with sources.
Success ! I can't use git protocol, but github via http works fine. Thank you Andrew :) pasman -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] augmenting MultiXacts to improve foreign keys
On Tue, Aug 09, 2011 at 09:41:00PM +0200, Florian Pflug wrote: > Couldn't we simply give the user a way to specify, per column, whether or > not that column is a "KEY" column? Creating a foreign key constraint could > still implicitly mark all referenced columns as KEY columns, but columns > would no longer be "KEY" columns simply because they're part of a UNIQUE > constraint. Users would be free to add arbitrary columns to the set of > "KEY" columns by doing ALTER TABLE ALTER COLUMN SET KEY. What would be the use case for manually expanding the set of key columns? If you have a heavily-updated table referenced by slowly-changing tables, it might pay off to disable the optimization entirely. That would be an esoteric escape hatch to provide for an uncommon use case, though. -- Noah Mischhttp://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] Policy on pulling in code from other projects?
On Sat, Jul 23, 2011 at 3:39 AM, Andrew Dunstan wrote: > > 1. I think the proposed use is of very marginal value at best, and > certainly not worth importing an external library for. > > Now that I've seen two people who seem to think that this is not an important feature I'll wade in and respond to this idea. I think it's very easy to doubt the value of a definitively recognizable string that represents a postgres database when you don't have a heterogenous environment with more than a hundred thousand applications of all types in it. To make matters worse, as language support on that platform continues to widen beyond its humble beginnings, there isn't a standard across those languages for what constitutes a postgres URL. This is the current situation at Heroku, where we currently run ~150,000 individual databases on our infrastructure as well as a variety of other databases such as MySQL, Redis, Mongo, Couch, Riak, Cassandra, &c. To head off the most obvious criticism, we aren't using connection strings in our system because there isn't any reasonable way to recognize them. A PG conninfo string is just a set of key value pairs with no dependably present signifier. This is why almost every database library from Ruby to Python to Java takes some form of a URL with a protocol called "postgres" in it in order to help select which driver to use. Further, support (and syntax!) for the more esoteric connection parameters varies from library to library as well as between languages. A good spec by the project would go a long way in resolving this, and I can at least be confident that we could get it adopted very quickly by all three of the Ruby-community Postgres libraries. In conclusion, this is a serious operational concern for me and my team and I will be personally dealing with fires caused by this for years to come regardless of the outcome of this thread. Best, -pvh -- Peter van Hardenberg Department of Data Heroku "Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
Re: [HACKERS] Reduced power consumption in autovacuum launcher process
Peter Geoghegan writes: > On 9 August 2011 23:07, Tom Lane wrote: >> Now that I've got the WaitLatch code fully swapped into my head, >> I'm thinking of pushing on to review/commit this patch of Peter's. > Thanks for giving this your attention. I had already planned to > produce a new revision this weekend, so I'd appreciate it if you could > hold off until Sunday or Monday. Actually, I'm nearly done with it already. Perhaps you could start thinking about the other polling loops. 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] augmenting MultiXacts to improve foreign keys
On Aug 9, 2011, at 4:40 PM, Tom Lane wrote: > Like Florian, I'm considerably more concerned about the aspect of > deciding which columns are "key columns" and whether they changed. Should we consider trying implement a system that can lock individual columns? Either way, my main concern is that we're going to end up pessimizing the common case of UPDATE, by making it do extra work to reduce the chances of a lock conflict from an incoming foreign key. Most of the time there won't be an incoming foreign key, or the referring row won't get updated, and that work will be wasted. It would be nice to just lock the row "for some kind of update" and sort out what exactly we locked only if a possible conflict comes along. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduced power consumption in autovacuum launcher process
On 9 August 2011 23:07, Tom Lane wrote: > Now that I've got the WaitLatch code fully swapped into my head, > I'm thinking of pushing on to review/commit this patch of Peter's. Thanks for giving this your attention. I had already planned to produce a new revision this weekend, so I'd appreciate it if you could hold off until Sunday or Monday. > I did not see any objections to such a change. I think we should pull > out this aspect and commit it to 9.1 as well as HEAD. That will provide > one less gotcha for anyone who develops against the 9.1 latch code and > later needs to port to 9.2. That is a good point. I'm aware that someone already made the mistake of giving the value of timeout as milliseconds rather than microseconds at one point, so this seems to be a fertile source of confusion. -- 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] Reduced power consumption in autovacuum launcher process
Peter Geoghegan writes: > Attached is revision of this patch that now treats the latch in > PGPROC, waitLatch, as the generic "process latch", rather than just > using it for sync rep; It is initialised appropriately as a shared > latch generically, within InitProcGlobal(), and ownership is > subsequently set within InitProcess(). We were doing so before, though > only for the benefit of sync rep. Now that I've got the WaitLatch code fully swapped into my head, I'm thinking of pushing on to review/commit this patch of Peter's. > On 18 July 2011 20:06, Heikki Linnakangas > wrote: >> Right, we can easily change the timeout argument to be in milliseconds >> instead of microseconds. > I've done so in this latest revision as a precautionary measure. I > don't see much point in sub-millisecond granularity, and besides, the > Windows implementation will not provide that granularity anyway as > things stand. I did not see any objections to such a change. I think we should pull out this aspect and commit it to 9.1 as well as HEAD. That will provide one less gotcha for anyone who develops against the 9.1 latch code and later needs to port to 9.2. 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] Ignore lost+found when checking if a directory is empty
Alvaro Herrera writes: > Excerpts from Jeff Davis's message of mar ago 09 16:03:26 -0400 2011: >> I think I agree with Peter here that it's not a very good idea, and I >> don't see a big upside. With tablespaces it seems to make a little bit >> more sense, but I'd still lean away from that idea. > What if the init script tries to start postmaster before the filesystems > are mounted? ISTM requiring a subdir is a good sanity check that the > system is ready to run. Not creating stuff directly on the mountpoint > ensures consistency. I went looking in the archives for previous discussions of this idea. Most of them seem to focus on tablespaces rather than the primary data directory, but the objections to doing it are pretty much the same either way. The security concerns I mentioned seem to boil down to this (from <25791.1132238...@sss.pgh.pa.us>): Yeah, you *can* make it not-root-owned on most Unixen. That doesn't mean it's a good idea to do so. For instance, if the root directory is owned by Joe Luser, what's to stop him from blowing away lost+found and thereby screwing up future fscks? You should basically never have more-privileged objects (such as lost+found) inside directories owned by less-privileged users --- it's just asking for trouble. 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] augmenting MultiXacts to improve foreign keys
Excerpts from Tom Lane's message of mar ago 09 16:40:15 -0400 2011: > Alvaro Herrera writes: > > Excerpts from Jeff Davis's message of mar ago 09 14:41:14 -0400 2011: > >> Right now, FKs aren't really very special, they are mostly just > >> syntactic sugar (right?). This proposal would make FKs special internal > >> mechanisms, and I don't see the benefit in doing so. > > > Well, you can get the same behavior by adding the constraint triggers > > manually. But those triggers are written in C, so you could equally > > claim that they are "special internal" already. The SPI interface has > > some special entry points to allow them to work correctly (for example > > passing a snapshot for the checks to run with). > > Yeah, the crosscheck-snapshot logic already puts the lie to any idea > that the RI triggers are equivalent to anything available at the SQL > level. > > ISTM you could pass down the "please do this with FOR KEY UPDATE not > just FOR SHARE" flag similarly to the way the crosscheck snapshot is > passed down, or maybe even just do it if you see a crosscheck snapshot > is present in the executor state (though that's a bit ugly). You mean FOR KEY SHARE ... Yeah, I could pass it down that way. But if we go that route, do we need to specify any specific rowmark clause at all? Maybe we could go back to FOR UPDATE and use the flag to tell the executor that it's actually FOR KEY SHARE, and deprecate the FOR SHARE clause altogether. > Like Florian, I'm considerably more concerned about the aspect of > deciding which columns are "key columns" and whether they changed. I will keep this in mind. I think this problem is considerably easier to solve than the different rowmark per xact in MultiXact, anyway. If we require the user to specify which unique indexes or constraints are appropriate for FKs, it becomes trivial. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] augmenting MultiXacts to improve foreign keys
Alvaro Herrera writes: > Excerpts from Jeff Davis's message of mar ago 09 14:41:14 -0400 2011: >> Right now, FKs aren't really very special, they are mostly just >> syntactic sugar (right?). This proposal would make FKs special internal >> mechanisms, and I don't see the benefit in doing so. > Well, you can get the same behavior by adding the constraint triggers > manually. But those triggers are written in C, so you could equally > claim that they are "special internal" already. The SPI interface has > some special entry points to allow them to work correctly (for example > passing a snapshot for the checks to run with). Yeah, the crosscheck-snapshot logic already puts the lie to any idea that the RI triggers are equivalent to anything available at the SQL level. ISTM you could pass down the "please do this with FOR KEY UPDATE not just FOR SHARE" flag similarly to the way the crosscheck snapshot is passed down, or maybe even just do it if you see a crosscheck snapshot is present in the executor state (though that's a bit ugly). Like Florian, I'm considerably more concerned about the aspect of deciding which columns are "key columns" and whether they changed. 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] Ignore lost+found when checking if a directory is empty
Excerpts from Jeff Davis's message of mar ago 09 16:03:26 -0400 2011: > On Tue, 2011-08-09 at 14:52 -0400, Brian Pitts wrote: > > When an ext2, ext3, or ext4 filesystem is mounted directly on the > > PGDATA directory, initdb will refuse to run because it sees the > > lost+found directory that mke2fs created and assumes the PGDATA > > directory is already in use for something other than PostgreSQL. > > Attached is a patch against master which will cause a directory that > > contains only lost+found to still be treated as empty. > > > > This was previously proposed in 2001; see > > http://archives.postgresql.org/pgsql-hackers/2001-03/msg01194.php > > In the referenced discussion (10 years ago), Tom seemed OK with it and > Peter did not seem to like it much. > > I think I agree with Peter here that it's not a very good idea, and I > don't see a big upside. With tablespaces it seems to make a little bit > more sense, but I'd still lean away from that idea. What if the init script tries to start postmaster before the filesystems are mounted? ISTM requiring a subdir is a good sanity check that the system is ready to run. Not creating stuff directly on the mountpoint ensures consistency. If you don't think this is a likely problem, search for Joe Conway's report about a NFS share being unmounted for a while when postmaster was started up, a couple of years ago. Yes, it's rare. Yes, it's real. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https
Andrew Dunstan writes: > On 08/09/2011 12:22 PM, Tom Lane wrote: >> No. As I pointed out upthread, the instant somebody changes the SIGALRM >> handler to a non-Postgres-aware one, you are already at risk of failure. >> Setting it back later is just locking the barn door after the horses >> left. Institutionalizing such a non-fix globally is even worse. > So what's your suggestion? I know what you said you'd like, but it > doesn't appear at all practical to me. [ shrug... ] Installing a perl module that mucks with the signal handlers is in the "don't do that" category. A kluge such as you suggest will not get it out of that category; all it will do is add useless overhead for people who are following the rules. 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] Ignore lost+found when checking if a directory is empty
Brian Pitts writes: > When an ext2, ext3, or ext4 filesystem is mounted directly on the PGDATA > directory, initdb will refuse to run because it sees the > lost+found directory that mke2fs created and assumes the PGDATA directory is > already in use for something other than PostgreSQL. > Attached is a patch against master which will cause a directory that contains > only lost+found to still be treated as empty. This has been proposed before, and rejected before, on the grounds that you shouldn't be using a mount-point directory as a data directory anyway. Better practice is to make a postgres-owned directory just underneath the mount point. A couple of reasons for that are: 1. Mount-point directories should be owned by root, never by an unprivileged account such as postgres. IIRC there are good security reasons for this practice, though I don't recall all the details right now. 2. Keeping the data directory one level down ensures a clean failure if the disk is for some reason not mounted when Postgres starts, or goes offline later. Otherwise, particularly if you're using a start script that will automatically try an initdb, you might end up with some data files on the / volume underneath where the mount point should have been. This is sure to lead to serious problems when the disk does come back online. There's at least one horror story in our archives from someone who had an auto-initdb startup script and one day his NFS disk was a few seconds slow to mount... > This was previously proposed in 2001; see > http://archives.postgresql.org/pgsql-hackers/2001-03/msg01194.php It's been discussed more recently than that, I believe. 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] Ignore lost+found when checking if a directory is empty
On Tue, 2011-08-09 at 14:52 -0400, Brian Pitts wrote: > Attached is a patch against master which will cause a directory that > contains only lost+found to still be treated as empty. Please add this to the September commitfest at: https://commitfest.postgresql.org/ 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] Ignore lost+found when checking if a directory is empty
On Tue, Aug 9, 2011 at 1:52 PM, Brian Pitts wrote: > When an ext2, ext3, or ext4 filesystem is mounted directly on the PGDATA > directory, initdb will refuse to run because it sees the > lost+found directory that mke2fs created and assumes the PGDATA directory is > already in use for something other than PostgreSQL. > Attached is a patch against master which will cause a directory that contains > only lost+found to still be treated as empty. > > This was previously proposed in 2001; see > http://archives.postgresql.org/pgsql-hackers/2001-03/msg01194.php > I have wanted that before, and the patch is very simple... Peter had a concern about that though, still a concern? """ Initdb or the database system can do anything they want in that directory, so it's not good to save lost blocks somewhere in the middle, even if chances are low you need them. I say, create a subdirectory. """ -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitació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] Ignore lost+found when checking if a directory is empty
On Tue, 2011-08-09 at 14:52 -0400, Brian Pitts wrote: > When an ext2, ext3, or ext4 filesystem is mounted directly on the > PGDATA directory, initdb will refuse to run because it sees the > lost+found directory that mke2fs created and assumes the PGDATA > directory is already in use for something other than PostgreSQL. > Attached is a patch against master which will cause a directory that > contains only lost+found to still be treated as empty. > > This was previously proposed in 2001; see > http://archives.postgresql.org/pgsql-hackers/2001-03/msg01194.php In the referenced discussion (10 years ago), Tom seemed OK with it and Peter did not seem to like it much. I think I agree with Peter here that it's not a very good idea, and I don't see a big upside. With tablespaces it seems to make a little bit more sense, but I'd still lean away from 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] augmenting MultiXacts to improve foreign keys
Excerpts from Florian Pflug's message of mar ago 09 15:41:00 -0400 2011: > First, I'd like to see us support FKs which reference non-unqiue columns. > As long as we restrict such FK constraints to ON UPDATE/DELETE RESTRICT > (or NO ACTION), there's no conceptual difficulty in doing that. Yeah, I > know that there'll probably be a lot of push-back on the grounds of standards > compliance, but I still believe it'd be a nice feature. > > Second, there are lots of reasons for adding UNIQUE constraints to columns > beside being on the referencing side of a FK constraint. If we make the > lock strength taken by UPDATE depend on whether or not the UPDATE modified a > column included in a UNIQUE constraint, we force people to drop UNIQUE > constraints to avoid deadlocks, and thus indirectly support sloppy schema > design. > > Couldn't we simply give the user a way to specify, per column, whether or > not that column is a "KEY" column? Creating a foreign key constraint could > still implicitly mark all referenced columns as KEY columns, but columns > would no longer be "KEY" columns simply because they're part of a UNIQUE > constraint. Users would be free to add arbitrary columns to the set of > "KEY" columns by doing ALTER TABLE ALTER COLUMN SET KEY. What you propose seems reasonable to me (allegedly not an expert in keys). However, I don't see that it conflicts with my proposal in any way -- I mean, on top of my patch you could build something that changes what columns are considered "keys". This is why the "KEY SHARE" rowmark option is going to be documented as "only for internal use", because it might change depending on how we define foreign-key-referenceable fields. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Ignore lost+found when checking if a directory is empty
When an ext2, ext3, or ext4 filesystem is mounted directly on the PGDATA
directory, initdb will refuse to run because it sees the
lost+found directory that mke2fs created and assumes the PGDATA directory is
already in use for something other than PostgreSQL.
Attached is a patch against master which will cause a directory that contains
only lost+found to still be treated as empty.
This was previously proposed in 2001; see
http://archives.postgresql.org/pgsql-hackers/2001-03/msg01194.php
--
Brian Pitts
Systems Administrator | EuPathDB Bioinformatics Resource Center
706-542-1447 | b...@uga.edu | http://eupathdb.org
>From be6eaa9474b267e669fe67a70140d46f69379968 Mon Sep 17 00:00:00 2001
From: Brian Pitts
Date: Tue, 9 Aug 2011 14:12:50 -0400
Subject: [PATCH] Ignore lost+found when checking if a directory is empty
---
src/port/pgcheckdir.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
new file mode 100644
index 9453bcb..01f0a2c
*** a/src/port/pgcheckdir.c
--- b/src/port/pgcheckdir.c
*** pg_check_dir(const char *dir)
*** 42,50
while ((file = readdir(chkdir)) != NULL)
{
if (strcmp(".", file->d_name) == 0 ||
! strcmp("..", file->d_name) == 0)
{
! /* skip this and parent directory */
continue;
}
else
--- 42,51
while ((file = readdir(chkdir)) != NULL)
{
if (strcmp(".", file->d_name) == 0 ||
! strcmp("..", file->d_name) == 0 ||
! strcmp("lost+found", file->d_name) == 0)
{
! /* skip this, parent, and e2fsck directories */
continue;
}
else
--
1.7.4.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] augmenting MultiXacts to improve foreign keys
On Aug9, 2011, at 19:01 , Alvaro Herrera wrote: > Note that this means that UPDATE would always have to check whether key > columns are being modified. (I loosely talk about "key columns" here > referring to columns involving unique indexes. On tables with no unique > indexes, I think this would have to mean all columns, but I haven't > thought much about this bit.) > > To implement this, we need to augment MultiXact to store the lock type > that each comprising Xid holds on the tuple. Two bits per Xid are > needed. My thinking is that we could simply extend the "members" to add > a byte for every four members. This should be relatively simple, though > this forecloses the option of using MultiXact for some other purpose > than locking tuples. This being purely theoretical, I don't have a > problem with that. Hm, I'm still not a huge fan of having the only the choice between locking all columns, or all columns that are part of a unique index. For multiple reasons. First, I'd like to see us support FKs which reference non-unqiue columns. As long as we restrict such FK constraints to ON UPDATE/DELETE RESTRICT (or NO ACTION), there's no conceptual difficulty in doing that. Yeah, I know that there'll probably be a lot of push-back on the grounds of standards compliance, but I still believe it'd be a nice feature. Second, there are lots of reasons for adding UNIQUE constraints to columns beside being on the referencing side of a FK constraint. If we make the lock strength taken by UPDATE depend on whether or not the UPDATE modified a column included in a UNIQUE constraint, we force people to drop UNIQUE constraints to avoid deadlocks, and thus indirectly support sloppy schema design. Couldn't we simply give the user a way to specify, per column, whether or not that column is a "KEY" column? Creating a foreign key constraint could still implicitly mark all referenced columns as KEY columns, but columns would no longer be "KEY" columns simply because they're part of a UNIQUE constraint. Users would be free to add arbitrary columns to the set of "KEY" columns by doing ALTER TABLE ALTER COLUMN SET KEY. 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] augmenting MultiXacts to improve foreign keys
On Aug9, 2011, at 19:01 , Alvaro Herrera wrote: > This would also help us find a solution to the problem that an aborted > subtransaction that updates or excl-locks a tuple causes an earlier > shared lock to be forgotten. We would deal with this by marking the > Xmax with a new MultiXact that includes both the lock and the update. > This means that this MultiXact would have to be WAL-logged. I would > leave that for a later patch, but I think it's good that there's a way > to fix it. Yeah, I pondered something similar in the past, but never got around to figuring out the details. This would also allow us to modify the behaviour of tuple locks under isolation level REPEATABLE READ to throw a serialization error if the row was locked by an invisible transaction. Doing so would allow us to get rid of the strange re-checking logic that RI triggers must currently use in REPEATABLE READ mode, and would thus make it possible to implement custom RI triggers in PL/pgSQL. I'll try to find some time to check how that fits in with the new per-tuple lock levels you proposed. 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] augmenting MultiXacts to improve foreign keys
Excerpts from Jeff Davis's message of mar ago 09 14:41:14 -0400 2011:
> On Tue, 2011-08-09 at 13:01 -0400, Alvaro Herrera wrote:
> > Note that the KEY UPDATE lock would be an internal option, not exposed
> > to SQL. I think we already have enough extensions in this area. We are
> > forced to expose KEY SHARE because RI triggers get to it via SPI, but I
> > would be happy to avoid doing it if I knew how.
>
> Right now, FKs aren't really very special, they are mostly just
> syntactic sugar (right?). This proposal would make FKs special internal
> mechanisms, and I don't see the benefit in doing so.
Well, you can get the same behavior by adding the constraint triggers
manually. But those triggers are written in C, so you could equally
claim that they are "special internal" already. The SPI interface has
some special entry points to allow them to work correctly (for example
passing a snapshot for the checks to run with).
In any case, this is certainly not something I'm really interested in
doing. I don't have a problem with simply adding the new syntax to SQL
and documenting it appropriately ("this is only for internal RI use").
> [ I didn't read through the previous threads yet, so perhaps this was
> already discussed. ]
Nope.
--
Álvaro Herrera
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] augmenting MultiXacts to improve foreign keys
On Tue, 2011-08-09 at 13:01 -0400, Alvaro Herrera wrote: > Note that the KEY UPDATE lock would be an internal option, not exposed > to SQL. I think we already have enough extensions in this area. We are > forced to expose KEY SHARE because RI triggers get to it via SPI, but I > would be happy to avoid doing it if I knew how. Right now, FKs aren't really very special, they are mostly just syntactic sugar (right?). This proposal would make FKs special internal mechanisms, and I don't see the benefit in doing so. [ I didn't read through the previous threads yet, so perhaps this was already discussed. ] 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] small issue with host names in hba
When a host name is used in pg_hba.conf, then we call
pg_getnameinfo_all() to get the host name for the client's IP address,
either in postmaster.c or in hba.c, whichever happens first. But if the
IP address has no host name, the getnameinfo calls return the IP address
in text form, which is then stored into port->remote_hostname as the
"host name". This "host name" is then compared to the name in
pg_hba.conf. It cannot match, because we only do this dance if the
entry in pg_hba.conf does not parse as an IP address. So I don't think
there is a direct security problem here, but it still seems odd.
The fix would appear to be using the NI_NAMEREQD flag to getnameinfo.
We could do that in hba.c, but in postmaster.c we would have to move
some code around to maintain a string version of the IP address for
logging. But I'm a little confused by what this code is really trying
to accomplish:
if (pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
remote_host, sizeof(remote_host),
remote_port, sizeof(remote_port),
(log_hostname ? 0 : NI_NUMERICHOST) | NI_NUMERICSERV))
{
int ret = pg_getnameinfo_all(&port->raddr.addr,
port->raddr.salen,
remote_host, sizeof(remote_host),
remote_port, sizeof(remote_port),
NI_NUMERICHOST | NI_NUMERICSERV);
if (ret)
ereport(WARNING,
(errmsg_internal("pg_getnameinfo_all() failed: %s",
gai_strerror(ret;
}
If log_hostname is off, this just make the same function call twice. If
log_hostname is on (which is what we are concerned about here), it makes
the same call but with the addition of the NI_NUMERICHOST flag. But
getnameinfo already returns a numeric representation of no actual host
name is available, unless said NI_NAMEREQD is used.
So, do we need to fix the issue described in the first paragraph?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] augmenting MultiXacts to improve foreign keys
Hi, Previously, we had some discussion to introduce a new type of tuple lock to let foreign keys be lighter weight, allowing for more concurrency. During the course of that discussion, it became evident that the solution being proposed didn't go all the way to solve the problem. A proposal by Noah Misch seems like it would fix the whole problem, but it requires some more rejiggering than I had considered. This email summarizes what I just posted in a blog article here: http://www.commandprompt.com/blogs/alvaro_herrera/2011/08/fixing_foreign_key_deadlocks_part_three/ and adds some implementation details. The proposal there is to create two new tuple lock types, not one. They have been called "KEY UPDATE" and "KEY SHARE". Proposed conflict table: KEY UPDATEFOR UPDATE FOR SHARE KEY SHARE KEY UPDATE X XX X FOR UPDATE X XX FOR SHAREX X KEY SHAREX DELETE always grabs KEY UPDATE lock on a tuple. UPDATE grabs KEY UPDATE if the key is being modified, otherwise FOR UPDATE. SELECT FOR UPDATE grabs FOR UPDATE. SELECT FOR SHARE grabs FOR SHARE. SELECT FOR KEY SHARE grabs FOR KEY SHARE. This is the mode used by RI triggers. Note that this means that UPDATE would always have to check whether key columns are being modified. (I loosely talk about "key columns" here referring to columns involving unique indexes. On tables with no unique indexes, I think this would have to mean all columns, but I haven't thought much about this bit.) Note that the KEY UPDATE lock would be an internal option, not exposed to SQL. I think we already have enough extensions in this area. We are forced to expose KEY SHARE because RI triggers get to it via SPI, but I would be happy to avoid doing it if I knew how. I would also love to get rid of FOR SHARE because I don't think they serve any purpose anymore, but since it has already been exposed to SQL for several releases, I'm not sure that it's a viable thing to do. To implement this, we need to augment MultiXact to store the lock type that each comprising Xid holds on the tuple. Two bits per Xid are needed. My thinking is that we could simply extend the "members" to add a byte for every four members. This should be relatively simple, though this forecloses the option of using MultiXact for some other purpose than locking tuples. This being purely theoretical, I don't have a problem with that. Note that the original keylocks patch I posted a couple of weeks ago has a caveat: if transaction A grabs share lock and transaction B grabs key lock, there's no way to know who owns which. I dismissed that problem as unimportant (and probably infrequent); the good thing about this new idea is that we wouldn't have that problem. This would also help us find a solution to the problem that an aborted subtransaction that updates or excl-locks a tuple causes an earlier shared lock to be forgotten. We would deal with this by marking the Xmax with a new MultiXact that includes both the lock and the update. This means that this MultiXact would have to be WAL-logged. I would leave that for a later patch, but I think it's good that there's a way to fix it. Thoughts? -- Álvaro Herrera -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https
On 08/09/2011 12:22 PM, Tom Lane wrote: Andrew Dunstan writes: On 08/08/2011 05:03 AM, Tim Bunce wrote: After giving it some more thought it seems reasonable to simply force the SIGALRM handler back to postgres when a plperlu function returns: pqsignal(SIGALRM, handle_sig_alarm); Maybe we need to do this in some more centralized spot. It seems unlikely that this problem is unique to plperlu, or even just confined to PLs. No. As I pointed out upthread, the instant somebody changes the SIGALRM handler to a non-Postgres-aware one, you are already at risk of failure. Setting it back later is just locking the barn door after the horses left. Institutionalizing such a non-fix globally is even worse. So what's your suggestion? I know what you said you'd like, but it doesn't appear at all practical to me. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https
Andrew Dunstan writes: > On 08/08/2011 05:03 AM, Tim Bunce wrote: >> After giving it some more thought it seems reasonable to simply force the >> SIGALRM handler back to postgres when a plperlu function returns: >> pqsignal(SIGALRM, handle_sig_alarm); > Maybe we need to do this in some more centralized spot. It seems > unlikely that this problem is unique to plperlu, or even just confined > to PLs. No. As I pointed out upthread, the instant somebody changes the SIGALRM handler to a non-Postgres-aware one, you are already at risk of failure. Setting it back later is just locking the barn door after the horses left. Institutionalizing such a non-fix globally is even worse. 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] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)
Heikki Linnakangas writes: > On 09.08.2011 18:04, Alvaro Herrera wrote: >> I think I vaguely remember that the reason for doing it this way is that >> the copy into the relcache worked, i.e. if the originals went away, >> there was no dangling pointer. Did you test this? > These strings are never freed, so I don't think it's possible to end up > with a dangling pointer. Well, in that case the relevant question is whether we need to worry about memory leaks due to multiple copies. Personally I'm wondering why the function is strdup'ing the default value at all. In practice, wouldn't it practically always be a compile time constant string? Why create possible issues by designing the code for a different use-case? In particular, why not declare the default value as "const char *" and specify that it's caller's responsibility that it live forever if it's not just a constant string? 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] Enforcing that all WAL has been replayed after restoring from backup
Heikki Linnakangas writes: > On 09.08.2011 18:20, Alvaro Herrera wrote: >> How about making the new backup_label field optional? If absent, assume >> current behavior. > That's how I actually did it in the patch. However, the problem wrt. > requiring initdb is not the new field in backup_label, it's the new > field in the control file. Yeah. I think it's too late to be fooling with pg_control for 9.1. Just fix it in HEAD. 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] plperl crash with Debian 6 (64 bit), pl/perlu, libwww and https
On 08/08/2011 05:03 AM, Tim Bunce wrote: After giving it some more thought it seems reasonable to simply force the SIGALRM handler back to postgres when a plperlu function returns: pqsignal(SIGALRM, handle_sig_alarm); Maybe we need to do this in some more centralized spot. It seems unlikely that this problem is unique to plperlu, or even just confined to PLs. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)
On 09.08.2011 18:04, Alvaro Herrera wrote: Excerpts from Heikki Linnakangas's message of mar ago 09 08:32:43 -0400 2011: On 09.08.2011 13:25, Heikki Linnakangas wrote: On 08.08.2011 22:11, Alvaro Herrera wrote: Perhaps the easiest way to fix it is as you suggest, by declaring the struct to take a pointer rather than the value directly. Not sure how to make both cases work sanely; the add_string_reloption path will need updates. Agreed, I propose the attached patch to do that. Committed this. Thanks. I think I vaguely remember that the reason for doing it this way is that the copy into the relcache worked, i.e. if the originals went away, there was no dangling pointer. Did you test this? These strings are never freed, so I don't think it's possible to end up with a dangling pointer. -- 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] Enforcing that all WAL has been replayed after restoring from backup
On 09.08.2011 18:20, Alvaro Herrera wrote: Excerpts from Heikki Linnakangas's message of mar ago 09 05:00:00 -0400 2011: I think this is a nice additional safeguard to have, making streamed backups more robust. I'd like to add this to 9.1, but it required an extra field to be added to the control file, so it would force an initdb. It's probably not worth that. Or, we could sneak in the extra boolean field to some currently unused pad space in the ControlFile struct. How about making the new backup_label field optional? If absent, assume current behavior. That's how I actually did it in the patch. However, the problem wrt. requiring initdb is not the new field in backup_label, it's the new field in the control file. -- 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] Enforcing that all WAL has been replayed after restoring from backup
Excerpts from Heikki Linnakangas's message of mar ago 09 05:00:00 -0400 2011: > I think this is a nice additional safeguard to have, making streamed > backups more robust. I'd like to add this to 9.1, but it required an > extra field to be added to the control file, so it would force an > initdb. It's probably not worth that. Or, we could sneak in the extra > boolean field to some currently unused pad space in the ControlFile struct. How about making the new backup_label field optional? If absent, assume current behavior. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)
Excerpts from Heikki Linnakangas's message of mar ago 09 08:32:43 -0400 2011: > On 09.08.2011 13:25, Heikki Linnakangas wrote: > > On 08.08.2011 22:11, Alvaro Herrera wrote: > >> Perhaps the easiest way to fix it is as you suggest, by declaring the > >> struct to take a pointer rather than the value directly. Not sure how > >> to make both cases work sanely; the add_string_reloption path will need > >> updates. > > > > Agreed, I propose the attached patch to do that. > > Committed this. Thanks. I think I vaguely remember that the reason for doing it this way is that the copy into the relcache worked, i.e. if the originals went away, there was no dangling pointer. Did you test this? -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP fix proposal for bug #6123
Florian Pflug wrote: > Another option would be to re-issue the DELETE from the BEFORE > DELETE trigger, and then return NULL. It'll cause the BEFORE > DELETE trigger to be invoked recursively, but presumably the > second invocation could easily detect that all required pre-delete > actions have already taken place and exit early (and return OLD). > In pseudo-code, something like > > BEFORE DELETE ON : > DELETE FROM WHERE parent_id = OLD.id; > IF FOUND THEN > -- Removing children might have modified our row, > -- so returning non-NULL is not an option > DELETE FROM WHERE id = OLD.id; > RETURN NULL; > ELSE > -- No children removed, so our row should be unmodified > RETURN OLD; > END IF; Yeah, that would cover it all right. That pretty much eliminates my objections to your "check for error after firing each BEFORE trigger" approach. -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] WIP fix proposal for bug #6123
On Aug9, 2011, at 15:41 , Kevin Grittner wrote: > Florian Pflug wrote: > >> To summarize, here's what I currently believe would be a sensible >> approach: >> >> After every BEFORE trigger invocation, if the trigger returned >> non-NULL, check if latest row version is still the same as when >> the trigger started. If not, complain. > > That certainly has the advantages of being a small, safe change and > of being easy to explain. It would certainly prevent the > astonishing sorts of behaviors which now occur and which can leave > people with database contents they thought they had guards against. > > The down side is that something this strict does make it hard to > achieve certain behaviors which could be desirable for maintaining > redundant data for performance. In my bottom-up delete scenario, > there would either need to be somewhere to note that a row was being > deleted so that the delete of the children could skip maintaining > it, or the cascade would need to be implemented in the AFTER > triggers, and validations would need to accept orphans which could > be created. Either approach can be made to work, but from the > application development side, it's not as clean or easy. Another option would be to re-issue the DELETE from the BEFORE DELETE trigger, and then return NULL. It'll cause the BEFORE DELETE trigger to be invoked recursively, but presumably the second invocation could easily detect that all required pre-delete actions have already taken place and exit early (and return OLD). In pseudo-code, something like BEFORE DELETE ON : DELETE FROM WHERE parent_id = OLD.id; IF FOUND THEN -- Removing children might have modified our row, -- so returning non-NULL is not an option DELETE FROM WHERE id = OLD.id; RETURN NULL; ELSE -- No children removed, so our row should be unmodified RETURN OLD; END IF; Or, more generally, you could check for modifications of the row to be deleted. I'm not sure if the ctid column is currectly available for OLD and NEW, but if it is, you could do BEFORE DELETE ON : IF EXISTS(SELECT 1 FROM WHERE ctid = OLD.ctid) -- Original row wasn't unmodified, actual delete should succeed RETURN OLD; ELSE -- Original was modified, actual delete would fail DELETE FROM WHERE pk = OLD.pk; RETURN NULL; END IF; This only requires that the "arbitrarily complex child removal logic" is smart enough not to update the parent table if no children were actually removed. > The suggested approach for UPDATE with my original approach to > DELETE would make me happier, but I'm still not clear on Robert's > use cases and how that would affect him. Can you clarify why you > feel UPDATE and DELETE should both do this? I'm concerned that proceeding with a DELETE even though the row has been modified has an equal chance of cause subtle data-integrity violations as the current behaviour. We might remove a row, even though the row, at the time of deletion, does *not* match the DELETE's WHERE condition. That seems like a violation of very basic guarantees to me. A real-world use-case where that might happen could consist of a table with a "reference_count" column, together with a BEFORE DELETE trigger which (indirectly via other triggers) sometimes causes a reference_count to be changed from 0 to >0. Simply inserting a new row instead of re-using the old one may not be an option because of UNIQUE constraints. The statement DELETE FROM reference_counted_table WHERE reference_count = 0 might then delete rows even though they *are* referenced, if the references where added from the BEFORE DELETE trigger. The result would probably dangling references, i.e. a data-integrity violation. One might attempt to solve that by re-checking the DELETE's WHERE condition before carrying out the DELETE. To avoid ending up where we started, i.e. with BEFORE triggers firing but no DELETE taking place, we'd have to error out if the re-check fails instead of silently ignoring the DELETE. At which point whether or not a DELETE of a single record succeeds or fails depends on the WHERE condition used to *find* that record. That, I feel, isn't a road we want to take... 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] Problem with sources.
On 08/09/2011 09:22 AM, Andrew Dunstan wrote: On 08/09/2011 09:10 AM, pasman pasmański wrote: When i try to fetch sources via git, there is an error: error: Unable to find 676e3c735dec32ba0448fdd0faf9ace37c1fa792 under http://git.postgresql.org/git/postgresql.git Cannot obtain needed object 676e3c735dec32ba0448fdd0faf9ace37c1fa792 error: Fetch failed. How to get current branch ? I always use the git protocol, which is working fine: [andrew@ophelia ~]$ git clone git://git.postgresql.org/git/postgresql.git ntestpg Cloning into ntestpg... remote: Counting objects: 418000, done. remote: Compressing objects: 100% (78367/78367), done. remote: Total 418000 (delta 348625), reused 406551 (delta 337799) Receiving objects: 100% (418000/418000), 119.93 MiB | 1.13 MiB/s, done. Resolving deltas: 100% (348625/348625), done. I just tested cloning over http, and it worked fine for me, although it took a while. You can also pull from the clone at github, which is kept pretty up to date, and probably has better bandwidth, using git://github.com/postgres/postgres.git or https://github.com/postgres/postgres.git cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Selecting user-defined CASTs
On 08/09/2011 01:27 AM, Tom Lane wrote: > Another approach is to check pg_depend. A cast installed by initdb will > match a "pin" entry in pg_depend (refclassid = pg_cast, refobjid = > cast's OID, deptype = 'p'). You're still out of luck for distinguishing > extension members in existing releases, but in 9.1 and up it'll be > possible to identify casts belonging to extensions from pg_depend > entries. Intriguing alternative. The query is for my dbtoyaml utility which is sort of equivalent to pg_dump -s --format=y (yaml). So for now, I'll stick to emulating what dumpCast in pg_dump.c does. Joe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP fix proposal for bug #6123
Florian Pflug wrote: > To summarize, here's what I currently believe would be a sensible > approach: > > After every BEFORE trigger invocation, if the trigger returned > non-NULL, check if latest row version is still the same as when > the trigger started. If not, complain. That certainly has the advantages of being a small, safe change and of being easy to explain. It would certainly prevent the astonishing sorts of behaviors which now occur and which can leave people with database contents they thought they had guards against. The down side is that something this strict does make it hard to achieve certain behaviors which could be desirable for maintaining redundant data for performance. In my bottom-up delete scenario, there would either need to be somewhere to note that a row was being deleted so that the delete of the children could skip maintaining it, or the cascade would need to be implemented in the AFTER triggers, and validations would need to accept orphans which could be created. Either approach can be made to work, but from the application development side, it's not as clean or easy. The suggested approach for UPDATE with my original approach to DELETE would make me happier, but I'm still not clear on Robert's use cases and how that would affect him. Can you clarify why you feel UPDATE and DELETE should both do this? -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] WIP fix proposal for bug #6123
Florian Pflug wrote: > I think it would be helpful if we had a more precise idea about > the intended use-cases. So far, the only use-case that has been > described in detail is the one which made Kevin aware of the > problem. But if I understood Kevin correctly, that fact that they > use BEFORE and not AFTER triggers it more of an accident than the > result of a conscious design decision. For the UPDATE case we have so far only identified one place where this was a problem -- although the grep we used would only show the simplest form of this (where the UPDATE can be replaced by setting fields in the NEW record). We've scheduled people to run through the system test scripts again with my quick fix in place so that we get an error rather than silent corruption (which might easily have been missed the last time), so others could still turn up. In any event, I have neither seen nor imagined any use cases in our shop where we need to allow the UPDATE behavior. > Though he did also mention that there might actually be advantages > to using BEFORE instead of AFTER triggers, because that prevents > other triggers from seeing a non-consistent state. Right, although I've only seen that for the DELETE triggers. We are using the BEFORE triggers to delete from the bottom up, and doing this in the AFTER trigger would expose states to triggers where children still exist in the absence of parents, which might fire validation failures from hard-to-predict places. It would certainly be more convenient on this end if the DELETE behavior from my patch was accepted, but I'm confident that there are workarounds for the long term if not. > What I can add is that AFAIR all instances of same-row UPDATES > from BEFORE triggers I ever encountered where replaceable by a > simple assignment to NEW. (That, however, is more of an > anti-usecase) If there is a valid use-case for UPDATE, it would have to be less direct -- for example, the BEFORE UPDATE trigger modifies some other table or row, and the trigger for *that* updates the original row. I really can't see any excuse for a BEFORE UPDATE ... FOR EACH ROW trigger to execute a new UPDATE statement referencing the row for which it is being fired. -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] Problem with sources.
On 08/09/2011 09:10 AM, pasman pasmański wrote: When i try to fetch sources via git, there is an error: error: Unable to find 676e3c735dec32ba0448fdd0faf9ace37c1fa792 under http://git.postgresql.org/git/postgresql.git Cannot obtain needed object 676e3c735dec32ba0448fdd0faf9ace37c1fa792 error: Fetch failed. How to get current branch ? I always use the git protocol, which is working fine: [andrew@ophelia ~]$ git clone git://git.postgresql.org/git/postgresql.git ntestpg Cloning into ntestpg... remote: Counting objects: 418000, done. remote: Compressing objects: 100% (78367/78367), done. remote: Total 418000 (delta 348625), reused 406551 (delta 337799) Receiving objects: 100% (418000/418000), 119.93 MiB | 1.13 MiB/s, done. Resolving deltas: 100% (348625/348625), done. 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
[HACKERS] Problem with sources.
When i try to fetch sources via git, there is an error: error: Unable to find 676e3c735dec32ba0448fdd0faf9ace37c1fa792 under http://git.postgresql.org/git/postgresql.git Cannot obtain needed object 676e3c735dec32ba0448fdd0faf9ace37c1fa792 error: Fetch failed. How to get current branch ? pasman -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)
On 09.08.2011 13:25, Heikki Linnakangas wrote: On 08.08.2011 22:11, Alvaro Herrera wrote: Perhaps the easiest way to fix it is as you suggest, by declaring the struct to take a pointer rather than the value directly. Not sure how to make both cases work sanely; the add_string_reloption path will need updates. Agreed, I propose the attached patch to do that. Committed this. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] some missing internationalization in pg_basebackup
I noticed that the progress reporting code in pg_basebackup does not
allow for translation. This would normally be easy to fix, but this
code has a number of tricky issues, including the INT64_FORMAT, possibly
some plural concerns, and some space alignment issues that hidden in
some of those hardcoded numbers.
I'm just posting it here as an open item in case someone has some ideas
in the meantime.
static void
progress_report(int tablespacenum, char *fn)
{
int percent = (int) ((totaldone / 1024) * 100 / totalsize);
if (percent > 100)
percent = 100;
if (verbose)
{
if (!fn)
/*
* No filename given, so clear the status line (used for last
* call)
*/
fprintf(stderr,
INT64_FORMAT "/" INT64_FORMAT " kB (100%%) %d/%d
tablespaces %35s\r",
totaldone / 1024, totalsize,
tablespacenum, tablespacecount, "");
else
fprintf(stderr,
INT64_FORMAT "/" INT64_FORMAT " kB (%d%%) %d/%d tablespaces
(%-30.30s)\r",
totaldone / 1024, totalsize,
percent,
tablespacenum, tablespacecount, fn);
}
else
fprintf(stderr, INT64_FORMAT "/" INT64_FORMAT " kB (%d%%) %d/%d
tablespaces\r",
totaldone / 1024, totalsize,
percent,
tablespacenum, tablespacecount);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] per-column FDW options, v5
(2011/08/09 1:16), Robert Haas wrote:
> 2011/8/8 Shigeru Hanada:
Currently table-level options are showin in result of \det+ command
(only verbose mode), in same style as fdw and foreign servers.
But \d is more popular for table describing, so moving table-level
options from \det+ to \d might be better. Thoughts?
>>
>> (2011/08/06 9:26), Robert Haas wrote:
>>> I'd show it both places.
>>
>> After taking a look at describe.c, I think some styles are applicable to
>> FDW options in result of \d command.
>>
>> (1) simple array literal style
>> Show table-level FDW options like other FDW options. It is simply a
>> result of array_out(); each options is shown as "key=value" with quoting
>> if necessary and delimited by ','. Whole line is surrounded by { and }.
>> If an element includes any character which need to be escaped, such
>> element is quoted with double-quotation.
>>
>> Ex)
>> FDW Options: {"delimiter=,","quote=\""}
>> #delimiter is a comma, and qutoe is a double-quote
>>
>> (2) reloptions style
>> Show FDW options like reloptions of \d+ result. Each options is shown
>> as "key=value" without quoting. Some special characters might make it
>> little illegible.
>>
>> Ex)
>> FDW Options: delimiter=,, quote="
>> #delimiter is a comma, and qutoe is a double-quote
>>
>> (3) OPTIONS clause style
>> Show FDW options as they were in OPTIONS clause. Each option is shown
>> as "key 'value'", and delimited with ','.
>>
>> Ex)
>> FDW Options: delimiter ',', quote
>> #delimiter is a comma, and qutoe is a single-quote
>>
>> (1) can be implemented with minimum change, and it also keeps the
>> behavior consistent with other existing FDW objects. But IMHO (3) is
>> most readable, though it breaks backward compatibility about the format
>> of FDW options used in the result of \d* command. Thoughts?
>
> I'm against #2, but I could go either way on #1 vs. #3. If you pick
> #3, would you also change the column options to be displayed that way,
> or would we end up with table and column options displayed
> differently?
I'd like to pick #3, and also change per-column options format. In
addition, I'd like to change options format for other FDW objects such
as wrappers, servers and user mappings for consistency. Of course, only
if it's acceptable to break backward compatibility...
Regards,
--
Shigeru Hanada
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Compiler warnings with stringRelOpts (was WIP: Fast GiST index build)
On 08.08.2011 22:11, Alvaro Herrera wrote:
Perhaps the easiest way to fix it is as you suggest, by declaring the
struct to take a pointer rather than the value directly. Not sure how
to make both cases work sanely; the add_string_reloption path will need
updates.
Agreed, I propose the attached patch to do that. Are there any
extensions out there that use add_string_relopt(), that I could use for
testing?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 4657425..900b222 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -371,8 +371,6 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
size_t size;
relopt_gen *newoption;
- Assert(type != RELOPT_TYPE_STRING);
-
oldcxt = MemoryContextSwitchTo(TopMemoryContext);
switch (type)
@@ -386,6 +384,9 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
case RELOPT_TYPE_REAL:
size = sizeof(relopt_real);
break;
+ case RELOPT_TYPE_STRING:
+ size = sizeof(relopt_string);
+ break;
default:
elog(ERROR, "unsupported option type");
return NULL; /* keep compiler quiet */
@@ -474,45 +475,29 @@ void
add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
validate_string_relopt validator)
{
- MemoryContext oldcxt;
relopt_string *newoption;
- int default_len = 0;
-
- oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-
- if (default_val)
- default_len = strlen(default_val);
- newoption = palloc0(sizeof(relopt_string) + default_len);
+ /* make sure the validator/default combination is sane */
+ if (validator)
+ (validator) (default_val);
- newoption->gen.name = pstrdup(name);
- if (desc)
- newoption->gen.desc = pstrdup(desc);
- else
- newoption->gen.desc = NULL;
- newoption->gen.kinds = kinds;
- newoption->gen.namelen = strlen(name);
- newoption->gen.type = RELOPT_TYPE_STRING;
+ newoption = (relopt_string *) allocate_reloption(kinds, RELOPT_TYPE_STRING,
+ name, desc);
newoption->validate_cb = validator;
if (default_val)
{
- strcpy(newoption->default_val, default_val);
- newoption->default_len = default_len;
+ newoption->default_val = MemoryContextStrdup(TopMemoryContext,
+ default_val);
+ newoption->default_len = strlen(default_val);
newoption->default_isnull = false;
}
else
{
- newoption->default_val[0] = '\0';
+ newoption->default_val = "";
newoption->default_len = 0;
newoption->default_isnull = true;
}
- /* make sure the validator/default combination is sane */
- if (newoption->validate_cb)
- (newoption->validate_cb) (newoption->default_val);
-
- MemoryContextSwitchTo(oldcxt);
-
add_reloption((relopt_gen *) newoption);
}
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index c7709cc..14f5034 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -108,7 +108,7 @@ typedef struct relopt_string
int default_len;
bool default_isnull;
validate_string_relopt validate_cb;
- char default_val[1]; /* variable length, zero-terminated */
+ char *default_val;
} relopt_string;
/* This is the table datatype for fillRelOptions */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Enforcing that all WAL has been replayed after restoring from backup
Currently, if you take a backup with "pg_basebackup -x" (which means it
will include all the WAL to required restore within the backup dump),
and hit Ctrl-C while the WAL is being streamed, you end up with a data
directory that you can start postmaster from, even though the backup was
not complete. So what appears to be a valid backup - it starts up fine -
can actually be corrupt.
I put in a check against that back in March, but it had to be reverted
because it broke crash recovery when the system crashed while a
pg_start_backup() based backup was in progress:
http://archives.postgresql.org/message-id/4da58686.1050...@enterprisedb.com
Here's a patch to add it back in a more fine-grained fashion. The patch
adds an extra line to backup_label, indicating whether the backup was
taken with pg_start/stop_backup(), or by streaming (= pg_basebackup).
For a backup taken with pg_start_backup(), the behavior is kept the same
as it has been - if the end-of-backup record is not reached during crash
recovery, the database starts up anyway. But for a streamed backup, you
get an error at startup.
I think this is a nice additional safeguard to have, making streamed
backups more robust. I'd like to add this to 9.1, but it required an
extra field to be added to the control file, so it would force an
initdb. It's probably not worth that. Or, we could sneak in the extra
boolean field to some currently unused pad space in the ControlFile struct.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6a6959f..d057e66 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -662,7 +662,8 @@ static bool CheckForStandbyTrigger(void);
static void xlog_outrec(StringInfo buf, XLogRecord *record);
#endif
static void pg_start_backup_callback(int code, Datum arg);
-static bool read_backup_label(XLogRecPtr *checkPointLoc);
+static bool read_backup_label(XLogRecPtr *checkPointLoc,
+ bool *backupEndRequired);
static void rm_redo_error_callback(void *arg);
static int get_sync_bit(int method);
@@ -6016,6 +6017,7 @@ StartupXLOG(void)
XLogRecord *record;
uint32 freespace;
TransactionId oldestActiveXID;
+ bool backupEndRequired = false;
/*
* Read control file and check XLOG status looks valid.
@@ -6149,7 +6151,7 @@ StartupXLOG(void)
if (StandbyMode)
OwnLatch(&XLogCtl->recoveryWakeupLatch);
- if (read_backup_label(&checkPointLoc))
+ if (read_backup_label(&checkPointLoc, &backupEndRequired))
{
/*
* When a backup_label file is present, we want to roll forward from
@@ -6328,7 +6330,10 @@ StartupXLOG(void)
* set backupStartPoint if we're starting recovery from a base backup
*/
if (haveBackupLabel)
+ {
ControlFile->backupStartPoint = checkPoint.redo;
+ ControlFile->backupEndRequired = backupEndRequired;
+ }
ControlFile->time = (pg_time_t) time(NULL);
/* No need to hold ControlFileLock yet, we aren't up far enough */
UpdateControlFile();
@@ -6698,9 +6703,13 @@ StartupXLOG(void)
* crashes while an online backup is in progress. We must not treat
* that as an error, or the database will refuse to start up.
*/
- if (InArchiveRecovery)
+ if (InArchiveRecovery || ControlFile->backupEndRequired)
{
- if (!XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
+ if (ControlFile->backupEndRequired)
+ereport(FATAL,
+ (errmsg("WAL ends before end of online backup"),
+ errhint("All WAL generated while online backup was taken must be available at recovery.")));
+ else if (!XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
ereport(FATAL,
(errmsg("WAL ends before end of online backup"),
errhint("Online backup started with pg_start_backup() must be ended with pg_stop_backup(), and all WAL up to that point must be available at recovery.")));
@@ -8531,6 +8540,7 @@ xlog_redo(XLogRecPtr lsn, XLogRecord *record)
if (XLByteLT(ControlFile->minRecoveryPoint, lsn))
ControlFile->minRecoveryPoint = lsn;
MemSet(&ControlFile->backupStartPoint, 0, sizeof(XLogRecPtr));
+ ControlFile->backupEndRequired = false;
UpdateControlFile();
LWLockRelease(ControlFileLock);
@@ -9013,6 +9023,8 @@ do_pg_start_backup(const char *backupidstr, bool fast, char **labelfile)
startpoint.xlogid, startpoint.xrecoff, xlogfilename);
appendStringInfo(&labelfbuf, "CHECKPOINT LOCATION: %X/%X\n",
checkpointloc.xlogid, checkpointloc.xrecoff);
+ appendStringInfo(&labelfbuf, "BACKUP METHOD: %s\n",
+ exclusive ? "pg_start_backup" : "streamed");
appendStringInfo(&labelfbuf, "START TIME: %s\n", strfbuf);
appendStringInfo(&labelfbuf, "LABEL: %s\n", backupidstr);
@@ -9768,15 +9780,19 @@ pg_xlogfile_name(PG_FUNCTION_ARGS)
*
* Returns TRUE if a backup_label was found (and fills the checkpoint
* location and its REDO location into *checkPointLoc and RedoStartLSN,
- * respect
Re: [HACKERS] libedit memory stomp is apparently fixed in OS X Lion
* David Fetter: > What is it about a GPLed psql client that is a non-starter for the > installer? I'm not a fan of the GPL, but in this case, the effects of > linking it in are quite limited in scope, i.e. they pertain to exactly > one binary. And the usual showstopper, OpenSSL, might well fall under the system library exception on MacOS X. Curiously, the GPL favors proprietary operating systems in this area. -- Florian Weimer BFK edv-consulting GmbH http://www.bfk.de/ Kriegsstraße 100 tel: +49-721-96201-1 D-76133 Karlsruhe fax: +49-721-96201-99 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Selecting user-defined CASTs
On 09.08.2011 08:27, Tom Lane wrote: select ... from pg_cast c where c.oid>= 16384; What that really does is eliminate the casts that were installed during initdb, which are at least a subset of the "system" ones, and might be all of them depending on what you feel a "system" cast is. The main shortcoming of it is that there's no very good way to eliminate casts installed by extensions, should you want to not consider those "user" casts. That will also exclude "system" casts that have been removed, and manually re-created later. -- 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
