Re: [HACKERS] tsearch Parser Hacking
On Mon, 14 Feb 2011, David E. Wheeler wrote: On Feb 14, 2011, at 11:37 PM, Oleg Bartunov wrote: it's not easy to hack tsearch parser, sorry. You can preparse your input before to_tsquery,to_tsvector. Yeah, I was thinking about s{/}{-}g before passing the values in. Might be the only way to do it for now? actually, it's not so difficult to *hack* parser to treat '/' as '-'. I thought about overriding some default parser behaviour, but didn't come to any useful solution. btw, some users already wrote their own parsers and even I have little tutorial: http://www.sai.msu.su/~megera/postgres/gist/tsearch/V2/docs/HOWTO-parser-tsearch2.html I wonder if it's worth to add it to http://www.postgresql.org/docs/8.4/static/test-parser.html Probably, good paper/presentation along with improving code docs would be enough for now, until someone got very bright idea about parser and time to implement it. Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- 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] tsearch Parser Hacking
On Mon, 14 Feb 2011, Tom Lane wrote: "David E. Wheeler" writes: Is it possible to modify the default tsearch parser so that / doesn't get lexed as a "file" token? There is zero, none, nada, provision for modifying the behavior of the default parser, other than by changing its compiled-in state transition tables. It doesn't help any that said tables are baroquely designed and utterly undocumented. what do you mean 'baroquely' ? Do you know 'gothic' design :? IMO, sooner or later we need to trash that code and replace it with something a bit more modification-friendly. We thought about configurable parser, but AFAIR, we didn't get any support for this at that time. Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- 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] tsearch Parser Hacking
On Feb 14, 2011, at 11:37 PM, Oleg Bartunov wrote: > it's not easy to hack tsearch parser, sorry. You can preparse your input > before to_tsquery,to_tsvector. Yeah, I was thinking about s{/}{-}g before passing the values in. Might be the only way to do it for now… Thanks, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tsearch Parser Hacking
David, it's not easy to hack tsearch parser, sorry. You can preparse your input before to_tsquery,to_tsvector. Oleg On Mon, 14 Feb 2011, David E. Wheeler wrote: Hackers, Is it possible to modify the default tsearch parser so that / doesn't get lexed as a "file" token? That is, instead of this: try=# select * from ts_debug('simple'::regconfig, 'w/d'); alias │description│ token │ dictionaries │ dictionary │ lexemes ───┼───┼───┼──┼┼─ file │ File or path name │ w/d │ {simple} │ simple │ {w/d} Ideally it'd think that / was the same as -: try=# select * from ts_debug('simple'::regconfig, 'w-d'); alias │ description │ token │ dictionaries │ dictionary │ lexemes ─┼─┼───┼──┼┼─ asciihword │ Hyphenated word, all ASCII │ w-d │ {simple} │ simple │ {w-d} hword_asciipart │ Hyphenated word part, all ASCII │ w │ {simple} │ simple │ {w} blank │ Space symbols │ - │ {} │ [null] │ [null] hword_asciipart │ Hyphenated word part, all ASCII │ d │ {simple} │ simple │ {d} (4 rows) Possible? Or would I have to write a completely new parser just to change this bit? Thanks, David Regards, Oleg _ Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru), Sternberg Astronomical Institute, Moscow University, Russia Internet: o...@sai.msu.su, http://www.sai.msu.su/~megera/ phone: +007(495)939-16-83, +007(495)939-23-83 -- 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] Sync Rep for 2011CF1
On Sat, Jan 15, 2011 at 4:40 PM, Simon Riggs wrote: > > Here's the latest patch for sync rep. > I was looking at this code and found something in SyncRepWaitOnQueue we declare a timeout variable that is a long and another that is a boolean (this last one in the else part of the "if (!IsOnSyncRepQueue())"), and then use the boolean one as if it were the long one + else + { + bool release = false; + bool timeout = false; + + SpinLockAcquire(&queue->qlock); + + /* +* Check the LSN on our queue and if its moved far enough then +* remove us from the queue. First time through this is +* unlikely to be far enough, yet is possible. Next time we are +* woken we should be more lucky. +*/ + if (XLByteLE(XactCommitLSN, queue->lsn)) + release = true; + else if (timeout > 0 && + TimestampDifferenceExceeds(GetCurrentTransactionStopTimestamp(), + now, + timeout)) + { + release = true; + timeout = true; + } the other two things are on postgresql.conf.sample: - we have two replication_timeout_client, obviously one of them should be replication_timeout_server - synchronous_replication_feedback is off by default, but docs says otherwise i also have been testing this, until now the only issue i have found is that if i set allow_standalone_primary to off and there isn't a standby connected i need to stop the server with -m immediate which is at least surprising -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte y capacitación de PostgreSQL -- 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] Debian readline/libedit breakage
On 02/14/2011 02:26 PM, Marko Kreen wrote: On Mon, Feb 14, 2011 at 3:08 PM, Martin Pitt wrote: thanks Markus for CC'ing me, I'm not on -hackers@. Markus Wanner [2011-02-14 13:37 +0100]: On 02/10/2011 11:34 PM, Joshua D. Drake wrote: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=607109 Note that the recent discussions happened on bug 608442, in particular http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=608442#30 and the following comments. Personally, I'm a bit suspicious about that solution (technically as well as from a licensing perspective), [...] For the record, so am I (see comment 30 in the link above), as it uses the very same ld.so in both cases. However, Andreas Barth pointed out that with LD_PRELOAD it's guaranteed that we do not "import" any code from the libreadline header files, which guarantees that psql doesn't become something that can be considered a "derived work". Technically, this is a bit fragile, of course, as there might be some subtle ABI differences which lead to crashes. However, the preloading workaround already makes the situation so much better than before, so IMHO it's better than the previous status quo. I don't really like this situation, and personally I'd rather move back to libreadline until OpenSSL or readline or PostgreSQL threatens Debian with a legal case for license violation (I daresay that the chances of this happening are very close to zero..). But oh well.. I think it would be better to revert to readline and make note that conversion depends on libedit's readiness for unicode. I doubt anybody in Debian is that gung-ho to veto current state... Informing libedit about relevant problem would be good too. I don't see any bugs about that in Debian's bugtracker, did you send them to upstream? from what I can see upstream libedit actually has utf8 support for a while now (as well as some other fixes) but the debian libedit version (and also the one of other distributions) is way too old for that so maybe most of the issues would be mood if debian updated to a newer libedit version... Stefan -- 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] Sync Rep for 2011CF1
On Mon, Feb 14, 2011 at 2:08 PM, Fujii Masao wrote: > On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas > wrote: >> I committed the patch with those changes, and some minor comment tweaks and >> other kibitzing. I have another comment: The description of wal_receiver_status_interval is in "18.5.4. Streaming Replication". But I think that it should be moved to "18.5.5. Standby Servers" since it's a parameter to control the behavior of the standby server rather than that of the master. 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] tsearch Parser Hacking
I agree that it will be a good idea to rewrite the entire thing. However, in the mean time, I sent a proposal earlier http://archives.postgresql.org/pgsql-hackers/2010-08/msg00019.php And a patch later: http://archives.postgresql.org/pgsql-hackers/2010-09/msg00476.php Tom asked me to look into Compound Word support but I found it not usable. Here was my response: http://archives.postgresql.org/pgsql-hackers/2011-01/msg00419.php I have not got any response since then, -Sushant. On Tue, Feb 15, 2011 at 9:33 AM, David E. Wheeler wrote: > On Feb 14, 2011, at 3:57 PM, Tom Lane wrote: > > > There is zero, none, nada, provision for modifying the behavior of the > > default parser, other than by changing its compiled-in state transition > > tables. > > > > It doesn't help any that said tables are baroquely designed and utterly > > undocumented. > > > > IMO, sooner or later we need to trash that code and replace it with > > something a bit more modification-friendly. > > I was afraid you'd say that. Thanks. > > David > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] Add support for logging the current role
On Mon, Feb 14, 2011 at 23:30, Stephen Frost wrote: > > * In assign_csvlog_fields(), we need to cleanup memory and memory context > > before return on error. > Fixed this and a couple of similar issues. Not yet fixed. Switched memory context is not restored on error. > Updated patch attached, git log below. Now I mark the patch to "Ready for Committer", because I don't have suggestions any more. For reference, I note my previous questions. Some of them might be TODO items, or might not. We can add the basic feature in 9.1, and improve it 9.2 or later versions. * csvlog_fields and csvlog_header won't work with non-default log_filename when it doesn't contain seconds in the format. They expect they can always open empty log files. * The long default value for csvlog_fields leads long text line in postgresql.conf, SHOW ALL, pg_settings view, but there were no better alternative solutions in the past discussion. * csvlog_fields is marked as PGC_POSTMASTER. It can protect mixed formats in a csv file on default log_filename, but other similar GUC variables are usually marked AS PGC_SIGHUP. -- Itagaki Takahiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Change pg_last_xlog_receive_location not to move backwards
On Sat, Feb 12, 2011 at 11:32 PM, Robert Haas wrote: > So, what if we did some renaming? I'd be inclined to start by > renaming "receivedUpTo" to Flush, and add a new position called > Stream. When walreciever is started, we set Stream to the position at > which streaming is going to begin (which can rewind) and leave Flush > alone (so it never rewinds). We then change the walreceiver feedback > mechanism to use the term stream_location rather than write_location; > and we could consider replacing pg_last_xlog_receive_location() with > pg_last_xlog_stream_location() and pg_last_xlog_flush_location(). You suggest that the shared variable Stream tracks the WAL write location, after it's set to the replication starting position? I don't think that the write location needs to be tracked in the shmem because other processes than walreceiver don't use it. What I proposed is: Walreceiver-local variables == 1. LogstreamResult.Write - Indicates the location of recently written WAL record - Can rewind - pg_stat_replication.write_location returns this 2. LogstreamResult.Flush - Indicates the location of recently flushed WAL record - Can rewind - pg_stat_replication.flush_location returns this Shmem variables === 3. WalRcv->receiveStart - Indicates the replication starting location - Updated only when walreceiver is started - Doesn't exist at the moment, so I propose to add this 4. WalRcv->receivedUpto - Indicates the latest location of all the flushed WAL records - Never rewinds (Can rewind at the moment, so I propose to prevent the rewind) - pg_last_xlog_receive_location returns this You propose to rename LogstreamResult.Write to .Stream, and merge it and receiveStart? > I'd also be inclined to go to the walreceiver code and and rename the > apply_location to replay_location, so that it matches > pg_last_xlog_replay_location(). The latter is in 9.0, but the former > is new to 9.1, so we can still fix it to be consistent without a > backward compatibility break. +1 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] pg_upgrade seems a tad broken
I wrote: > I tried to do a pg_upgrade from 9.0.x to HEAD today. The pg_upgrade run > went through without complaint, and I could start the postmaster, but > every connection attempt fails with > psql: FATAL: could not read block 0 in file "base/11964/11683": read only 0 > of 8192 bytes > The database OID varies depending on which database I try to connect to, > but the filenode doesn't. In the source 9.0 database, this relfilenode > belongs to pg_largeobject_metadata. I'm not sure whether pg_upgrade > would've preserved relfilenode numbering, so that may or may not be a > useful hint as to where the problem is. But in any case it's busted. Closer investigation shows that in the new database, relfilenode 11683 belongs to pg_class_oid_index, which explains why it's being touched during backend startup. It is indeed of zero length, and surely should not be. I can't resist the guess that something about the recently added hacks for pg_largeobject_metadata is not right. 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] pg_upgrade seems a tad broken
I tried to do a pg_upgrade from 9.0.x to HEAD today. The pg_upgrade run went through without complaint, and I could start the postmaster, but every connection attempt fails with psql: FATAL: could not read block 0 in file "base/11964/11683": read only 0 of 8192 bytes The database OID varies depending on which database I try to connect to, but the filenode doesn't. In the source 9.0 database, this relfilenode belongs to pg_largeobject_metadata. I'm not sure whether pg_upgrade would've preserved relfilenode numbering, so that may or may not be a useful hint as to where the problem is. But in any case it's busted. 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] CommitFest 2011-01 as of 2011-02-04
On mån, 2011-02-14 at 11:49 -0500, Stephen Frost wrote: > Perhaps a thought for next time would be to offset things a bit. eg: > > CF 2011-03 (or whatever): > 2011-02-14: Patches should all be submitted > 2011-02-14: Reviewers start > 2011-03-01: Committers start w/ 'Ready for Committer' patches > 2011-03-14: Patches not marked 'Ready for Committer' get bounced > 2011-03-31: All patches committed > > I'm not against the 'waiting on author' approach, but I do feel like > if we're going to continue to have it, we need to spread it out a bit > more. I don't think it is realistic to add even more dates and bounds and guidelines. People are already widely ignoring the current ones. If you want to have the ability the bounce things more aggressively, I'd argue for shorter and more frequent commitfests. Say, one week per month. -- 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] pl/python do not delete function arguments
On mån, 2011-02-14 at 22:22 +0100, Jan Urbański wrote: > The problem is that every *second* call to the function fails, > regardless of the number. The first execution succeeds, but then > PLy_delete_args deletes the argument from the globals, and when the > next execution tries to fetch "n" from it, it raises a KeyError. This isn't quite right either, because it obviously depends on the recursion somehow. So in SELECT recursion_test(5); SELECT recursion_test(4); it is the first recursive invocation of the (4) call that fails. If you just do SELECT recursion_test(1); SELECT recursion_test(1); SELECT recursion_test(1); nothing fails. (We'd have noticed that sooner, obviously. ;-) ) But in SELECT recursion_test(1); SELECT recursion_test(4); SELECT recursion_test(1); it's the last (1) call, which is not recursive, that fails. Your patch obviously fixes all that, but I'm wondering if we have another problem with the procedure caching somehow. And I'm also wondering what to put into the commit message. :-/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why two dashes in extension load files
On Mon, Feb 14, 2011 at 8:03 PM, Tom Lane wrote: > Robert Haas writes: >> Are we deparsing the names of the SQL files to infer the set of >> version numbers we have to worry about? It seems to me that if >> there's a list of known version numbers somewhere, we can use dash as >> the separator without any special restricton. > > The list of known version numbers is inferred from the available files, > not vice versa. IMO that's a feature not a bug. A manually maintained > list would just be one more thing to forget to update. I could go either way on that one; I was just throwing it up against the wall to see whether it would stick. -- 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] ALTER TYPE 2: skip already-provable no-work rewrites
On Mon, Feb 14, 2011 at 7:52 AM, Noah Misch wrote: >> I'm half-tempted to put that part off to >> 9.2, in the hopes of getting a more substantial solution that can also >> handle things like text -> xml which we don't have time to re-engineer >> right now. > > I see. After sleeping on it, I think this route makes most sense. The ability to downgrade a rewrite to a scan is really a separate feature, and I'd like to see us implement that in a more complete way when/if we're going to do it; and I'd rather commit it at the beginning of a development cycle when we have more time to find any lurking bugs. So I've committed a change that just handles the unconstrained domain case. I think for 9.2 we should revisit the following areas: 1. Downgrading rewrites to scans (vs. skipping them altogether). One idea is that we might modify CREATE CAST so that you can do this: CREATE CAST (source_type AS target_type) WITH [ CHECK ] FUNCTION function_name (argument_type [, ...]) [ AS ASSIGNMENT | AS IMPLICIT ]; The inclusion of the keyword "check" there would inform the system that the binary representation can't change, but (as distinguished from WITHOUT FUNCTION) an error might be thrown. Of course, I'm not quite sure how to get this information over to the alter table machinery cleanly. 2. Detecting binary-coercible cases that involve typemods, rather than just type OIDs. 3. Avoiding index rebuilds. -- 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] why two dashes in extension load files
On Feb 14, 2011, at 8:18 PM, Tom Lane wrote: >> >> Yes, but the truth is that the extension name, at least, is known from the >> control file. > > Yeah, I think it's true in the current code base that we always know the > extension name we are interested in. However, that's no protection if > we allow extensions to contain the separator substring. Consider > foo--bar--baz.sql > Is this an update script for foo (from version bar to version baz), > or is it an install script for some other extension named foo--bar? > > Also, I think it'd be better if we didn't assume that we will always > know the extension name when trying to make sense of a script name. > That's the sort of assumption that will bite you on the rear eventually. Works for me. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why two dashes in extension load files
"David E. Wheeler" writes: > On Feb 14, 2011, at 5:03 PM, Tom Lane wrote: >>> Are we deparsing the names of the SQL files to infer the set of >>> version numbers we have to worry about? It seems to me that if >>> there's a list of known version numbers somewhere, we can use dash as >>> the separator without any special restricton. >> The list of known version numbers is inferred from the available files, >> not vice versa. IMO that's a feature not a bug. A manually maintained >> list would just be one more thing to forget to update. > Yes, but the truth is that the extension name, at least, is known from the > control file. Yeah, I think it's true in the current code base that we always know the extension name we are interested in. However, that's no protection if we allow extensions to contain the separator substring. Consider foo--bar--baz.sql Is this an update script for foo (from version bar to version baz), or is it an install script for some other extension named foo--bar? Also, I think it'd be better if we didn't assume that we will always know the extension name when trying to make sense of a script name. That's the sort of assumption that will bite you on the rear eventually. 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] tsearch Parser Hacking
On Feb 14, 2011, at 3:57 PM, Tom Lane wrote: > There is zero, none, nada, provision for modifying the behavior of the > default parser, other than by changing its compiled-in state transition > tables. > > It doesn't help any that said tables are baroquely designed and utterly > undocumented. > > IMO, sooner or later we need to trash that code and replace it with > something a bit more modification-friendly. I was afraid you'd say that. Thanks. David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [DOCS] [HACKERS] "Extension" versus "module"
On Feb 14, 2011, at 5:42 PM, Tom Lane wrote: >> Remember also that not all modules out there on the net will have been >> updated either, so we must be able to discuss "extension-izing a >> module". (??) > > Right. So it seems like we ought to stick with more or less the > existing terminology: those various components under contrib/ are > modules. Some of them are also extensions, but not all. The similarity of the meaning of the words "extension" and "module" is unfortunate, as it might be hard for one to remember which is which. But given the precedent of the word "module," I don't suppose there's much choice. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why two dashes in extension load files
On Feb 14, 2011, at 5:03 PM, Tom Lane wrote: >> Are we deparsing the names of the SQL files to infer the set of >> version numbers we have to worry about? It seems to me that if >> there's a list of known version numbers somewhere, we can use dash as >> the separator without any special restricton. > > The list of known version numbers is inferred from the available files, > not vice versa. IMO that's a feature not a bug. A manually maintained > list would just be one more thing to forget to update. Yes, but the truth is that the extension name, at least, is known from the control file. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] .gitignore patch for coverage builds
On Mon, Feb 14, 2011 at 7:38 PM, Jeff Janes wrote: > On Wed, Jan 26, 2011 at 2:41 PM, Alvaro Herrera > wrote: >> Excerpts from Tom Lane's message of mié ene 26 19:20:52 -0300 2011: >>> Robert Haas writes: >>> > On Wed, Jan 26, 2011 at 4:44 PM, Tom Lane wrote: >>> >> Ick. That's an awful lot of stuff to have global ignores for. >>> >>> > The "coverage" directory ignore seems a little icky, but the rest >>> > seems unlikely to pick up anything incidental. >>> >>> Tying /coverage to the root as in his V2 makes that better, >> >> Hmm, I don't think that works, because you can run "make coverage" in >> any subdir and it will create a "coverage" subdir there. > > I like being told that I have a coverage directory outstanding when I > run "git status". > > The hundreds of other files, not so much. > > >>> but I'm >>> still unexcited about the thesis that we should auto-ignore the results >>> of any random tool somebody wants to run in their source tree. >> >> Well, in this case it's not any random tool, because it's integrated >> into our makefiles. > > I agree. Should this be added to commit-fest 2011-Next? I think there's little reason not to go ahead and commit this now. It's a trivial patch, Tom is the only one objecting, and there are at least four votes on the other side. The only question in my mind is whether we ought to try to ignore the coverage directories, or just the other glob patterns. -- 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] pg_ctl failover Re: Latches, signals, and waiting
On Tue, Feb 15, 2011 at 2:10 AM, Stephen Frost wrote: > Fujii, > > * Fujii Masao (masao.fu...@gmail.com) wrote: >> Yeah, I rebased the patch to the current git master and attached it. > > Reviewing this, I just had a couple of comments and questions. Overall, > I think it looks good and hence will be marking it 'Ready for > Committer'. Thanks for the review! > * You removed trigger_file from the list in > doc/src/sgml/high-availability.sgml and I'm not sure I agree with > that. It's still perfectly valid and could be used by someone > instead of pg_ctl promote. I'd recommend two things: > - Adding comments into this recovery.conf snippet Adding the following is enough? +# NOTE that if you plan to use "pg_ctl promote" command to promote +# the standby, no trigger file needs to be specified. > - Adding a comment indicationg that trigger_file is only needed if > you're not using pg_ctl promote. Where should I add such a comment? > * I'm not happy that pg_ctl.c doesn't #include something which defines > all the file names which are used, couldn't we use a header which > makes sense and is pulled in by pg_ctl.c and xlog.c to #define all of > these? Still, that's not really the fault of this patch. That would make sense. But I'm not sure that's possible. As a trial, I added '#include "access/xlog.h"' into pg_ctl.c and compiled the source, but I got many compilation errors. So probably hacking Makefiles is required to do that. Do you know where should be changed? > * I'm a bit worried that there's just only so many USR signals that we > can send and it looks like we're burning another one here. Should we > be considering a better way to do this? You're worried about the case where users wrongly send the SIGUSR2 to the startup process, and then the standby is brought up to the master unexpectedly? 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] sepgsql contrib module
Andrew Dunstan writes: > On 02/14/2011 08:36 PM, Tom Lane wrote: >> It looks to me like /selinux/mls is some weird phony-filesystem file, >> because "cat" prints one character (a "1") while "ls" claims the file is >> of zero length. So it's probably something consed up by the kernel, >> like /proc/. Do you have selinux enabled on your machine? > Np, but that really shouldn't be a build requirement, ISTM, even if it > is a test requirement. [ A few reboots later... ] Yeah, I've confirmed that /selinux/mls isn't there at all when SELinux is disabled. When it is there, it reflects the setting of SELINUXTYPE ("targeted" or "mls"). So that explains what /usr/share/selinux/devel/Makefile is doing, but it doesn't make it a good idea. >> (BTW, testing what seems to be a kernel-configuration-reporting flag at >> build time strikes me as pretty awful design.) > Yeah, I agree. Yup, this is just broken by design. It looks like /usr/share/selinux/devel/Makefile basically just sets NAME and TYPE and then calls /usr/share/selinux/devel/include/Makefile, so we could avoid the dependence on the build machine's current state if we did that for ourselves. Of course that just begs the question of what we should set these variables *to*. Since the file being built is only used for regression testing, it wouldn't be unreasonable to pick some values, but it's not clear to me whether things would go blooey if the eventual test machine had different settings. On the whole, I don't think that sepgsql-regtest.pp should be built or installed at all during the build phase. It ought to be generated during regression test startup, instead. 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] tsearch Parser Hacking
On Mon, Feb 14, 2011 at 6:57 PM, Tom Lane wrote: > "David E. Wheeler" writes: >> Is it possible to modify the default tsearch parser so that / doesn't get >> lexed as a "file" token? > > There is zero, none, nada, provision for modifying the behavior of the > default parser, other than by changing its compiled-in state transition > tables. > > It doesn't help any that said tables are baroquely designed and utterly > undocumented. > > IMO, sooner or later we need to trash that code and replace it with > something a bit more modification-friendly. I added this to the TODO as something that can be tackled in the future. I've been wishing it would be possible to add other tokens as well (Python dotted path 'foo.bar.baz', Perl namespace path 'Foo::Bar', more flexible version number parsing, etc). David Blewett -- 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] ALTER TYPE 2: skip already-provable no-work rewrites
On Mon, Feb 14, 2011 at 04:06:59PM -0500, Stephen Frost wrote: > * Noah Misch (n...@leadboat.com) wrote: > > On Mon, Feb 14, 2011 at 01:12:21PM -0500, Stephen Frost wrote: > > > In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a > > > passed-in argument to move through a list with.. I'd suggest using a > > > local variable that is set from what's passed in. I do see that's > > > inheirited, but still, you've pretty much redefined that entire > > > function anyway.. > > > > Could do that. However, the function would reference the original argument > > just > > once, to make that copy. I'm not seeing a win. > > Perhaps I'm just deficient in this area, but I think of arguments, > unless specifically intended otherwise, to be 'read-only' and based on > that assumption, seeing it come up as an lvalue hits me as wrong. > > I'm happy enough to let someone else decide if they agree with me or you > though. :) Same here. If there's a general project tendency either way, I'll comply. I haven't noticed one, but I haven't been looking. I've attached a new version of the patch that attempts to flesh out the comments based on your feedback. Does it improve things? > > The validity of this optimization does not > > rely on any user-settable property of a data type, but it does lean heavily > > on > > the execution semantics of specific nodes. > > After thinking through this and diving into coerce_to_target_type() a > bit, I'm finally coming to understand how this is working. The gist of > it, if I follow correctly, is that if the planner doesn't think we have > to do anything but copy this value to make it the new data type, then > we're good to go. That makes sense, when you think about it, but boy > does it go the long way around to get there. Essentially. The planner is not yet involved. We're looking at an expression tree after parse analysis, before planning. > Essentially, coerce_to_target_type() is returning an expression tree and > ATColumnChangeSetWorkLevel() is checking to see if that expression tree > is "copy the value". Maybe this is a bit much, but if > coerce_to_target_type() knows the expression given to it is a > straight-up copy, perhaps it could pass that information along in an > easier to use format than an expression tree? That would obviate the > need for ATColumnChangeSetWorkLevel() entirely, if I understand > correctly. PostgreSQL has a strong tradition of passing around expression trees and walking them to (re-)discover facts. See the various clauses.h functions. Also, when you have a USING clause, coerce_to_target_type() doesn't know the whole picture. We could teach it to discover the whole picture, but that would amount to a very similar tree walk in a different place. > Of course, coerce_to_target_type() is used by lots of other places, > almost all of which probably have to have an expression tree to stuff > into a plan, so maybe a simpler function could be defined which operates > at the level that ATColumnChangeSetWorkLevel() needs? Offhand, I can't think of any concrete implementation along those lines that would simplify the overall task. Did you have something more specific in mind? > Or perhaps other > places would benefit from knowing that a given conversion is an actual > no-op rather than copying the value? RelabelType itself does not cause a copy; the existing datum passes through. In the case of ALTER TABLE, we do eventually copy the datum via heap_form_tuple. There may be other places that would benefit from similar analysis. For that reason, I originally had ATColumnChangeSetWorkLevel() in parse_coerce.c with the name GetCoerceExemptions(). I'm not aware of any specific applications, though. For now it seemed like premature abstraction. Thanks again, nm diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 452ced6..466be25 100644 diff --git a/src/backend/commands/index 1db42d0..ab0bcda 100644 *** a/src/backend/commands/tablecmds.c --- b/src/backend/commands/tablecmds.c *** *** 71,76 --- 71,77 #include "storage/smgr.h" #include "utils/acl.h" #include "utils/builtins.h" + #include "utils/datum.h" #include "utils/fmgroids.h" #include "utils/inval.h" #include "utils/lsyscache.h" *** *** 142,147 typedef struct AlteredTableInfo --- 143,149 List *newvals;/* List of NewColumnValue */ boolnew_notnull;/* T if we added new NOT NULL constraints */ boolrewrite;/* T if we a rewrite is forced */ + boolverify; /* T if we shall verify constraints */ Oid newTableSpace; /* new tablespace; 0 means no change */ /* Objects to rebuild after completing ALTER TYPE operations */ List *changedConstraintOids; /* OIDs of constraints to rebuild */ *** *** 336,342 static void ATPr
Re: [HACKERS] sepgsql contrib module
On 02/14/2011 08:36 PM, Tom Lane wrote: Andrew Dunstan writes: Yeah. The next thing I hit was this: [andrew@aurelia sepgsql]$ make -f /usr/share/selinux/devel/Makefile sepgsql-regtest.pp cat: /selinux/mls: No such file or directory make: *** No rule to make target `sepgsql-regtest.pp'. Stop. [andrew@aurelia sepgsql]$ Hmph. A build with --with-selinux goes through for me on a pretty-vanilla Fedora 13 installation (at least the build and install steps, I dunno how to test it). It looks to me like /selinux/mls is some weird phony-filesystem file, because "cat" prints one character (a "1") while "ls" claims the file is of zero length. So it's probably something consed up by the kernel, like /proc/. Do you have selinux enabled on your machine? Np, but that really shouldn't be a build requirement, ISTM, even if it is a test requirement. (BTW, testing what seems to be a kernel-configuration-reporting flag at build time strikes me as pretty awful design.) Yeah, I agree. 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] Re: [COMMITTERS] pgsql: Basic Recovery Control functions for use in Hot Standby. Pause,
On Wed, Feb 9, 2011 at 5:12 PM, Magnus Hagander wrote: > I was also worried about the non-hot-standby case, but I see that the > patch makes sure you can't enable pause when not in hot standby mode. > Which in itself might be surprising - perhaps we need a NOTICE for > when that happens as well? > > And it definitely needs to be mentioned in the docs for > pause_at_recovery_target that it only works in hot standby. +1 with all the above. 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
[HACKERS] Re: [COMMITTERS] pgsql: Basic Recovery Control functions for use in Hot Standby. Pause,
On Tue, Feb 15, 2011 at 9:21 AM, Simon Riggs wrote: > On Wed, 2011-02-09 at 15:22 +0900, Fujii Masao wrote: > >> On the second thought, I think it's useful to emit the NOTICE message when >> recovery reaches the pause point, as follows. >> >> NOTICE: Recovery will not complete until pg_xlog_replay_resume() is >> called. > > I'm OK with adding a message, but NOTICE is the wrong level. > > My proposal is this message > > LOG: Recovery has paused. Execute pg_xlog_replay_resume() to continue. I agree to use LOG level. But "Execute pg_xlog_replay_resume() to continue." looks like a hint rather than log. So what about: LOG: Recovery has paused. HINT: Execute pg_xlog_replay_resume() to continue. 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: [DOCS] [HACKERS] "Extension" versus "module"
Simon Riggs writes: > I would say that some modules are extensions, but not all. A standalone > executable might be part of a module, but would not be an extension. > Remember also that not all modules out there on the net will have been > updated either, so we must be able to discuss "extension-izing a > module". (??) Right. So it seems like we ought to stick with more or less the existing terminology: those various components under contrib/ are modules. Some of them are also extensions, but not all. 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] "Extension" versus "module"
On Mon, 2011-02-14 at 12:48 +0100, Dimitri Fontaine wrote: > Tom Lane writes: > > Appendix F (contrib.sgml and its subsidiary files) is pretty consistent > > about using "module" to refer to a contrib, uh, module. > > I'm now thinking in those terms: the module is the shared object library > that the backend needs to dlopen(). The extension is the SQL level > object that wraps all its components. I would say that some modules are extensions, but not all. A standalone executable might be part of a module, but would not be an extension. Remember also that not all modules out there on the net will have been updated either, so we must be able to discuss "extension-izing a module". (??) -- Simon Riggs http://www.2ndQuadrant.com/books/ 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] sepgsql contrib module
Andrew Dunstan writes: > Yeah. The next thing I hit was this: > [andrew@aurelia sepgsql]$ make -f /usr/share/selinux/devel/Makefile > sepgsql-regtest.pp > cat: /selinux/mls: No such file or directory > make: *** No rule to make target `sepgsql-regtest.pp'. Stop. > [andrew@aurelia sepgsql]$ Hmph. A build with --with-selinux goes through for me on a pretty-vanilla Fedora 13 installation (at least the build and install steps, I dunno how to test it). It looks to me like /selinux/mls is some weird phony-filesystem file, because "cat" prints one character (a "1") while "ls" claims the file is of zero length. So it's probably something consed up by the kernel, like /proc/. Do you have selinux enabled on your machine? (BTW, testing what seems to be a kernel-configuration-reporting flag at build time strikes me as pretty awful design.) 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] Replication server timeout patch
On Mon, 2011-02-14 at 14:13 -0800, Daniel Farina wrote: > On Mon, Feb 14, 2011 at 12:48 AM, Fujii Masao wrote: > > On Sat, Feb 12, 2011 at 8:58 AM, Daniel Farina wrote: > >> Context diff equivalent attached. > > > > Thanks for the patch! > > > > As I said before, the timeout which this patch provides doesn't work well > > when the walsender gets blocked in sending WAL. At first, we would > > need to implement a non-blocking write function as an infrastructure > > of the replication timeout, I think. > > http://archives.postgresql.org/message-id/AANLkTi%3DPu2ne%3DVO-%2BCLMXLQh9y85qumLCbBP15CjnyUS%40mail.gmail.com I wasn't aware that had been raised before. Thanks for noting it again. I guess that's why you thought "wait forever" was a good idea ;-) > Interesting point...if that's accepted as required-for-commit, what > are the perceptions of the odds that, presuming I can write the code > quickly enough, that there's enough infrastructure/ports already in > postgres to allow for a non-blocking write on all our supported > platforms? I'd like to see what you come up with. I would rate that as important, though not essential for sync replication. -- Simon Riggs http://www.2ndQuadrant.com/books/ 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] CommitFest 2011-01 as of 2011-02-04
* Robert Haas (robertmh...@gmail.com) wrote: > But the > trickiest part of this whole process is that, on the one hand, it's > not fair for committers to ignore other people's patches, but on the > other hand, it's not fair to expect committers to sacrifice getting > their own projects done to get other people's projects done. It wasn't my intent to ask the committers to do more but rather to change the timings a bit so that when committers begin their "commitfest month", the patches are already in better quality and more apt to be ready for committer. Anyhow, was just a thought. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] why two dashes in extension load files
Robert Haas writes: > Are we deparsing the names of the SQL files to infer the set of > version numbers we have to worry about? It seems to me that if > there's a list of known version numbers somewhere, we can use dash as > the separator without any special restricton. The list of known version numbers is inferred from the available files, not vice versa. IMO that's a feature not a bug. A manually maintained list would just be one more thing to forget to update. 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] .gitignore patch for coverage builds
On Wed, Jan 26, 2011 at 2:41 PM, Alvaro Herrera wrote: > Excerpts from Tom Lane's message of mié ene 26 19:20:52 -0300 2011: >> Robert Haas writes: >> > On Wed, Jan 26, 2011 at 4:44 PM, Tom Lane wrote: >> >> Ick. That's an awful lot of stuff to have global ignores for. >> >> > The "coverage" directory ignore seems a little icky, but the rest >> > seems unlikely to pick up anything incidental. >> >> Tying /coverage to the root as in his V2 makes that better, > > Hmm, I don't think that works, because you can run "make coverage" in > any subdir and it will create a "coverage" subdir there. I like being told that I have a coverage directory outstanding when I run "git status". The hundreds of other files, not so much. >> but I'm >> still unexcited about the thesis that we should auto-ignore the results >> of any random tool somebody wants to run in their source tree. > > Well, in this case it's not any random tool, because it's integrated > into our makefiles. I agree. Should this be added to commit-fest 2011-Next? Also, should "make clean-coverage" be changed to remove all of those files from the entire tree and not just root? Cheers, Jeff -- 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: [COMMITTERS] pgsql: Basic Recovery Control functions for use in Hot Standby. Pause,
On Wed, 2011-02-09 at 15:22 +0900, Fujii Masao wrote: > On the second thought, I think it's useful to emit the NOTICE message when > recovery reaches the pause point, as follows. > > NOTICE: Recovery will not complete until pg_xlog_replay_resume() is > called. I'm OK with adding a message, but NOTICE is the wrong level. My proposal is this message LOG: Recovery has paused. Execute pg_xlog_replay_resume() to continue. -- Simon Riggs http://www.2ndQuadrant.com/books/ 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] tsearch Parser Hacking
On 14 February 2011 23:57, Tom Lane wrote: > "David E. Wheeler" writes: >> Is it possible to modify the default tsearch parser so that / doesn't get >> lexed as a "file" token? > > There is zero, none, nada, provision for modifying the behavior of the > default parser, other than by changing its compiled-in state transition > tables. > > It doesn't help any that said tables are baroquely designed and utterly > undocumented. This is very true. I intended to look into adding new tokens, but gave up when I couldn't see how those transition tables worked. > IMO, sooner or later we need to trash that code and replace it with > something a bit more modification-friendly. +1 for annihilating the existing code at some point. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why two dashes in extension load files
On Mon, Feb 14, 2011 at 10:13 AM, Tom Lane wrote: > Peter Eisentraut writes: >> Why do the extension load files need two dashes, like xml2--1.0.sql? >> Why isn't one enough? > > Because we'd have to forbid dashes in extension name and version > strings. This was judged to be a less annoying solution. See > yesterday's discussion. Are we deparsing the names of the SQL files to infer the set of version numbers we have to worry about? It seems to me that if there's a list of known version numbers somewhere, we can use dash as the separator without any special restricton. For example foo-bar-baz-bletch.sql is either an upgrade script from version bar-baz to version bletch, or else it's an upgrade script from bar to baz-bletch. But presumably no real-world cases will actually be ambiguous, assuming any sort of half-way sane version numbering scheme. -- 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] tsearch Parser Hacking
"David E. Wheeler" writes: > Is it possible to modify the default tsearch parser so that / doesn't get > lexed as a "file" token? There is zero, none, nada, provision for modifying the behavior of the default parser, other than by changing its compiled-in state transition tables. It doesn't help any that said tables are baroquely designed and utterly undocumented. IMO, sooner or later we need to trash that code and replace it with something a bit more modification-friendly. 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] CommitFest 2011-01 as of 2011-02-04
Sorry for the previous, content-free reply. On Mon, Feb 14, 2011 at 11:49 AM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> Here's where I think we are with this CommitFest. > > Subject: Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04 > > I'm gonna go out on a limb and hope you meant '2011-02-14' there. :) Yeah, sorry. >> So there are two basic difficulties with wrapping the CommitFest up. > > I have to say that I've always been a bit suprised by the idea that the > CommitFest is intended to be done and all patches *committed* at the end > of the month. It's been working really rather well, which is due in > great part to the excellent CF managers (thanks again for being that, > again). That said, we have quite a few non-committer reviewers who > provide good feedback and move the patch back to 'waiting for author' > and that whole process takes a while. It does, but frankly I don't see much reason to change it, since it's been working pretty well on the whole. Andrew was on point when he mentioned that it's not obvious what committers get out of working on other people's patches. Obviously, the answer is, well, they get a better PostgreSQL, and that's ultimately good for all of us. But the trickiest part of this whole process is that, on the one hand, it's not fair for committers to ignore other people's patches, but on the other hand, it's not fair to expect committers to sacrifice getting their own projects done to get other people's projects done. -- 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] FOR KEY LOCK foreign keys
Excerpts from Marti Raudsepp's message of lun feb 14 19:39:25 -0300 2011: > On Fri, Feb 11, 2011 at 09:13, Noah Misch wrote: > > The patch had a trivial conflict in planner.c, plus plenty of offsets. I've > > attached the rebased patch that I used for review. For anyone following > > along, > > all the interesting hunks touch heapam.c; the rest is largely mechanical. A > > "diff -w" patch is also considerably easier to follow. > > Here's a simple patch for the RelationGetIndexAttrBitmap() function, > as explained in my last post. I don't know if it's any help to you, > but since I wrote it I might as well send it up. This applies on top > of Noah's rebased patch. Got it, thanks. > I did some tests and it seems to work, although I also hit the same > visibility bug as Noah. Yeah, that bug is fixed with the attached, though I am rethinking this bit. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support 0001-Fix-visibility-bug-and-poorly-worded-comment.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] CommitFest 2011-01 as of 2011-02-04
On Mon, Feb 14, 2011 at 11:49 AM, Stephen Frost wrote: > I have to say that I've always been a bit suprised by the idea that the > CommitFest is intended to be done and all patches *committed* at the end > of the month. It's been working really rather well, which is due in > great part to the excellent CF managers (thanks again for being that, > again). That said, we have quite a few non-committer reviewers who > provide good feedback and move the patch back to 'waiting for author' > and that whole process takes a while. > > Perhaps a thought for next time would be to offset things a bit. eg: > > CF 2011-03 (or whatever): > 2011-02-14: Patches should all be submitted > 2011-02-14: Reviewers start > 2011-03-01: Committers start w/ 'Ready for Committer' patches > 2011-03-14: Patches not marked 'Ready for Committer' get bounced > 2011-03-31: All patches committed > > I'm not against the 'waiting on author' approach, but I do feel like if > we're going to continue to have it, we need to spread it out a bit more. > I do think this would place more work on the CF manager, unfortunately, > but I'd hope that they would primairly be focused on managing the > reviews and not be as busy during the last 2 weeks. Maybe one day I'll > be brave enough to offer to manage one and see. :) > > Thanks again, Robert, you've done an excellent job managing the CF. > > Stephen > > -BEGIN PGP SIGNATURE- > Version: GnuPG v1.4.10 (GNU/Linux) > > iEYEARECAAYFAk1ZXTQACgkQrzgMPqB3kiiLAQCfUVusKmhcQW1KNGQwZSFpdONx > G4oAnjzPLSQpyaounlTMrumdoQe58yA/ > =zuCX > -END PGP SIGNATURE- > > -- 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] FOR KEY LOCK foreign keys
On Fri, Feb 11, 2011 at 09:13, Noah Misch wrote: > The patch had a trivial conflict in planner.c, plus plenty of offsets. I've > attached the rebased patch that I used for review. For anyone following > along, > all the interesting hunks touch heapam.c; the rest is largely mechanical. A > "diff -w" patch is also considerably easier to follow. Here's a simple patch for the RelationGetIndexAttrBitmap() function, as explained in my last post. I don't know if it's any help to you, but since I wrote it I might as well send it up. This applies on top of Noah's rebased patch. I did some tests and it seems to work, although I also hit the same visibility bug as Noah. Test case I used: THREAD A: create table foo (pk int primary key, ak int); create unique index on foo (ak) where ak != 0; create unique index on foo ((-ak)); create table bar (foo_pk int references foo (pk)); insert into foo values(1,1); begin; insert into bar values(1); THREAD B: begin; update foo set ak=2 where ak=1; Regards, Marti From e069cef91c686aa87e220336198267e5a5a2aeac Mon Sep 17 00:00:00 2001 From: Marti Raudsepp Date: Tue, 15 Feb 2011 00:33:35 +0200 Subject: [PATCH] Only acquire KEY LOCK for colums that can be referenced by foreign keys Don't consider columns in unique indexes that have expressions or WHERE predicates. --- src/backend/utils/cache/relcache.c | 23 +-- src/include/utils/rel.h|2 +- src/include/utils/relcache.h |2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 4d37e8e..5119288 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3608,7 +3608,8 @@ RelationGetIndexPredicate(Relation relation) * simple index keys, but attributes used in expressions and partial-index * predicates.) * - * If "unique" is true, only attributes of unique indexes are considered. + * If "keyAttrs" is true, only attributes that can be referenced by foreign + * keys are considered. * * Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that * we can include system attributes (e.g., OID) in the bitmap representation. @@ -3617,7 +3618,7 @@ RelationGetIndexPredicate(Relation relation) * be bms_free'd when not needed anymore. */ Bitmapset * -RelationGetIndexAttrBitmap(Relation relation, bool unique) +RelationGetIndexAttrBitmap(Relation relation, bool keyAttrs) { Bitmapset *indexattrs; Bitmapset *uindexattrs; @@ -3627,7 +3628,7 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique) /* Quick exit if we already computed the result. */ if (relation->rd_indexattr != NULL) - return bms_copy(unique ? relation->rd_uindexattr : relation->rd_indexattr); + return bms_copy(keyAttrs ? relation->rd_keyattr : relation->rd_indexattr); /* Fast path if definitely no indexes */ if (!RelationGetForm(relation)->relhasindex) @@ -3653,12 +3654,18 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique) Relation indexDesc; IndexInfo *indexInfo; int i; + bool isKey; indexDesc = index_open(indexOid, AccessShareLock); /* Extract index key information from the index's pg_index row */ indexInfo = BuildIndexInfo(indexDesc); + /* Can this index be referenced by a foreign key? */ + isKey = indexInfo->ii_Unique && +indexInfo->ii_Expressions == NIL && +indexInfo->ii_Predicate == NIL; + /* Collect simple attribute references */ for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++) { @@ -3668,7 +3675,7 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique) { indexattrs = bms_add_member(indexattrs, attrnum - FirstLowInvalidHeapAttributeNumber); -if (indexInfo->ii_Unique) +if (isKey) uindexattrs = bms_add_member(uindexattrs, attrnum - FirstLowInvalidHeapAttributeNumber); } @@ -3676,13 +3683,9 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique) /* Collect all attributes used in expressions, too */ pull_varattnos((Node *) indexInfo->ii_Expressions, &indexattrs); - if (indexInfo->ii_Unique) - pull_varattnos((Node *) indexInfo->ii_Expressions, &uindexattrs); /* Collect all attributes in the index predicate, too */ pull_varattnos((Node *) indexInfo->ii_Predicate, &indexattrs); - if (indexInfo->ii_Unique) - pull_varattnos((Node *) indexInfo->ii_Predicate, &uindexattrs); index_close(indexDesc, AccessShareLock); } @@ -3692,11 +3695,11 @@ RelationGetIndexAttrBitmap(Relation relation, bool unique) /* Now save a copy of the bitmap in the relcache entry. */ oldcxt = MemoryContextSwitchTo(CacheMemoryContext); relation->rd_indexattr = bms_copy(indexattrs); - relation->rd_uindexattr = bms_copy(uindexattrs); + relation->rd_keyattr = bms_copy(uindexattrs); MemoryContextSwitchTo(oldcxt); /* We return our original working copy for caller to play with */ - return unique ? uindexattrs : indexat
[HACKERS] tsearch Parser Hacking
Hackers, Is it possible to modify the default tsearch parser so that / doesn't get lexed as a "file" token? That is, instead of this: try=# select * from ts_debug('simple'::regconfig, 'w/d'); alias │description│ token │ dictionaries │ dictionary │ lexemes ───┼───┼───┼──┼┼─ file │ File or path name │ w/d │ {simple} │ simple │ {w/d} Ideally it'd think that / was the same as -: try=# select * from ts_debug('simple'::regconfig, 'w-d'); alias │ description │ token │ dictionaries │ dictionary │ lexemes ─┼─┼───┼──┼┼─ asciihword │ Hyphenated word, all ASCII │ w-d │ {simple} │ simple │ {w-d} hword_asciipart │ Hyphenated word part, all ASCII │ w │ {simple} │ simple │ {w} blank │ Space symbols │ - │ {} │ [null] │ [null] hword_asciipart │ Hyphenated word part, all ASCII │ d │ {simple} │ simple │ {d} (4 rows) Possible? Or would I have to write a completely new parser just to change this bit? Thanks, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql contrib module
On 02/14/2011 04:21 PM, Tom Lane wrote: Andrew Dunstan writes: Thew makefile still has this bogosity: sepgsql-regtest.pp: sepgsql-regtest.te $(MAKE) -f $(DESTDIR)/usr/share/selinux/devel/Makefile $@ We need to fix that up before we even think of trying to get buildfarm coverage. The presence and location of this makefile should be determined at configure time, ISTM. I'd suggest just getting rid of the $(DESTDIR), which is flat out wrong, and leaving it as-is otherwise. The portability level of this code is somewhere at the bad-joke level anyway; if you're trying to get it to run anywhere but a recent Fedora/RHEL system, I really doubt that path is the first or biggest problem you'll hit. Yeah. The next thing I hit was this: [andrew@aurelia sepgsql]$ make -f /usr/share/selinux/devel/Makefile sepgsql-regtest.pp cat: /selinux/mls: No such file or directory make: *** No rule to make target `sepgsql-regtest.pp'. Stop. [andrew@aurelia sepgsql]$ That's not very nice. 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] Replication server timeout patch
On Mon, Feb 14, 2011 at 12:48 AM, Fujii Masao wrote: > On Sat, Feb 12, 2011 at 8:58 AM, Daniel Farina wrote: >> Context diff equivalent attached. > > Thanks for the patch! > > As I said before, the timeout which this patch provides doesn't work well > when the walsender gets blocked in sending WAL. At first, we would > need to implement a non-blocking write function as an infrastructure > of the replication timeout, I think. > http://archives.postgresql.org/message-id/AANLkTi%3DPu2ne%3DVO-%2BCLMXLQh9y85qumLCbBP15CjnyUS%40mail.gmail.com Interesting point...if that's accepted as required-for-commit, what are the perceptions of the odds that, presuming I can write the code quickly enough, that there's enough infrastructure/ports already in postgres to allow for a non-blocking write on all our supported platforms? -- fdr -- 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] Scheduled maintenance affecting gitmaster
On Mon, Feb 14, 2011 at 16:15, Magnus Hagander wrote: > On Mon, Feb 14, 2011 at 10:39, Stefan Kaltenbrunner > wrote: >> On 02/14/2011 10:09 AM, Magnus Hagander wrote: >>> On Mon, Feb 14, 2011 at 07:13, Stefan Kaltenbrunner >>> wrote: On 02/14/2011 01:27 AM, Tom Lane wrote: > > Magnus Hagander writes: >> >> Unfortunately, one of the worst-case scenarios appears to have >> happened - a machine did not come back up after a reboot. >> ... >> We'll get back to you with more information as soon as we have it. > > I didn't see any followup to this? yeah - the hosting company managed to reboot the box for us which brought it back to life in the middle of the night (with both magnus and me asleep). >>> >>> Indeed. But the good news is that once it came back up, the VM with >>> the git server started ok :-) >>> >>> > gitmaster seems to be responding as of now, is it safe to push? yes it is - however we will need to schedule another maintenance window soon to finish the stuff we actually wanted to do. >>> >>> So, after some discussion with Stefan, we (well, I guess I) decided we >>> should just go ahead and declare the maintenance window not closed >>> yet, and finish off the upgrade right now :-) Given that the majority >>> of our commits don't happen now, we'll hopefully have it done by the >>> time the US folks wake up again. >>> >>> So, maintenance window again, starting now, and we'll let you know as >>> soon as we're done. And we're definitely hoping for the machine to >>> come back up properly this time :-) >> >> and it did not... We are trying to figure out what the actual problem >> here really is because it seems to boot just fine when powercycled just >> not with a software initiated reboot. >> We will notify once we have more information... > > Status update on this - Stefan is currently working with the > datacenter people on getting this fixed (they are now available > on-site), since we are now having an actual issue with the machine > (GRUB failure on boot) rather than just a failure to shut down. We are still having issues with this box. For that reason, we have moved the gitmaster server over to another box, where it's now up and running. DNS has been updated, but it will take some time for it to sync out. For those of you who want access now, you ca nreach the new master server at 98.129.198.116. Note, however, that mail relaying from this machine does not currently work, so commit messages will be out for a bit longer while we work on clearing things up. -- 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] sepgsql contrib module
Andrew Dunstan writes: > Thew makefile still has this bogosity: > sepgsql-regtest.pp: sepgsql-regtest.te > $(MAKE) -f $(DESTDIR)/usr/share/selinux/devel/Makefile $@ > We need to fix that up before we even think of trying to get buildfarm > coverage. The presence and location of this makefile should be > determined at configure time, ISTM. I'd suggest just getting rid of the $(DESTDIR), which is flat out wrong, and leaving it as-is otherwise. The portability level of this code is somewhere at the bad-joke level anyway; if you're trying to get it to run anywhere but a recent Fedora/RHEL system, I really doubt that path is the first or biggest problem you'll hit. 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] pl/python do not delete function arguments
On 14/02/11 22:13, Jan Urbański wrote: > On 14/02/11 21:06, Peter Eisentraut wrote: >> On ons, 2011-02-09 at 10:02 +0100, Jan Urbański wrote: >>> On 09/02/11 04:52, Hitoshi Harada wrote: 2010/12/31 Jan Urbański : > (continuing the flurry of patches) > > Here's a patch that stops PL/Python from removing the function's > arguments from its globals dict after calling it. It's > an incremental patch on top of the plpython-refactor patch sent in > http://archives.postgresql.org/message-id/4d135170.3080...@wulczer.org. > > Git branch for this patch: > https://github.com/wulczer/postgres/tree/dont-remove-arguments > > Apart from being useless, as the whole dict is unreffed and thus freed > in PLy_procedure_delete, removing args actively breaks things for > recursive invocation of the same function. The recursive callee after > returning will remove the args from globals, and subsequent access to > the arguments in the caller will cause a NameError (see new regression > test in patch). I've reviewed this. The patch is old enough to be rejected by patch command, but I manged to apply it by hand. It compiles clean. Added tests pass. I created fibonacci function similar to recursion_test in the patch and confirmed the recursion raises error on 9.0 but not on 9.1. Doc is not with the patch since this change is to remove unnecessary optimization internally. "Ready for Committer" >>> >>> Thanks, >>> >>> patch merged with HEAD attached. >> >> Curiously, without the patch the recursion_test(4) call fails but >> recursion_test(5) passes. Anyone know why? Baah, damn, did not read the "without patch" part. The problem is that every *second* call to the function fails, regardless of the number. The first execution succeeds, but then PLy_delete_args deletes the argument from the globals, and when the next execution tries to fetch "n" from it, it raises a KeyError. Jan -- 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] pl/python do not delete function arguments
On 14/02/11 21:06, Peter Eisentraut wrote: > On ons, 2011-02-09 at 10:02 +0100, Jan Urbański wrote: >> On 09/02/11 04:52, Hitoshi Harada wrote: >>> 2010/12/31 Jan Urbański : (continuing the flurry of patches) Here's a patch that stops PL/Python from removing the function's arguments from its globals dict after calling it. It's an incremental patch on top of the plpython-refactor patch sent in http://archives.postgresql.org/message-id/4d135170.3080...@wulczer.org. Git branch for this patch: https://github.com/wulczer/postgres/tree/dont-remove-arguments Apart from being useless, as the whole dict is unreffed and thus freed in PLy_procedure_delete, removing args actively breaks things for recursive invocation of the same function. The recursive callee after returning will remove the args from globals, and subsequent access to the arguments in the caller will cause a NameError (see new regression test in patch). >>> >>> I've reviewed this. The patch is old enough to be rejected by patch >>> command, but I manged to apply it by hand. >>> It compiles clean. Added tests pass. >>> I created fibonacci function similar to recursion_test in the patch >>> and confirmed the recursion raises error on 9.0 but not on 9.1. >>> Doc is not with the patch since this change is to remove unnecessary >>> optimization internally. >>> >>> "Ready for Committer" >> >> Thanks, >> >> patch merged with HEAD attached. > > Curiously, without the patch the recursion_test(4) call fails but > recursion_test(5) passes. Anyone know why? Damn, I remember that bug and thought I fixed it. I think calls with even numbers passed and calls with odd numbers failed... I'll try to see if a merge error did not introduce that bug back. > Btw., I get a KeyError, not a NameError as you say above. Will look into that too. Cheers, Jan -- 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] ALTER TYPE 2: skip already-provable no-work rewrites
* Noah Misch (n...@leadboat.com) wrote: > On Mon, Feb 14, 2011 at 01:12:21PM -0500, Stephen Frost wrote: > > First question is- why do you use #ifdef USE_ASSERT_CHECKING ..? > > The other six code sites checking assert_enabled directly do the same. Wow, I could have sworn that I looked at what others were doing and didn't see that. Sorry for the noise. > > In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a > > passed-in argument to move through a list with.. I'd suggest using a > > local variable that is set from what's passed in. I do see that's > > inheirited, but still, you've pretty much redefined that entire > > function anyway.. > > Could do that. However, the function would reference the original argument > just > once, to make that copy. I'm not seeing a win. Perhaps I'm just deficient in this area, but I think of arguments, unless specifically intended otherwise, to be 'read-only' and based on that assumption, seeing it come up as an lvalue hits me as wrong. I'm happy enough to let someone else decide if they agree with me or you though. :) > The way I like to think about it is that we're recursively walking an > expression > tree to determine which of three categories it falls into: always produces the > same value without error; always produces the same value on success, but may > throw an error; neither of the above. We have a whitelist of node types that > are acceptable, and anything else makes us assume the worst. (The nodes we > accept are simple enough that the recursion degenerates to iteration.) It's that degeneration that definitely hits me as making the whole thing look a bit 'funny'. When I first looked at the loop, I was looking for a tree structure and trying to figure out how it could work with just a simple while(). > http://archives.postgresql.org/message-id/20101231013534.ga7...@tornado.leadboat.com > > Should I restore some of that? Any other particular text that would have > helped? I definitely think the examples given, enumerating the types of nodes that matter for this (and why they're the ones we look for), and the rules that are followed would help a great deal. Anyone else who comes across this code may be wondering "do I need to do something here for this new node type that I just added". > > It also seems like it'd make more sense to me to > > be a while() controlled by (IsA(expr, Var) && ((Var *) expr)->varattno > > == varattno), since that's really the normal "stopping point". > > If we can optimize to some extent, that is the stopping point. If not, > tab->rewrite is the stopping point. I picked the no-optimization case as > "normal" and used that as the loop condition, but one could go either way. I would think you could still short-circuit the loop when you've discovered a case where we have to rewrite the table anyway. Having the comments updated to reflect what's going on and why stopping on tab->rewrite, in particular, makes sense is more important though. > The validity of this optimization does not > rely on any user-settable property of a data type, but it does lean heavily on > the execution semantics of specific nodes. After thinking through this and diving into coerce_to_target_type() a bit, I'm finally coming to understand how this is working. The gist of it, if I follow correctly, is that if the planner doesn't think we have to do anything but copy this value to make it the new data type, then we're good to go. That makes sense, when you think about it, but boy does it go the long way around to get there. Essentially, coerce_to_target_type() is returning an expression tree and ATColumnChangeSetWorkLevel() is checking to see if that expression tree is "copy the value". Maybe this is a bit much, but if coerce_to_target_type() knows the expression given to it is a straight-up copy, perhaps it could pass that information along in an easier to use format than an expression tree? That would obviate the need for ATColumnChangeSetWorkLevel() entirely, if I understand correctly. Of course, coerce_to_target_type() is used by lots of other places, almost all of which probably have to have an expression tree to stuff into a plan, so maybe a simpler function could be defined which operates at the level that ATColumnChangeSetWorkLevel() needs? Or perhaps other places would benefit from knowing that a given conversion is an actual no-op rather than copying the value? Just my 2c, I don't believe the patch could cause problems now that I'm understanding it better, but it sure does seem excessive to use an expression tree to figure out when something is a no-op. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] FOR KEY LOCK foreign keys
Excerpts from Noah Misch's message of vie feb 11 04:13:22 -0300 2011: > I observe visibility breakage with this test case: > > [ ... ] > > The problem seems to be that funny t_cid (2249). Tracing through heap_update, > the new code is not setting t_cid during this test case. So I can fix this problem by simply adding a call to HeapTupleHeaderSetCmin when the stuff about ComboCid does not hold, but seeing that screenful plus the subsequent call to HeapTupleHeaderAdjustCmax feels wrong. I think this needs to be rethought ... -- Á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] why two dashes in extension load files
t...@sss.pgh.pa.us (Tom Lane) writes: > Peter Eisentraut writes: >> On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote: >>> Peter Eisentraut writes: Why do the extension load files need two dashes, like xml2--1.0.sql? Why isn't one enough? > >>> Because we'd have to forbid dashes in extension name and version >>> strings. This was judged to be a less annoying solution. See >>> yesterday's discussion. > >> I'm not convinced. There was nothing in that discussion why any >> particular character would have to be allowed in a version number. > > Well, there's already a counterexample in the current contrib stuff: > uuid-ossp. We could rename that to uuid_ossp of course, but it's > not clear to me that there's consensus for forbidding dashes here. I suspect that "_" might be troublesome. Let me observe on Debian policy... It requires that package names consist as follows: Package names (both source and binary, see Package, Section 5.6.7) must consist only of lower case letters (a-z), digits (0-9), plus (+) and minus (-) signs, and periods (.). They must be at least two characters long and must start with an alphanumeric character. http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Source I suspect that we'll need to have a policy analagous to that. Also worth observing: Debian package files are of the form: "${package}_${version}-${dversion}_${arch}.deb" where package and version have fairly obvious interpretation, and... - dversion indicates a sequence handled by Debian - arch indicates CPU architecture (i386, amd64, ...) Probably the dversion/arch bits aren't of interest to us, but the remainder of the notation used by Debian seems not inapplicable for us. -- let name="cbbrowne" and tld="gmail.com" in String.concat "@" [name;tld];; http://linuxdatabases.info/info/languages.html Signs of a Klingon Programmer - 4. "You cannot really appreciate Dilbert unless you've read it in the original Klingon." -- 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] why two dashes in extension load files
2011/2/14 Tom Lane : > =?ISO-8859-1?Q?C=E9dric_Villemain?= > writes: >> why do we care if there is a dash in the middle of a text where there >> are no numbers ? > > Umm ... we are not requiring version names to be numbers. good point I was believing we had something like multi-name-1.2.3-5.6.7 at a maximum. > > 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 > -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et 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] why two dashes in extension load files
=?ISO-8859-1?Q?C=E9dric_Villemain?= writes: > why do we care if there is a dash in the middle of a text where there > are no numbers ? Umm ... we are not requiring version names to be numbers. 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] pl/python do not delete function arguments
On ons, 2011-02-09 at 10:02 +0100, Jan Urbański wrote: > On 09/02/11 04:52, Hitoshi Harada wrote: > > 2010/12/31 Jan Urbański : > >> (continuing the flurry of patches) > >> > >> Here's a patch that stops PL/Python from removing the function's > >> arguments from its globals dict after calling it. It's > >> an incremental patch on top of the plpython-refactor patch sent in > >> http://archives.postgresql.org/message-id/4d135170.3080...@wulczer.org. > >> > >> Git branch for this patch: > >> https://github.com/wulczer/postgres/tree/dont-remove-arguments > >> > >> Apart from being useless, as the whole dict is unreffed and thus freed > >> in PLy_procedure_delete, removing args actively breaks things for > >> recursive invocation of the same function. The recursive callee after > >> returning will remove the args from globals, and subsequent access to > >> the arguments in the caller will cause a NameError (see new regression > >> test in patch). > > > > I've reviewed this. The patch is old enough to be rejected by patch > > command, but I manged to apply it by hand. > > It compiles clean. Added tests pass. > > I created fibonacci function similar to recursion_test in the patch > > and confirmed the recursion raises error on 9.0 but not on 9.1. > > Doc is not with the patch since this change is to remove unnecessary > > optimization internally. > > > > "Ready for Committer" > > Thanks, > > patch merged with HEAD attached. Curiously, without the patch the recursion_test(4) call fails but recursion_test(5) passes. Anyone know why? Btw., I get a KeyError, not a NameError as you say above. -- 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] CommitFest 2011-01 as of 2011-02-04
Robert Haas writes: > We have committed 45 patches and returned with feedback or rejected > 23. There are 30 remaining patches, every single one of which has > been reviewed. 20 of those are marked Ready for Committer; 5 are > marked Waiting on Author; 5 are marked Needs Review. However, again, I just took the liberty to mark the 2 extension patches as commited, even if we have some more "details" to fix. Well if we include the PL stuff, it's detail and a half, but I though marking them commited would help us following here. I hope it does. Will sleep on that. Now. :) I would like to see this commit fest extended by at least a week, maybe two, like has been proposed before. The overall rhythm has been quite impressive, and I'm not seeing such a slowdown that it's useless to try continuing. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] pageinspect's infomask and infomask2 as smallint
Thanks to Noah Misch's review of the keylock patch I noticed that pageinspect's heap_page_items(bytea) function returns infomask and infomask2 as smallint (signed). But the fields in the tuple header are 16 bits unsigned, so if the high (16th) bit is set, it returns negative values which seem hard to handle. Not a problem for infomask, because the high bit is used for a VACUUM FULL-era flag; but in infomask2 it is used. This seems hard to fix for existing installations with the unpackaged module already loaded -- IIRC it's not acceptable to drop a function, which is what would need to be done here. I report this because it seems the first case to test upgradability of a module. -- Á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] why two dashes in extension load files
2011/2/14 Tom Lane : > "David E. Wheeler" writes: >> On Feb 14, 2011, at 8:54 AM, Tom Lane wrote: I'm not convinced. There was nothing in that discussion why any particular character would have to be allowed in a version number. > >>> Well, there's already a counterexample in the current contrib stuff: >>> uuid-ossp. We could rename that to uuid_ossp of course, but it's >>> not clear to me that there's consensus for forbidding dashes here. why do we care if there is a dash in the middle of a text where there are no numbers ? > >> I'd be fine if commas were used instead. > > Commas do not seem like an improvement to me at all --- they are widely > used as list separators. > > I guess the real question is what's Peter's concrete objection to the > double-dash method? I have to admit that I am a bit surprised by this -- stuff too. An objection might be completely non-technical, but advocacy : "what this funny new name convention those PostgreSQL folks did invent ?!" -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et 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] ALTER TYPE 2: skip already-provable no-work rewrites
Hi Stephen, Thanks for jumping in on this one. On Mon, Feb 14, 2011 at 01:12:21PM -0500, Stephen Frost wrote: > First question is- why do you use #ifdef USE_ASSERT_CHECKING ..? The other six code sites checking assert_enabled directly do the same. > assert_enabled exists and will work the way you expect regardless, no? Yes. > Strikes me as unlikely that the checks would be a real performance > problem.. Agreed. > In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a > passed-in argument to move through a list with.. I'd suggest using a > local variable that is set from what's passed in. I do see that's > inheirited, but still, you've pretty much redefined that entire > function anyway.. Could do that. However, the function would reference the original argument just once, to make that copy. I'm not seeing a win. > Also, I feel like that while(!tab->rewrite) really deserves more > explanation of what's happening than the function-level comment above > gives it. I'd prefer to see a comment above the while() explaining > that we're moving through a list to see if there's any level at which > expr is something complicated or is referring to a domain which has > constraints on it (presuming that I've followed what's going on > correctly, that is..). The way I like to think about it is that we're recursively walking an expression tree to determine which of three categories it falls into: always produces the same value without error; always produces the same value on success, but may throw an error; neither of the above. We have a whitelist of node types that are acceptable, and anything else makes us assume the worst. (The nodes we accept are simple enough that the recursion degenerates to iteration.) Here's the comment explaining the algorithm, from an earlier version of the patch: + /* GetCoerceExemptions() + *Assess invariants of a coercion expression. + * + * Various common expressions arising from type coercion are subject to + * optimizations. For example, a simple varchar -> text cast will never change + * the underlying data (COERCE_EXEMPT_NOCHANGE) and never yield an error + * (COERCE_EXEMPT_NOERROR). A varchar(8) -> varchar(4) will never change the + * data, but it may yield an error. Given a varno and varattno denoting "the" + * source datum, determine which invariants hold for an expression by walking it + * per these rules: + * + *1. A Var with the varno/varattno in question has both invariants. + *2. A RelabelType node inherits the invariants of its sole argument. + *3. A CoerceToDomain node inherits any COERCE_EXEMPT_NOCHANGE invariant from + *its sole argument. When GetDomainConstraints() == NIL, it also inherits + *COERCE_EXEMPT_NOERROR. Otherwise, COERCE_EXEMPT_NOERROR becomes false. + *4. All other nodes have neither invariant. + * + * Returns a bit string that may contain the following bits: + *COERCE_EXEMPT_NOCHANGE: expression result will always have the same binary + *representation as a Var expression having the given varno and + *varattno + *COERCE_EXEMPT_NOERROR: expression will never throw an error + */ The return value changed, but the rest remains accurate. Here's a similar explanation from the design thread; it covers a more general case: http://archives.postgresql.org/message-id/20101231013534.ga7...@tornado.leadboat.com Should I restore some of that? Any other particular text that would have helped? > It also seems like it'd make more sense to me to > be a while() controlled by (IsA(expr, Var) && ((Var *) expr)->varattno > == varattno), since that's really the normal "stopping point". If we can optimize to some extent, that is the stopping point. If not, tab->rewrite is the stopping point. I picked the no-optimization case as "normal" and used that as the loop condition, but one could go either way. > These are all more stylistic issues than anything else. Last, but not > least, I do worry about how this may impact contrib modules, external > projects, or user-added data types, such as PostGIS, hstore, and ip4r. > Could we/should we limit this to only PG data types that we 'know' are > binary compatible? Is there any way or reason such external modules > could be fouled up by this? External modules are safe. If a binary coercion cast (CREATE CAST ... WITHOUT FUNCTION) implements the type conversion for an ALTER TABLE ... SET DATA TYPE, coerce_to_target_type() will inject a RelabelType node, and this code will pick up on that and avoid the rewrite. If such a cast were defined erroneously, the user is no less in trouble. He'd get a table rewrite, but the rewrite would just transfer the bits unchanged. The validity of this optimization does not rely on any user-settable property of a data type, but it does lean heavily on the execution semantics of specific nodes. Thanks, nm --
Re: [HACKERS] sepgsql contrib module
On 02/14/2011 11:47 AM, Kohei Kaigai wrote: We really need to get a buildfarm which is building with this. To that end, would you mind providing directions so someone else could set up a buildfarm member to test it..? It seems to me not difficult to describe a direction to build, install and run regression test. Do we have any Fedora 14 environment in the buildfarm? It is the most suitable distribution to set up sepgsql module, because the default installation already has selinux configurations. Thew makefile still has this bogosity: sepgsql-regtest.pp: sepgsql-regtest.te $(MAKE) -f $(DESTDIR)/usr/share/selinux/devel/Makefile $@ We need to fix that up before we even think of trying to get buildfarm coverage. The presence and location of this makefile should be determined at configure time, ISTM. 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] SSI bug?
YAMAMOTO Takashi wrote: >> Did you notice whether the loop involved multiple tuples within a >> single page? > > if i understand correctly, yes. > > the following is a snippet of my debug code (dump targets when > triggerCheckTargetForConflictsIn loops >1000 times) and its > output.the same locktag_field3 value means the same page, right? Right. > the table seems mostly hot-updated, if it matters. > idx_scan | 53681 > idx_tup_fetch | 52253 > n_tup_ins | 569 > n_tup_upd | 12054 > n_tup_del | 476 > n_tup_hot_upd | 12041 > n_live_tup| 93 > n_dead_tup| 559 That probably matters a lot. > analyze_count | 4922528128875102208 > autoanalyze_count | 7598807461784802080 > > (values in the last two columns seems bogus. > i don't know if it's related or not.) That seems unlikely to be related to this problem. It sure does look odd, though. Maybe post that in a separate thread? Thanks for all the additional info. I'll keep digging. -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] sepgsql contrib module
Excerpts from Kohei Kaigai's message of lun feb 14 13:47:58 -0300 2011: > > We really need to get a buildfarm which is building with this. To that > > end, would you mind providing directions so someone else could set up a > > buildfarm member to test it..? > > It seems to me not difficult to describe a direction to build, install and > run regression test. > Do we have any Fedora 14 environment in the buildfarm? > It is the most suitable distribution to set up sepgsql module, because the > default installation already has selinux configurations. It would be good to fix the Makefile problem that prevents the code to build elsewhere. There's another thread about a hardcoded path to a system Makefile in contrib/sepgsql that causes that module to FTBFS. Is there a way to fix that? I had a look some days ago and it didn't seem like there was a way to query the system for the proper path to use. -- Á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] pg_terminate_backend and pg_cancel_backend by not administrator user
Torello Querci wrote: > I attach a path for this It's too late in the release cycle to consider this for version 9.1. Please add it to the open CommitFest for consideration for 9.2: https://commitfest.postgresql.org/action/commitfest_view/open -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] pika failing since the per-column collation patch
Le 12 févr. 2011 à 18:51, Peter Eisentraut a écrit : > On lör, 2011-02-12 at 13:34 +0100, Rémi Zara wrote: >> Since the per-column collation patch went in, pika (NetBSD 5.1/mips) started >> failing consistently with this diff: >> >> *** >> /home/pgbuildfarm/workdir/HEAD/pgsql.15101/src/test/regress/expected/polymorphism.out >> Sat Feb 12 02:16:07 2011 >> --- >> /home/pgbuildfarm/workdir/HEAD/pgsql.15101/src/test/regress/results/polymorphism.out >> Sat Feb 12 09:10:21 2011 >> *** >> *** 624,630 >> >> -- such functions must protect themselves if varying element type isn't OK >> select max(histogram_bounds) from pg_stats; >> ! ERROR: cannot compare arrays of different element types >> -- test variadic polymorphic functions >> create function myleast(variadic anyarray) returns anyelement as $$ >>select min($1[i]) from generate_subscripts($1,1) g(i) >> --- 624,630 >> >> -- such functions must protect themselves if varying element type isn't OK >> select max(histogram_bounds) from pg_stats; >> ! ERROR: locale operation to be invoked, but no collation was derived >> -- test variadic polymorphic functions >> create function myleast(variadic anyarray) returns anyelement as $$ >>select min($1[i]) from generate_subscripts($1,1) g(i) >> >> Is there something I can do to help investigate this ? > > It's only failing on this one machine, but there isn't anything > platform-specific in this code, so I'd look for memory management faults > on the code or a compiler problem. Try with lower optimization for a > start. > Same failure with -O0 (and more shared memory). Regards, Rémi Zara -- 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] ALTER TYPE 2: skip already-provable no-work rewrites
Noah, I'm even less familiar w/ this code than Robert, but figured I'd give a shot at reviewing this anyway. I definitely like avoiding table rewrites if I can get away with it. :) First question is- why do you use #ifdef USE_ASSERT_CHECKING ..? assert_enabled exists and will work the way you expect regardless, no? Strikes me as unlikely that the checks would be a real performance problem.. In ATColumnChangeSetWorkLevel(), I'm really not a huge fan of using a passed-in argument to move through a list with.. I'd suggest using a local variable that is set from what's passed in. I do see that's inheirited, but still, you've pretty much redefined that entire function anyway.. Also, I feel like that while(!tab->rewrite) really deserves more explanation of what's happening than the function-level comment above gives it. I'd prefer to see a comment above the while() explaining that we're moving through a list to see if there's any level at which expr is something complicated or is referring to a domain which has constraints on it (presuming that I've followed what's going on correctly, that is..). It also seems like it'd make more sense to me to be a while() controlled by (IsA(expr, Var) && ((Var *) expr)->varattno == varattno), since that's really the normal "stopping point". These are all more stylistic issues than anything else. Last, but not least, I do worry about how this may impact contrib modules, external projects, or user-added data types, such as PostGIS, hstore, and ip4r. Could we/should we limit this to only PG data types that we 'know' are binary compatible? Is there any way or reason such external modules could be fouled up by this? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] SSI bug?
Heikki Linnakangas wrote: > Looking at the prior/next version chaining, aside from the > looping issue, isn't it broken by lock promotion too? There's a > check in RemoveTargetIfNoLongerUsed() so that we don't release a > lock target if its priorVersionOfRow is set, but what if the tuple > lock is promoted to a page level lock first, and > PredicateLockTupleRowVersionLink() is called only after that? Or > can that not happen because of something else that I'm missing? I had to ponder that a while. Here's my thinking. Predicate locks only matter when there is a write. Predicate locks on heap tuples only matter when there is an UPDATE or DELETE of a locked tuple. The problem these links are addressing is that an intervening transaction might UPDATE the transaction between the read of the tuple and a later UPDATE or DELETE. We want the second UPDATE to see that it conflicts with a read from before the first UPDATE. The first UPDATE creates the link from the "before" tuple ID the "after" tuple ID at the target level. What predicate locks exist on the second target are irrelevant when it comes to seeing the conflict between the second UPDATE (or DELETE) and the initial read. So I don't see where granularity promotion for locks on the second target is a problem as long as the target itself doesn't get deleted because of the link to the prior version of the tuple. Promotion of the lock granularity on the prior tuple is where we have problems. If the two tuple versions are in separate pages then the second UPDATE could miss the conflict. My first thought was to fix that by requiring promotion of a predicate lock on a tuple to jump straight to the relation level if nextVersionOfRow is set for the lock target and it points to a tuple in a different page. But that doesn't cover a situation where we have a heap tuple predicate lock which gets promoted to page granularity before the tuple is updated. To handle that we would need to say that an UPDATE to a tuple on a page which is predicate locked by the transaction would need to be promoted to relation granularity if the new version of the tuple wasn't on the same page as the old version. That's all doable without too much trouble, but more than I'm likely to get done today. It would be good if someone can confirm my thinking on this first, too. That said, the above is about eliminating false negatives from some corner cases which escaped notice until now. I don't think the changes described above will do anything to prevent the problems reported by YAMAMOTO Takashi. Unless I'm missing something, it sounds like tuple IDs are being changed or reused while predicate locks are held on the tuples. That's probably not going to be overwhelmingly hard to fix if we can identify how that can happen. I tried to cover HOT issues, but it seems likely I missed something. :-( I will be looking at it. -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] sepgsql contrib module
> We really need to get a buildfarm which is building with this. To that > end, would you mind providing directions so someone else could set up a > buildfarm member to test it..? It seems to me not difficult to describe a direction to build, install and run regression test. Do we have any Fedora 14 environment in the buildfarm? It is the most suitable distribution to set up sepgsql module, because the default installation already has selinux configurations. Thanks, > -Original Message- > From: Stephen Frost [mailto:sfr...@snowman.net] > Sent: 14 February 2011 16:29 > To: Kohei Kaigai > Cc: Robert Haas; KaiGai Kohei; PgHacker > Subject: Re: [HACKERS] sepgsql contrib module > > KaiGai, > > * Kohei Kaigai (kohei.kai...@eu.nec.com) wrote: > > > It would be good to have some buildfarm coverage of this code. Can > > > we find anyone brave enough to set up a buildfarm critter using > > > --with-selinux? > > > > > Although I don't have an account on the buildfarm, I'll set up an > > environment for daily build with --with-selinux. > > Because it needs kernel with selinux, libselinux and security policy, > > I think it is good idea to set up a virtual machine for the purpose. > > We really need to get a buildfarm which is building with this. To that > end, would you mind providing directions so someone else could set up a > buildfarm member to test it..? > > Thanks, > > Stephen -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane writes: > I don't really think that's a behavior we want to encourage. ISTM the > cases that are going to be trouble are paths you failed to think about, > and therefore what you want to do is look over the whole output set to > see if there are any surprising paths... Mmm, yes. Ok. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Debian readline/libedit breakage
* Florian Weimer (fwei...@bfk.de) wrote: > Source? I've only seen GPLed copies. We wouldn't face this issue > with LGPL code. Yeah, Greg corrected me on this already. So we have both FSF folks *and* OpenSSL people being foolish. Sigh. Stephen signature.asc Description: Digital signature
Re: [HACKERS] Debian readline/libedit breakage
* Stephen Frost: > * Greg Smith (g...@2ndquadrant.com) wrote: >> -GNU libreadine is certainly never going to add an OpenSSL exemption > > I really wish they would, that's just them being obnoxious- it's already > LGPL, after all.. Source? I've only seen GPLed copies. We wouldn't face this issue with LGPL code. -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine writes: > Tom Lane writes: >>> [ about omitting rows for which there is no update path ] >> Yeah, possibly. I'm a bit concerned about cases where the author meant >> to provide an update path and forgot: it would be fairly obvious in this >> representation but maybe you could keep making the same oversight if the >> row's not there at all. Also, it's easy enough to write "where path is >> not null" if you want to filter the rows that way. > I would expect the author to check with something like > WHERE installed = '1.0' and available = '1.2' I don't really think that's a behavior we want to encourage. ISTM the cases that are going to be trouble are paths you failed to think about, and therefore what you want to do is look over the whole output set to see if there are any surprising paths... 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] why two dashes in extension load files
On Feb 14, 2011, at 9:14 AM, Tom Lane wrote: > Commas do not seem like an improvement to me at all --- they are widely > used as list separators. Fair enough. > I guess the real question is what's Peter's concrete objection to the > double-dash method? Hey, I know, a double-dash between the extension name and first version, and -> between the first and second version: foo--1.2->1.4.sql ;-P David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why two dashes in extension load files
"David E. Wheeler" writes: > On Feb 14, 2011, at 8:54 AM, Tom Lane wrote: >>> I'm not convinced. There was nothing in that discussion why any >>> particular character would have to be allowed in a version number. >> Well, there's already a counterexample in the current contrib stuff: >> uuid-ossp. We could rename that to uuid_ossp of course, but it's >> not clear to me that there's consensus for forbidding dashes here. > I'd be fine if commas were used instead. Commas do not seem like an improvement to me at all --- they are widely used as list separators. I guess the real question is what's Peter's concrete objection to the double-dash method? 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] pg_ctl failover Re: Latches, signals, and waiting
Fujii, * Fujii Masao (masao.fu...@gmail.com) wrote: > Yeah, I rebased the patch to the current git master and attached it. Reviewing this, I just had a couple of comments and questions. Overall, I think it looks good and hence will be marking it 'Ready for Committer'. * You removed trigger_file from the list in doc/src/sgml/high-availability.sgml and I'm not sure I agree with that. It's still perfectly valid and could be used by someone instead of pg_ctl promote. I'd recommend two things: - Adding comments into this recovery.conf snippet - Adding a comment indicationg that trigger_file is only needed if you're not using pg_ctl promote. * I'm not happy that pg_ctl.c doesn't #include something which defines all the file names which are used, couldn't we use a header which makes sense and is pulled in by pg_ctl.c and xlog.c to #define all of these? Still, that's not really the fault of this patch. * I'm a bit worried that there's just only so many USR signals that we can send and it looks like we're burning another one here. Should we be considering a better way to do this? Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] why two dashes in extension load files
On Feb 14, 2011, at 8:54 AM, Tom Lane wrote: >> I'm not convinced. There was nothing in that discussion why any >> particular character would have to be allowed in a version number. > > Well, there's already a counterexample in the current contrib stuff: > uuid-ossp. We could rename that to uuid_ossp of course, but it's > not clear to me that there's consensus for forbidding dashes here. I'd be fine if commas were used instead. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane writes: > I intentionally left out columns that seem like extension implementation > details rather than things users of the extension need to know. Hence, > no directory, encoding, or module_pathname. There's no fundamental > reason not to include these, I guess, although maybe there could be some > security objection to showing directory. But do we need 'em? I share your view on the directory and module_pathname, but though that maybe encoding could be the source of subtle errors and that users would be happy to know what PostgreSQL is using. But well, that's not holding enough water now that I think some more about it. > I was thinking the other way --- you can split it with > regexp_split_to_array (or regexp_split_to_table) if you want to, but > having a compact human-readable form is probably the most important > case. It's not a big deal either way though. Anyone else want to > vote? I'm not set one way or the other and won't share another opinion on that :) > Sorry, I only meant that in this example I put the rows coming from > single scripts first. I didn't mean to suggest that the function would > guarantee any particular output ordering. Ok. > Yeah, possibly. I'm a bit concerned about cases where the author meant > to provide an update path and forgot: it would be fairly obvious in this > representation but maybe you could keep making the same oversight if the > row's not there at all. Also, it's easy enough to write "where path is > not null" if you want to filter the rows that way. I would expect the author to check with something like WHERE installed = '1.0' and available = '1.2' But again, the preference here is about either "cluttering" the default output more than necessary or having to type a WHERE clause to double check your setup. No strong opinion here, just a preference… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] why two dashes in extension load files
On Mon, Feb 14, 2011 at 6:49 PM, Peter Eisentraut wrote: > On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote: >> Peter Eisentraut writes: >> > Why do the extension load files need two dashes, like xml2--1.0.sql? >> > Why isn't one enough? >> >> Because we'd have to forbid dashes in extension name and version >> strings. This was judged to be a less annoying solution. See >> yesterday's discussion. > > I'm not convinced. There was nothing in that discussion why any > particular character would have to be allowed in a version number. I'd > propose that dashes should be prohibited in version names anyway, > because downstream packaging will want to use that to separate packaging > revisions. It might be better to discuss that explicitly rather than > hiding it in some thread of another title. I think the question is more - what do we disallow in package name? Eg. Debian disallows '_' and uses it as magic separator. It works, but it not as obvious as '-' vs '--', and '--' allows both '_' and '-' in package name. Unlikely anyone will want '--' in package name. I would vote for current '--' and keeping version name simple, no '_' and '-' there. As we want to do some logic on that. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why two dashes in extension load files
Peter Eisentraut writes: > On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote: >> Peter Eisentraut writes: >>> Why do the extension load files need two dashes, like xml2--1.0.sql? >>> Why isn't one enough? >> Because we'd have to forbid dashes in extension name and version >> strings. This was judged to be a less annoying solution. See >> yesterday's discussion. > I'm not convinced. There was nothing in that discussion why any > particular character would have to be allowed in a version number. Well, there's already a counterexample in the current contrib stuff: uuid-ossp. We could rename that to uuid_ossp of course, but it's not clear to me that there's consensus for forbidding dashes here. 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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine writes: > Tom Lane writes: >> and pg_available_extension_versions that produces a row per install >> script, with columns >> >> name >> version ((name, version) is primary key) >> comment >> requires >> relocatable >> schema >> >> where the last four columns can vary across versions due to secondary >> control files. > I like this primary key because that's also the one for debian stable > distributions :) Joking apart, aren't we missing the encoding somewhere? I intentionally left out columns that seem like extension implementation details rather than things users of the extension need to know. Hence, no directory, encoding, or module_pathname. There's no fundamental reason not to include these, I guess, although maybe there could be some security objection to showing directory. But do we need 'em? >> The output might look like this: >> >> 1.0 1.1 1.0--1.1 >> 1.1 1.2 1.1--1.2 >> unpackaged 1.0 unpackaged--1.0 >> 1.0 1.2 1.0--1.1--1.2 >> 1.0 unpackaged >> 1.1 1.0 >> 1.1 unpackaged >> 1.2 1.1 >> 1.2 1.0 >> 1.2 unpackaged >> unpackaged 1.1 unpackaged--1.0--1.1 >> unpackaged 1.2 unpackaged--1.0--1.1--1.2 > What about having this chain column be an array of version strings? If > you want to see it this way, use array_to_string(path, '--')⦠I was thinking the other way --- you can split it with regexp_split_to_array (or regexp_split_to_table) if you want to, but having a compact human-readable form is probably the most important case. It's not a big deal either way though. Anyone else want to vote? >> where the first three rows correspond to available update scripts and >> the rest are synthesized. > The ordering is not clearly apparent, but I don't think it matters. Sorry, I only meant that in this example I put the rows coming from single scripts first. I didn't mean to suggest that the function would guarantee any particular output ordering. >> (Looking at this, it looks like it could get pretty bulky pretty >> quickly. Maybe we should eliminate all rows in which the path would be >> NULL? Or just eliminate rows in which the target doesn't have an >> install script, which would remove the three rows with target = >> unpackaged in the above example?) > Removing NULL path rows seems the best option to me. Yeah, possibly. I'm a bit concerned about cases where the author meant to provide an update path and forgot: it would be fairly obvious in this representation but maybe you could keep making the same oversight if the row's not there at all. Also, it's easy enough to write "where path is not null" if you want to filter the rows 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] CommitFest 2011-01 as of 2011-02-04
* Robert Haas (robertmh...@gmail.com) wrote: > Here's where I think we are with this CommitFest. Subject: Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04 I'm gonna go out on a limb and hope you meant '2011-02-14' there. :) > So there are two basic difficulties with wrapping the CommitFest up. I have to say that I've always been a bit suprised by the idea that the CommitFest is intended to be done and all patches *committed* at the end of the month. It's been working really rather well, which is due in great part to the excellent CF managers (thanks again for being that, again). That said, we have quite a few non-committer reviewers who provide good feedback and move the patch back to 'waiting for author' and that whole process takes a while. Perhaps a thought for next time would be to offset things a bit. eg: CF 2011-03 (or whatever): 2011-02-14: Patches should all be submitted 2011-02-14: Reviewers start 2011-03-01: Committers start w/ 'Ready for Committer' patches 2011-03-14: Patches not marked 'Ready for Committer' get bounced 2011-03-31: All patches committed I'm not against the 'waiting on author' approach, but I do feel like if we're going to continue to have it, we need to spread it out a bit more. I do think this would place more work on the CF manager, unfortunately, but I'd hope that they would primairly be focused on managing the reviews and not be as busy during the last 2 weeks. Maybe one day I'll be brave enough to offer to manage one and see. :) Thanks again, Robert, you've done an excellent job managing the CF. Stephen signature.asc Description: Digital signature
Re: [HACKERS] why two dashes in extension load files
On mån, 2011-02-14 at 10:13 -0500, Tom Lane wrote: > Peter Eisentraut writes: > > Why do the extension load files need two dashes, like xml2--1.0.sql? > > Why isn't one enough? > > Because we'd have to forbid dashes in extension name and version > strings. This was judged to be a less annoying solution. See > yesterday's discussion. I'm not convinced. There was nothing in that discussion why any particular character would have to be allowed in a version number. I'd propose that dashes should be prohibited in version names anyway, because downstream packaging will want to use that to separate packaging revisions. It might be better to discuss that explicitly rather than hiding it in some thread of another title. Other comments? -- 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] using a lot of maintenance_work_mem
Frederik Ramm wrote: > I am (ab)using a PostgreSQL database (with PostGIS extension) in > a large data processing job - each day, I load several GB of data, > run a lot of analyses on it, and then throw everything away again. > Loading, running, and dumping the results takes about 18 hours > every day. > > The job involves a lot of index building and sorting, and is run > on a 64-bit machine with 96 GB of RAM. > > Naturally I would like the system to use as much RAM as possible > before resorting to disk-based operations, but no amount of > maintenance_work_mem setting seems to make it do my bidding. If you can tolerate some risk that for a given day you might fail to generate the analysis, or you might need to push the schedule back to get it, you could increase performance by compromising recoverability. You seem to be willing to consider such risk based on your mention of a RAM disk. - If a single session can be maintained for loading and using the data, you might be able to use temporary tables and a large temp_buffers size. Of course, when the connection closes, the tables are gone. - You could turn off fsync and full_page_writes, but on a crash your database might be corrupted beyond usability. - You could turn off synchronous_commit. - Make sure you have archiving turned off. - If you are not already doing so, load the data into each table within the same database transaction which does CREATE TABLE or TRUNCATE TABLE. Other than the possibility that the temp table might keep things in RAM, these suggestions don't directly address your question, but I thought they might be helpful. -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] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane writes: > Thinking about this some more ... it seems like we now need two separate > views, because there is some information that could change per-version, > and some that really only makes sense at the per-extension level. Makes sense. > For instance, we could have pg_available_extensions that produces a row > per primary control file, with columns > > name(view's effective primary key) > default_version > installed_version (NULL if not installed) > comment (if one is present in primary control file) Check. > and pg_available_extension_versions that produces a row per install > script, with columns > > name > version ((name, version) is primary key) > comment > requires > relocatable > schema > > where the last four columns can vary across versions due to secondary > control files. I like this primary key because that's also the one for debian stable distributions :) Joking apart, aren't we missing the encoding somewhere? > Or we could combine these into just one view with pkey (name, version), > but then the default_version and installed_version columns would be the > same across all rows with the same extension name, which seems confusing > and unnormalized. Let's go with two views. Once we have that it's easy enough to LEFT JOIN if we want a summarized view. Maybe we could even revive \dX. Without pattern it would show the short form (pg_available_extension) and given a pattern pg_available_extension_versions. > I suggest instead that we invent a SRF, say > pg_extension_update_paths(extension_name text) returns setof record, > that returns a row for each pair of distinct version names found in > the extension's install and update scripts, with columns Agreed. > source version name > target other version name > pathupdate path from source to target, or NULL if none > > The output might look like this: > > 1.0 1.1 1.0--1.1 > 1.1 1.2 1.1--1.2 > unpackaged 1.0 unpackaged--1.0 > 1.0 1.2 1.0--1.1--1.2 > 1.0 unpackaged > 1.1 1.0 > 1.1 unpackaged > 1.2 1.1 > 1.2 1.0 > 1.2 unpackaged > unpackaged 1.1 unpackaged--1.0--1.1 > unpackaged 1.2 unpackaged--1.0--1.1--1.2 What about having this chain column be an array of version strings? If you want to see it this way, use array_to_string(path, '--')… > where the first three rows correspond to available update scripts and > the rest are synthesized. The ordering is not clearly apparent, but I don't think it matters. > (Looking at this, it looks like it could get pretty bulky pretty > quickly. Maybe we should eliminate all rows in which the path would be > NULL? Or just eliminate rows in which the target doesn't have an > install script, which would remove the three rows with target = > unpackaged in the above example?) Removing NULL path rows seems the best option to me. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] "Extension" versus "module"
Dimitri Fontaine writes: > Another concern has to do with PLs. We said that with the dependency > mechanism it would be good to have PLs be EXTENSIONs. But those are > core provided extensions, one of them installed by default. > If we make PLs extensions, we might also want to have CREATE LANGUAGE > either ERROR out or silently do the CREATE EXTENSION instead, meaning > that CREATE LANGUAGE behavior would depend on creating_extension. > Sounds like a crock but ensures compatibility. Yeah. I was sort of wondering whether we could get rid of pg_pltemplate altogether, and instead rely on the extension mechanism to package up the correct parameters for installing a language. However, one thing that'd have to be solved before going very far in this direction is the question of allowing CREATE EXTENSION to non-superusers. We'd at least need to be able to duplicate the current functionality of allowing CREATE LANGUAGE to database owners (with an override available to the DBA). This seems like a matter for a separate thread though, and not on pgsql-docs. 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] sepgsql contrib module
KaiGai, * Kohei Kaigai (kohei.kai...@eu.nec.com) wrote: > > It would be good to have some buildfarm coverage of this code. Can we > > find anyone brave enough to set up a buildfarm critter using > > --with-selinux? > > > Although I don't have an account on the buildfarm, I'll set up an environment > for daily build with --with-selinux. > Because it needs kernel with selinux, libselinux and security policy, I think > it is good idea to set up a virtual machine for the purpose. We really need to get a buildfarm which is building with this. To that end, would you mind providing directions so someone else could set up a buildfarm member to test it..? Thanks, Stephen signature.asc Description: Digital signature
[HACKERS] CommitFest 2011-01 as of 2011-02-04
Here's where I think we are with this CommitFest. We have committed 45 patches and returned with feedback or rejected 23. There are 30 remaining patches, every single one of which has been reviewed. 20 of those are marked Ready for Committer; 5 are marked Waiting on Author; 5 are marked Needs Review. However, again, even the ones that are marked as Needs Review have in fact been reviewed multiple times, in many cases by multiple people. Thanks to all who have contributed to the reviewing effort, especially Stephen Frost, Hitoshi Hirada, Alex Hunsaker, Noah Misch, and Itagaki Takahiro. I respectfully submit that every patch which is not yet Ready for Committer is in that state not because of any neglect on the part of anyone in the community, but because it's got problems. It isn't done; it turned out to be more complicated than anticipated; it wasn't updated in a timely fashion; and/or we had difficulty reaching agreement on the best way forward (and still haven't). Most of these patches should probably be deferred to 9.2. So there are two basic difficulties with wrapping the CommitFest up. One is that the 20 patches which are marked Ready for Committer really deserve to be looked at by a committer, and hopefully committed. Unfortunately, my ability to help in this area will be somewhat limited, because at least half of the remaining patches are in areas that I know nothing about (e.g. 7 PL/python patches). The second problem is that the patches that are in serious trouble including synchronous replication and SQL/MED, which are key features I know we're all hoping to have for 9.2. However, we have to face the fact that getting these features in may involve quite a bit of delay. With respect to SQL/MED, Heikki tells me that he is working on the core patch, and I am hopeful that will be committed soon. However, file_fdw is in pretty serious trouble because (1) the copy API patch that it depends on still isn't committed and (2) it's going to be utterly broken if we don't do something about the client_encoding vs. file_encoding problem; there was a patch to do that in this CF, but we gave up on it. And postgresql_fdw was hacked up by Heikki but he didn't seem to have much confidence in it and no one else has reviewed his hacked-up version. With respect to synchronous replication, Dan Farina and I have extracted some of the less-intrusive bits of that patch. One of those changes (standby replies) is committed, though it seems to need a bit of fixup, and the other two are awaiting review. Simon also reported that he is working on this, but I'm not certain of the specifics. Based on the looking at that patch that I did so far, it certainly needs cleanup, and there may be some more serious architectural issues that need to be addressed also. -- 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] "Extension" versus "module"
Tom Lane writes: > Hmm ... but what of contrib "modules" that don't build shared libraries > at all --- pgbench and pg_upgrade for example? > > I think "shared library" is a perfectly fine term for that kind of > object, and we don't need an alias for it anyway. In my view, if there's no script, that is no SQL object to install in a database, then it's not an extension. I would think that we keep the directory named contrib for them. Then, an extension can be implemented partly with a module, coded in C, installed as a shared library. Another concern has to do with PLs. We said that with the dependency mechanism it would be good to have PLs be EXTENSIONs. But those are core provided extensions, one of them installed by default. If we make PLs extensions, we might also want to have CREATE LANGUAGE either ERROR out or silently do the CREATE EXTENSION instead, meaning that CREATE LANGUAGE behavior would depend on creating_extension. Sounds like a crock but ensures compatibility. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Range Types: empty ranges
Jan Wieck wrote: > Does ['15:15:00','15:15:00') make any more sense? Doesn't this > essentially mean > > >= '15:15:00' && < '15:15:00' > > which again doesn't include a single point on the time line? It defines a position in time with zero duration. Some of the graphics programming I've done in the past was based on a system where, at the pixel level, the coordinates referred to the boundaries *between* the pixels. If you were to have a number of horizontal bars, for example, the size of which you would adjust to represent fluctuating data, you might have an x coordinate of 20 for the left edge, and [20,30) would paint 10 pixels. I guess you *could* destroy and recreate the object when the number dropped to zero and came off it again; but the concept of [20,20) to draw zero pixels but maintain the positional anchor can be convenient. I see parallel concepts for some data domains in a database. -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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine writes: > Tom Lane writes: >> Also, I've been looking at the pg_available_extensions issue a bit. >> I don't yet have a proposal for exactly how we ought to redefine it, >> but I did notice that the existing code is terribly confused by >> secondary control files: it doesn't realize that they're not primary >> control files, so you get e.g. hstore and hstore-1.0 as separate >> listings. > I'd think that's it's a good idea if dealt with "correctly" because now > that ALTER EXTENSION UPDATE can deal with more than one target VERSION > I expect the view to show each available update here. Thinking about this some more ... it seems like we now need two separate views, because there is some information that could change per-version, and some that really only makes sense at the per-extension level. For instance, we could have pg_available_extensions that produces a row per primary control file, with columns name(view's effective primary key) default_version installed_version (NULL if not installed) comment (if one is present in primary control file) and pg_available_extension_versions that produces a row per install script, with columns name version ((name, version) is primary key) comment requires relocatable schema where the last four columns can vary across versions due to secondary control files. Or we could combine these into just one view with pkey (name, version), but then the default_version and installed_version columns would be the same across all rows with the same extension name, which seems confusing and unnormalized. > If possible adding the "update chain sequence" information as computed > in the code would be great. Because we can't ask people to figure that > out all by themselves, the best way to check your upgrading setup is > fine would be to run SELECT * FROM pg_available_extensions; and read the > result. I think this is probably a good thing to provide but it shouldn't go in either of the above views, on two grounds: (1) it's going to be relatively expensive to compute, and most people won't need it; (2) the views could only sensibly cover paths from current version to listed version, which isn't good enough. What an extension author actually wants to know is "have I introduced any undesirable update paths anywhere?" I suggest instead that we invent a SRF, say pg_extension_update_paths(extension_name text) returns setof record, that returns a row for each pair of distinct version names found in the extension's install and update scripts, with columns source version name target other version name pathupdate path from source to target, or NULL if none The output might look like this: 1.0 1.1 1.0--1.1 1.1 1.2 1.1--1.2 unpackaged 1.0 unpackaged--1.0 1.0 1.2 1.0--1.1--1.2 1.0 unpackaged 1.1 1.0 1.1 unpackaged 1.2 1.1 1.2 1.0 1.2 unpackaged unpackaged 1.1 unpackaged--1.0--1.1 unpackaged 1.2 unpackaged--1.0--1.1--1.2 where the first three rows correspond to available update scripts and the rest are synthesized. (Looking at this, it looks like it could get pretty bulky pretty quickly. Maybe we should eliminate all rows in which the path would be NULL? Or just eliminate rows in which the target doesn't have an install script, which would remove the three rows with target = unpackaged in the above example?) Thoughts? 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: empty ranges
On 2/11/2011 1:50 PM, Kevin Grittner wrote: Josh Berkus wrote: if I, in one of my applications, accidentally defined something as having the range '('15:15:00','15:15:00')', I would *want* the database to through an error and not accept it. I can agree with that, but I think that range '[15:15:00,15:15:00)' should be valid as a zero-length range between, for example, '[15:00:00,15:15:00)' and '[15:15:00,15:30:00)'. Does ['15:15:00','15:15:00') make any more sense? Doesn't this essentially mean >= '15:15:00' && < '15:15:00' which again doesn't include a single point on the time line? Jan -- Anyone who trades liberty for security deserves neither liberty nor security. -- Benjamin Franklin -- 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] mingw64
Den 2011-02-12 11:10 skrev Ralf Wildenhues: > Hello, and sorry for the delay, > > * Peter Rosin wrote on Sat, Jan 29, 2011 at 02:26:24PM CET: >> Or is plain 'ar' used somewhere instead of 'x86_64-w64-mingw32-ar'? > > Automake outputs 'AR = ar' in Makefile.in for rules creating old > libraries iff neither AC_PROG_LIBTOOL nor another method to define > AR correctly is used in configure.ac. > > So this issue concerns packages using Automake but not using Libtool. > > I figured with AM_PROG_AR eventually being needed anyway that would fix > this in one go ... > > A good workaround, as already mentioned, is to use this in configure.ac: > AC_CHECK_TOOL([AR], [ar], [false]) I just cannot understand why the workaround isn't always working in this case. There was a log posted with this in it (in http://archives.postgresql.org/pgsql-hackers/2011-01/msg02697.php): ... configure:5962: checking for x86_64-w64-mingw32-ranlib configure:5978: found /mingw/bin/x86_64-w64-mingw32-ranlib configure:5989: result: x86_64-w64-mingw32-ranlib configure:6055: checking for x86_64-w64-mingw32-strip configure:6071: found /mingw/bin/x86_64-w64-mingw32-strip configure:6082: result: x86_64-w64-mingw32-strip configure:6145: checking whether it is possible to strip libraries configure:6150: result: yes configure:6164: checking for x86_64-w64-mingw32-ar configure:6180: found /mingw/bin/x86_64-w64-mingw32-ar configure:6191: result: x86_64-w64-mingw32-ar configure:6257: checking for x86_64-w64-mingw32-dlltool configure:6273: found /mingw/bin/x86_64-w64-mingw32-dlltool configure:6284: result: x86_64-w64-mingw32-dlltool configure:6349: checking for x86_64-w64-mingw32-dllwrap configure:6365: found /mingw/bin/x86_64-w64-mingw32-dllwrap configure:6376: result: x86_64-w64-mingw32-dllwrap configure:6441: checking for x86_64-w64-mingw32-windres configure:6457: found /mingw/bin/x86_64-w64-mingw32-windres configure:6468: result: x86_64-w64-mingw32-windres ... Which seem to match this snippet from configure.in: ... AC_PROG_RANLIB PGAC_CHECK_STRIP AC_CHECK_TOOL(AR, ar, ar) if test "$PORTNAME" = "win32"; then AC_CHECK_TOOL(DLLTOOL, dlltool, dlltool) AC_CHECK_TOOL(DLLWRAP, dllwrap, dllwrap) AC_CHECK_TOOL(WINDRES, windres, windres) fi ... Sure, AC_CHECK_TOOL has under-quoted arguments and the last argument is 'ar' instead of 'false'. But that shouldn't really matter here. (Or does it?) Still, elsewhere in the thread there's a report about the wrong ar being used. (in http://archives.postgresql.org/pgsql-hackers/2011-01/msg02713.php) Sure, the configure log and the "wrong ar"-report are not from the same person, but the configure script should be the same for everybody (git log hints that this part of configure has been stable for a couple of years). It just doesn't add up. Cheers, Peter -- 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] sepgsql contrib module
Sorry for the late responding, because of my relocation. > It would be good to have some buildfarm coverage of this code. Can we > find anyone brave enough to set up a buildfarm critter using > --with-selinux? > Although I don't have an account on the buildfarm, I'll set up an environment for daily build with --with-selinux. Because it needs kernel with selinux, libselinux and security policy, I think it is good idea to set up a virtual machine for the purpose. Thanks, > -Original Message- > From: Robert Haas [mailto:robertmh...@gmail.com] > Sent: 03 February 2011 05:27 > To: KaiGai Kohei > Cc: KaiGai Kohei; PgHacker > Subject: Re: [HACKERS] sepgsql contrib module > > On Wed, Feb 2, 2011 at 11:43 PM, Robert Haas wrote: > > Committed. > > I did some more polishing of the documentation as well. > > It would be good to have some buildfarm coverage of this code. Can we > find anyone brave enough to set up a buildfarm critter using > --with-selinux? > > -- > 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] "Extension" versus "module"
Dimitri Fontaine writes: > Tom Lane writes: >> Appendix F (contrib.sgml and its subsidiary files) is pretty consistent >> about using "module" to refer to a contrib, uh, module. > I'm now thinking in those terms: the module is the shared object library > that the backend needs to dlopen(). The extension is the SQL level > object that wraps all its components. Hmm ... but what of contrib "modules" that don't build shared libraries at all --- pgbench and pg_upgrade for example? I think "shared library" is a perfectly fine term for that kind of object, and we don't need an alias for it anyway. 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] Scheduled maintenance affecting gitmaster
On Mon, Feb 14, 2011 at 10:39, Stefan Kaltenbrunner wrote: > On 02/14/2011 10:09 AM, Magnus Hagander wrote: >> On Mon, Feb 14, 2011 at 07:13, Stefan Kaltenbrunner >> wrote: >>> On 02/14/2011 01:27 AM, Tom Lane wrote: Magnus Hagander writes: > > Unfortunately, one of the worst-case scenarios appears to have > happened - a machine did not come back up after a reboot. > ... > We'll get back to you with more information as soon as we have it. I didn't see any followup to this? >>> >>> yeah - the hosting company managed to reboot the box for us which brought it >>> back to life in the middle of the night (with both magnus and me asleep). >> >> Indeed. But the good news is that once it came back up, the VM with >> the git server started ok :-) >> >> gitmaster seems to be responding as of now, is it safe to push? >>> >>> yes it is - however we will need to schedule another maintenance window soon >>> to finish the stuff we actually wanted to do. >> >> So, after some discussion with Stefan, we (well, I guess I) decided we >> should just go ahead and declare the maintenance window not closed >> yet, and finish off the upgrade right now :-) Given that the majority >> of our commits don't happen now, we'll hopefully have it done by the >> time the US folks wake up again. >> >> So, maintenance window again, starting now, and we'll let you know as >> soon as we're done. And we're definitely hoping for the machine to >> come back up properly this time :-) > > and it did not... We are trying to figure out what the actual problem > here really is because it seems to boot just fine when powercycled just > not with a software initiated reboot. > We will notify once we have more information... Status update on this - Stefan is currently working with the datacenter people on getting this fixed (they are now available on-site), since we are now having an actual issue with the machine (GRUB failure on boot) rather than just a failure to shut down. -- 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] why two dashes in extension load files
Peter Eisentraut writes: > Why do the extension load files need two dashes, like xml2--1.0.sql? > Why isn't one enough? Because we'd have to forbid dashes in extension name and version strings. This was judged to be a less annoying solution. See yesterday's discussion. 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] using a lot of maintenance_work_mem
Frederik Ramm writes: > Now I assume that there are reasons that you're doing this. memutils.h > has the (for me) cryptic comment about MaxAllocSize: "XXX This is > deliberately chosen to correspond to the limiting size of varlena > objects under TOAST. See VARATT_MASK_SIZE in postgres.h.", but > VARATT_MASK_SIZE has zero other occurences in the source code. Hm, I guess that comment needs updated then. > If I were to either (a) increase MaxAllocSize to, say, 48 GB instead of > 1 GB, or (b) hack tuplesort.c to ignore MaxAllocSize, just for my local > setup - would that likely be viable in my situation, or would I break > countless things? You would break countless things. It might be okay anyway in a trusted environment, ie, one without users trying to crash the system, but there are a lot of security-critical implications of that test. If we were actually trying to support such large allocations, what I'd be inclined to do is introduce a separate call along the lines of MemoryContextAllocLarge() that lacks the safety check. But before expending time on that, I'd want to see some evidence that it's actually helpful for production situations. I'm a bit dubious that you're going to gain much here. 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] using a lot of maintenance_work_mem
Hi, I am (ab)using a PostgreSQL database (with PostGIS extension) in a large data processing job - each day, I load several GB of data, run a lot of analyses on it, and then throw everything away again. Loading, running, and dumping the results takes about 18 hours every day. The job involves a lot of index building and sorting, and is run on a 64-bit machine with 96 GB of RAM. Naturally I would like the system to use as much RAM as possible before resorting to disk-based operations, but no amount of maintenance_work_mem setting seems to make it do my bidding. I'm using PostgreSQL 8.3 but would be willing and able to upgrade to any later version. Some googling has unearthed the issue - which is likely known to all of you, just repeating it to prove I've done my homework - that tuplesort.c always tries to double its memory allocation, and will refuse to do so if that results in an allocation greater than MaxAllocSize: if ((Size) (state->memtupsize * 2) >= MaxAllocSize / sizeof(SortTuple)) return false; And MaxAllocSize is hardcoded to 1 GB in memutils.h. (All this based on Postgres 9.1alpha source - I didn't want to bring something up that has been fixed already.) Now I assume that there are reasons that you're doing this. memutils.h has the (for me) cryptic comment about MaxAllocSize: "XXX This is deliberately chosen to correspond to the limiting size of varlena objects under TOAST. See VARATT_MASK_SIZE in postgres.h.", but VARATT_MASK_SIZE has zero other occurences in the source code. If I were to either (a) increase MaxAllocSize to, say, 48 GB instead of 1 GB, or (b) hack tuplesort.c to ignore MaxAllocSize, just for my local setup - would that likely be viable in my situation, or would I break countless things? I can afford some experimentation; as I said, I'm throwing away the database every day anyway. I just thought I'd solicit your advice before I do anything super stupid. - If I can use my setup to somehow contribute to further PostgreSQL development by trying out some things, I'll be more than happy to do so. I do C/C++ but apart from building packages for several platforms, I haven't worked with the PostgreSQL source code. Of course the cop-out solution would be to just create a huge RAM disk and instruct PostgreSQL to use that for disk-based sorting. I'll do that if all of you say "OMG don't touch MaxAllocSize" ;) Bye Frederik -- Frederik Ramm ## eMail frede...@remote.org ## N49°00'09" E008°23'33" -- 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] Add support for logging the current role
Itagaki, * Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: > We need to design csvlog_header more carefully. csvlog_header won't work > if log_filename is low-resolution, ex. log-per-day. This isn't any different a problem to the issue of someone changing the csvlog_fields GUC but not checking if the log file already exists on restart. I've suggested a number of different options, but none of them are terribly good, and I havn't heard anyone supporting any solution to this issue. > It's still useful when > a DBA reads the file manually, but documentation would better. Eh? If you mean that we should add documentation to make users aware of the possible issue of changing these values without making sure the log file gets rotated- sure, I'd be happy to do that. > Or, should we skip writing headers when the open log file is not > empty? This doesn't help the csvlog_fields issue, unfortunately. I don't think it'd be hard to implement this to help with the header issue, I'm just not sure if it makes sense to do so when the actual list of fields could change... > * It might be my misunderstanding, but there was a short description for %U > for in log_line_prefix in postgresql.conf, right? Did you remove it in the > latest version? No, and I don't see where I ever added it.. I've fixed it. > * In assign_csvlog_fields(), we need to cleanup memory and memory context > before return on error. > + /* check if no option matched, and if so, return error */ > + if (curr_option == MAX_CSVLOG_OPTS) > + return NULL; Fixed this and a couple of similar issues. > * An added needless "return" should be removed. Meh, I like explicit returns, but since it generated a hunk all by itself, I'll clear it out. Updated patch attached, git log below. Thanks, Stephen commit 304e35ebb74f68da69163ed9dd1dd453b67181e7 Author: Stephen Frost Date: Mon Feb 14 09:26:03 2011 -0500 csvlog_fields: fix leak, other cleanup Fix a couple of potential memory leaks in assign_csvlog_fields(). Also added a few comments, removed an extra 'return;', and added %U to the sample postgresql.conf. commit 592c2564ffde77fc29ff28fdedd2c9f2dafd Merge: 33639eb cebbaa1 Author: Stephen Frost Date: Sun Feb 13 21:11:44 2011 -0500 Merge branch 'master' of git://git.postgresql.org/git/postgresql into log_csv_options commit 33639ebfe67b0dd58a0a89161e9f0d5237830ed4 Author: Stephen Frost Date: Sun Feb 13 21:08:08 2011 -0500 Add csvlog_header GUC, other cleanup This patch adds a csvlog_header option which will start each CSV log file with a header which matches the GUC (and hence the format of the CSV log file generated). Numerous other whitespace clean-ups, removed build_default_csvlog_list(), since it wasn't actually necessary or useful. Added an array which lists the text strings of the various CSVLOG options to simplify assign_csvlog_fields(). commit 6bd2b9f1d2bc3b166a3e5598ee590e25159c61a5 Author: Stephen Frost Date: Fri Feb 11 11:16:17 2011 -0500 Rename log_csv_fields GUC to csvlog_fields This patch renames the log_csv_fileds GUC to csvlog_fields, to better match the other csvlog_* options. Also cleaned up the CSV generation code a bit by moving the comma-adding code out of the switch() statement. commit a281ca611e6181339e92b488c815e0cb8c1298d2 Merge: d81 183d3cf Author: Stephen Frost Date: Fri Feb 11 08:37:27 2011 -0500 Merge branch 'master' of git://git.postgresql.org/git/postgresql into log_csv_options commit d81c425a4c320540769084ceeb7d23bc3662 Author: Stephen Frost Date: Sun Feb 6 14:02:05 2011 -0500 Change log_csv_options listing to a table This patch changes the listing of field options available to log_csv_options into a table, which will hopefully both look better and be clearer. commit f9851cdfaeb931f01c015f5651b72d16957c7114 Merge: 3e71e33 5ed45ac Author: Stephen Frost Date: Sun Feb 6 13:26:17 2011 -0500 Merge branch 'master' of git://git.postgresql.org/git/postgresql into log_csv_options commit 3e71e338a2b9352d730f59a989027e33d99bea50 Author: Stephen Frost Date: Fri Jan 28 22:44:33 2011 -0500 Cleanup log_csv_options patch Clean up of various function declarations to hopefully be correct and clean and matching PG conventions. Also move TopMemoryContext usage to later, since the local variables don't need to be in TopMemoryContext. Lastly, ensure that a comma is not produced after the last CSV field, and that one is produced if application_name is not the last field. Review by Itagaki Takahiro, thanks! commit 1825def11badd661d219fa4c516f06e0ad423443 Merge: ff249ae 847e8c7 Author: Stephen Frost Date: Wed Jan 19 06:50:03 2011 -0500 Merge branch 'master' of git://git.postgresql.org/git/postgresql into log_csv_options commit ff249aeac7216da623