Re: [HACKERS] SQL/MED - file_fdw
On Tue, Feb 8, 2011 at 00:30, Shigeru HANADA wrote: > Fixed version is attached. I reviewed your latest git version, that is a bit newer than the attached patch. http://git.postgresql.org/gitweb?p=users/hanada/postgres.git;a=commit;h=0e1a1e1b0e168cb3d8ff4d637747d0ba8f7b8d55 The code still works with small adjustment, but needs to be rebased on the latest master, especially for extension support and copy API changes. Here are a list of comments and suggestions: * You might forget some combination or unspecified options in file_fdw_validator(). For example, format == NULL or !csv && header cases. I've not tested all cases, but please recheck validations used in BeginCopy(). * estimate_costs() needs own row estimation rather than using baserel. > > What value does baserel->tuples have? > > Foreign tables are never analyzed for now. Is the number correct? > No, baserel->tuples is always 0, and baserel->pages is 0 too. > In addition, width and rows are set to 100 and 1, as default values. It means baserel is not reliable at all, right? If so, we need alternative solution in estimate_costs(). We adjust statistics with runtime relation size in estimate_rel_size(). Also, we use get_rel_data_width() for not analyzed tables. We could use similar technique in file_fdw, too. * Should use int64 for file byte size (or, uint32 in blocks). unsigned long might be 32bit. ulong is not portable. * Oid List (list_make1_oid) might be more handy than Const to hold relid in FdwPlan.fdw_private. * I prefer FileFdwExecutionState to FileFdwPrivate, because there are two different 'fdw_private' variables in FdwPlan and FdwExecutionState. * The comment in fileIterate seems wrong. It should be /* Store the next tuple as a virtual tuple. */ , right? * #include is needed. -- 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] XMin Hot Standby Feedback patch
On Wed, 2011-02-16 at 11:25 +0900, Fujii Masao wrote: > On Wed, Feb 16, 2011 at 6:15 AM, Simon Riggs wrote: > > On Tue, 2011-02-15 at 12:15 -0500, Robert Haas wrote: > >> Looks pretty good to me, though I haven't tested it. I like some of > >> the safety valves you put in there, but I don't understand this part > > > > Reworked logic covering all feedback, plus tests, plus docs. > > > > Last comments before commit please. > > When I started the standby with hot_standby = off and hot_standby_feedback = > on, > I got the following assertion error. > > - > sby LOG: streaming replication successfully connected to primary > TRAP: FailedAssertion("!(((result) >= ((TransactionId) 3)))", File: > "procarray.c", Line: 1027) > act LOG: unexpected EOF on standby connection > sby LOG: WAL receiver process (PID 17572) was terminated by signal 6: Aborted > sby LOG: terminating any other active server processes > - Thanks > vacuum_defer_cleanup_age on the *standby* should not affect the > feedback xid. Not sure, will think some more. > VACUUM TABLE on the *primary* doesn't use the feedback xid at all. > Is this intentional? Yes, I was in the middle of fixing that. -- 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] updated patch for foreach stmt
Stephen Frost writes: > * Pavel Stehule (pavel.steh...@gmail.com) wrote: >> There is only bad keywords in doc - SCALE instead SLICE and a maybe a >> usage of slicing need a example. > Err, yeah, a couple of stupid documentation issues, sorry about that. Applied with assorted cleanup. I left the syntax as-is, since that seems to be the plurality position at the moment. 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] Change pg_last_xlog_receive_location not to move backwards
On Tue, Feb 15, 2011 at 9:41 PM, Robert Haas wrote: > On Tue, Feb 15, 2011 at 12:34 AM, Fujii Masao wrote: >> 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. > > Well, my proposal was to expose it, on the theory that it's useful. > As we stream the WAL, we write it, so I think for all intents and > purposes write == stream. But using it to convey the starting > position makes more sense if you call it stream than it does if you > call it write. Umm.. I could not find any use case to expose the WAL write location besides flush one. So I'm not sure if it's really useful to track the write location in the shmem besides the walreceiver-local memory. What use case do you think of? Personally the term "stream" sounds more ambiguous than "write". I cannot imagine what location the pg_last_xlog_stream_location or stream_location actually returns, from the function name; WAL location that has been received? written? flushed? replayed? Since the "write" sounds cleaner, I like it. 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] updated patch for foreach stmt
2011/2/16 Tom Lane : > Andrew Dunstan writes: >> On 02/15/2011 08:59 PM, Robert Haas wrote: >>> Anyhoo, forcing the explicit ARRAY keyword in there seems like pretty >>> cheap future-proofing to me. YMMV. > >> If this is the syntax that makes you do things like: >> FOREACH foo IN ARRAY ARRAY[1,2,3] >> I have to say I find that pretty darn ugly still. > > Yeah, that was the argument against requiring ARRAY. So it comes down > to whether you think we need future-proofing here. I can't immediately > see any reason for us to need a keyword right there, but ... the combination of two keywords isn't nice, but we can ensure so result of expression will has a requested type. It's more verbose, it's more secure. We can to check a allowed keywords like SCALING in compile time, we can use a more keywords - A hash type can need a separation between KEY and VALUE - so any keyword there enables a higher possibilities in future. We can do it without a auxiliary keyword too, but parser will be more complex. Regards Pavel Stehule > > 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] updated patch for foreach stmt
Andrew Dunstan writes: > On 02/15/2011 08:59 PM, Robert Haas wrote: >> Anyhoo, forcing the explicit ARRAY keyword in there seems like pretty >> cheap future-proofing to me. YMMV. > If this is the syntax that makes you do things like: > FOREACH foo IN ARRAY ARRAY[1,2,3] > I have to say I find that pretty darn ugly still. Yeah, that was the argument against requiring ARRAY. So it comes down to whether you think we need future-proofing here. I can't immediately see any reason for us to need a keyword right there, but ... 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: pg_ctl failover Re: [HACKERS] Latches, signals, and waiting
On Wed, Feb 16, 2011 at 11:30 AM, Robert Haas wrote: > Committed with minor tweaks to comments and documentation. Thanks a lot! 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] why two dashes in extension load files
On Tue, Feb 15, 2011 at 14:12, Robert Haas wrote: > On Tue, Feb 15, 2011 at 3:26 PM, marcin mank wrote: >> how about : we use a single dash as the separator, and if the >> extension author insists on having a dash in the name, as a punishment >> he must duplicate the dash, i.e.: >> uuid--ossp-1.0--5.5.sql > > That has a certain poetic justice to it. Im not sure I see the poetic justice in trying to punish others for *our* arbitrary naming rules. *shrug* -- Sent 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 09:49, 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. :) > >> So there are two basic difficulties with wrapping the CommitFest up. > > [ ... ] It's been working really rather well, which is due in > great part to the excellent CF managers (thanks again for being that, > again). [ ... ] > Thanks again, Robert, you've done an excellent job managing the CF. I'd like to second this sentiment. I'm positive no one can really appreciate the amount of work that goes into managing a commitfest other than the elite few who have. I imagine its not unlike being a mother: constantly bitching, ( out of necessity mind you-- don't commit that! its HOT!, did you finish all of your reviews? no? there are starving people in china! etc.) and yet underrated, under-thanked and often undervalued. Anyway, good job and thanks for volunteering to be the "bad" guy that tries to keep the 9.1 time table sane. -- Sent 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 Wed, Feb 16, 2011 at 2:08 AM, Robert Haas wrote: > On Mon, Feb 14, 2011 at 12:25 AM, Fujii Masao wrote: >> On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas >> wrote: >>> I added a XLogWalRcvSendReply() call into XLogWalRcvFlush() so that it also >>> sends a status update every time the WAL is flushed. If the walreceiver is >>> busy receiving and flushing, that would happen once per WAL segment, which >>> seems sensible. >> >> This change can make the callback function "WalRcvDie()" call ereport(ERROR) >> via XLogWalRcvFlush(). This looks unsafe. > > Good catch. Is the cleanest solution to pass a boolean parameter to > XLogWalRcvFlush() indicating whether we're in the midst of dying? Agreed if the comment about why such a boolean parameter is required is added. 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] updated patch for foreach stmt
On 02/15/2011 08:59 PM, Robert Haas wrote: On Tue, Feb 15, 2011 at 8:44 PM, Tom Lane wrote: Robert Haas writes: On Tue, Feb 8, 2011 at 3:26 PM, Stephen Frost wrote: Alright, so, like I said, I really like this feature and would like to see it included. Amen to that! I think the syntax Tom suggested before was FOREACH thingy IN ARRAY arr rather than just FOREACH thingy IN arr. Actually, I'm on record as saying the opposite: we shouldn't need to distinguish the exact data type at the syntax level, so long as the FOREACH construct is understood to mean "iterate through the members of the composite object produced by this expression": http://archives.postgresql.org/pgsql-hackers/2010-12/msg01579.php I am not, however, wedded to that position --- if people are happier with explicit use of ARRAY here, I won't fight hard to get rid of it. Anyway I'm going to start on this patch next, so last chance for opinions about the syntax ... Oh, I was looking at this one: http://archives.postgresql.org/pgsql-hackers/2010-12/msg01557.php Anyhoo, forcing the explicit ARRAY keyword in there seems like pretty cheap future-proofing to me. YMMV. If this is the syntax that makes you do things like: FOREACH foo IN ARRAY ARRAY[1,2,3] I have to say I find that pretty darn ugly still. 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: 9.1 (git head) does not compile using --with-libedit-preferred on Ubuntu 10.10
On 16/02/11 15:59, Greg Stark wrote: On Wed, Feb 16, 2011 at 2:43 AM, Mark Kirkwood wrote: What's this libbsd then eh? Sure enough it is this guy that defines these symbols. So it is the way it is being built on the Ubuntu (or Debian) platform. Oh, for what it's worth there are several different libedits out there with various related heritages. It's possible you're looking at two completely different packages. On Debian /usr/share/doc//README.Debian is supposed to say where the upstream source was. Yeah, good point: $ dpkg -S /usr/lib/libedit.so.2 libedit2: /usr/lib/libedit.so.2 $ aptitude show libedit2 Package: libedit2 State: installed Automatically installed: no Version: 2.11-20080614-1build1 Priority: standard Section: libs Maintainer: Ubuntu Developers Uncompressed Size: 201k Depends: libbsd0 (>= 0.0), libc6 (>= 2.11), libncurses5 (>= 5.6+20071006-3) Description: BSD editline and history libraries The editline library provides generic line editing and history functions. It slightly resembles GNU readline Homepage: http://ftp.netbsd.org/pub/NetBSD/NetBSD-release-5-0/src/lib/libedit/ -- 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: 9.1 (git head) does not compile using --with-libedit-preferred on Ubuntu 10.10
Look at the libeditline-dev packages I think those might be more modern than the libedit packages. But I'm not sure myself, I don't really know the history, I just remember being confused by it once in the past. -- greg -- 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: 9.1 (git head) does not compile using --with-libedit-preferred on Ubuntu 10.10
On Wed, Feb 16, 2011 at 2:43 AM, Mark Kirkwood wrote: > What's this libbsd then eh? Sure enough it is this guy that defines these > symbols. So it is the way it is being built on the Ubuntu (or Debian) > platform. Oh, for what it's worth there are several different libedits out there with various related heritages. It's possible you're looking at two completely different packages. On Debian /usr/share/doc//README.Debian is supposed to say where the upstream source was. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 9.1 (git head) does not compile using --with-libedit-preferred on Ubuntu 10.10
On 16/02/11 15:05, Mark Kirkwood wrote: On 16/02/11 14:54, Tom Lane wrote: It's pretty hard to see how those two things would be related. I think more likely libedit is providing a function named setproctitle, which seems like a rather stupid thing for them to have done. You are correct - it defines setproctitle, good grief. ...for some level of completeness in case this comes up again: it is not libedit that is at fault here. I downloaded the src for versions 2 amd 3 and neither setproctitle not optreset are defined anywhere. Looking at my Ubuntu install reveals: $ ldd /usr/lib/libedit.so.2 linux-vdso.so.1 => (0x7fffd1bd3000) libbsd.so.0 => /lib/libbsd.so.0 (0x7ff219cc9000) libncurses.so.5 => /lib/libncurses.so.5 (0x7ff219a85000) libc.so.6 => /lib/libc.so.6 (0x7ff219701000) libdl.so.2 => /lib/libdl.so.2 (0x7ff2194fd000) /lib64/ld-linux-x86-64.so.2 (0x7ff21a12) What's this libbsd then eh? Sure enough it is this guy that defines these symbols. So it is the way it is being built on the Ubuntu (or Debian) platform. Cheers Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PERFORM] pgbench to the MAXINT
On Fri, Feb 11, 2011 at 8:35 AM, Stephen Frost wrote: > Greg, > > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Greg Smith writes: >> > Poking around a bit more, I just discovered another possible approach is >> > to use erand48 instead of rand in pgbench, which is either provided by >> > the OS or emulated in src/port/erand48.c That's way more resolution >> > than needed here, given that 2^48 pgbench accounts would be a scale of >> > 2.8M, which makes for a database of about 42 petabytes. >> >> I think that might be a good idea --- it'd reduce the cross-platform >> variability of the results quite a bit, I suspect. random() is not >> to be trusted everywhere, but I think erand48 is pretty much the same >> wherever it exists at all (and src/port/ provides it elsewhere). > > Works for me. Greg, will you be able to work on this change? If not, I > might be able to. Seeing as how this patch has not been updated, I think it's time to mark this one Returned with Feedback. -- 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] Usability tweaks for extension commands
On Feb 15, 2011, at 5:18 PM, Tom Lane wrote: > Currently, ALTER EXTENSION UPDATE throws an error if there's nothing to > do: > > regression=# create extension adminpack ; > CREATE EXTENSION > regression=# alter extension adminpack update; > ERROR: version to install or update to must be different from old version > > On reflection it seems like this is overly paranoid, and it'd be more > useful if the ALTER just reported a NOTICE along the lines of "version > so-and-so is already installed". Any objections? Makes sense to me. > Another thought is that it'd probably be useful for there to be a > "CREATE OR REPLACE EXTENSION" syntax, with the behavior of "install the > extension if it's not present, else make sure it's of the specified or > default version"; this behavior parallels CREATE OR REPLACE LANGUAGE > which is something we've been refining for awhile. I am not however > entirely sure what to do with the SCHEMA option if the extension already > exists --- we might be able to do SET SCHEMA, but perhaps that's too > aggressive. This one is a bit over my head, alas. 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] Replication server timeout patch
On Tue, Feb 15, 2011 at 7:13 AM, 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 > > 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'm not sure if there's already enough infrastructure for a non-blocking write. But the patch which I submitted before might help to implement that. http://archives.postgresql.org/message-id/AANLkTinSvcdAYryNfZqd0wepyh1Pf7YX6Q0KxhZjas6a%40mail.gmail.com 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: pg_ctl failover Re: [HACKERS] Latches, signals, and waiting
On Sun, Feb 13, 2011 at 10:18 PM, Fujii Masao wrote: > Thanks for the review! > > On Thu, Feb 10, 2011 at 11:25 PM, Magnus Hagander wrote: >> I see that the docs part of the patch removes the mentioning of >> reporting servers - is that intentional, or a mistake? Seems that >> usecase still remains, no? > > It was intentional, but I agree with you. I re-added the mention to > the reporting servers. > > On Thu, Feb 10, 2011 at 11:30 PM, Magnus Hagander wrote: >> Also, the patch no longer applies, since it conflicts with >> faa0550572583f51dba25611ab0f1d1c31de559b. >> >> Since you (Fujii-san) wrote both of them, feel like rebasing it >> properly for current master? > > Yeah, I rebased the patch to the current git master and attached it. Committed with minor tweaks to comments and documentation. -- 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] XMin Hot Standby Feedback patch
On Wed, Feb 16, 2011 at 6:15 AM, Simon Riggs wrote: > On Tue, 2011-02-15 at 12:15 -0500, Robert Haas wrote: >> Looks pretty good to me, though I haven't tested it. I like some of >> the safety valves you put in there, but I don't understand this part > > Reworked logic covering all feedback, plus tests, plus docs. > > Last comments before commit please. When I started the standby with hot_standby = off and hot_standby_feedback = on, I got the following assertion error. - sby LOG: streaming replication successfully connected to primary TRAP: FailedAssertion("!(((result) >= ((TransactionId) 3)))", File: "procarray.c", Line: 1027) act LOG: unexpected EOF on standby connection sby LOG: WAL receiver process (PID 17572) was terminated by signal 6: Aborted sby LOG: terminating any other active server processes - vacuum_defer_cleanup_age on the *standby* should not affect the feedback xid. VACUUM TABLE on the *primary* doesn't use the feedback xid at all. Is this intentional? 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] CommitFest 2011-01 as of 2011-02-04
On Wed, Feb 16, 2011 at 02:14, Andrew Dunstan wrote: >> I've been kind of wondering why you haven't already committed it. If >> you're confident that the code is in good shape, I don't particularly >> see any benefit to holding off. > > +10. The sooner the better. Thanks comments. I've applied the COPY API patch. -- 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] updated patch for foreach stmt
* Robert Haas (robertmh...@gmail.com) wrote: > On Tue, Feb 15, 2011 at 8:44 PM, Tom Lane wrote: > > Anyway I'm going to start on this patch next, so last chance for > > opinions about the syntax ... > > Oh, I was looking at this one: > > http://archives.postgresql.org/pgsql-hackers/2010-12/msg01557.php > > Anyhoo, forcing the explicit ARRAY keyword in there seems like pretty > cheap future-proofing to me. YMMV. +1 for this, I don't see it as a big deal, and I would hate to discover there's some reason we care (I dunno, implicit casts from ARRAY to hstore ?) in the future that we're not thinking about now. This also means there's no ambiguity as to what the iterator variable should be declared as- if you're doing a FOREACH .. ARRAY, then your iterator is an ARRAY (if it's not a scalar, of course), full stop. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] 9.1 (git head) does not compile using --with-libedit-preferred on Ubuntu 10.10
On 16/02/11 14:54, Tom Lane wrote: It's pretty hard to see how those two things would be related. I think more likely libedit is providing a function named setproctitle, which seems like a rather stupid thing for them to have done. You are correct - it defines setproctitle, good grief. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] contrib loose ends: 9.0 to 9.1 incompatibilities
On Tue, Feb 15, 2011 at 7:10 PM, Tom Lane wrote: > 1. We could just revert the pg_proc.h changes so that these two > functions are still shown as taking only 2 arguments. Since GIN doesn't > actually look at the signature claimed in pg_proc, this won't break > anything functionally. It's pretty ugly though, and potentially will > confuse people down the road. -1. > 2. We could add extra pg_proc.h entries matching the old signatures. > For the moment these would be stub functions that call the same C code, > though eventually perhaps they could be changed to throw errors. +1. > A related issue is that we similarly changed the signatures of GIN > support functions that properly belong to intarray and tsearch2. > That affects what the "unpackaged" conversion scripts need to expect. > > What I'm inclined to do there is just change the scripts to absorb > the old functions as-is without trying to correct their signatures. > Doing otherwise is a bit painful because they are operator class > members, and there's no easy way to unhook them from the opclasses > without dropping the opclasses. The only other fix I can think of > is a direct UPDATE on pg_proc to fix the proargtypes entries, which > would work but seems even uglier. Hmm. Can we just invent a way to hook them from the opclasses? I have a feeling that now that this extension stuff is in we're going to discover a bunch of these little utility commands that we managed to get by without in the past but now that we're getting more organized about it, we'll need 'em. > There are some similar issues in pg_trgm as well. I believe we can fix > these with the available facilities so long as we don't mind the fact > that opclasses upgraded from 9.0 installations will be subtly different > from ones installed fresh in 9.1, for example the new operators being > considered "loose" in the opfamily instead of being bound into an > operator class. While I don't immediately see any problems likely to > arise from that, it's something that could perhaps bite us eventually. > But there's no other answer except embarking on a project to materially > upgrade the capabilities of ALTER OPERATOR CLASS/FAMILY, something I > really don't want to be doing right now. Or maybe that answers my question. > BTW, none of these issues are new with the extensions patch; they are > things we broke awhile ago. I'm thinking it's really past time that > we set up some routine buildfarm-style testing to see if pg_upgrade > from the previous version still works. Yeah. -- 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] updated patch for foreach stmt
On Tue, Feb 15, 2011 at 8:44 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Feb 8, 2011 at 3:26 PM, Stephen Frost wrote: >>> Alright, so, like I said, I really like this feature and would like to >>> see it included. > >> Amen to that! > >> I think the syntax Tom suggested before was FOREACH thingy IN ARRAY >> arr rather than just FOREACH thingy IN arr. > > Actually, I'm on record as saying the opposite: we shouldn't need to > distinguish the exact data type at the syntax level, so long as the > FOREACH construct is understood to mean "iterate through the members of > the composite object produced by this expression": > > http://archives.postgresql.org/pgsql-hackers/2010-12/msg01579.php > > I am not, however, wedded to that position --- if people are happier > with explicit use of ARRAY here, I won't fight hard to get rid of it. > > Anyway I'm going to start on this patch next, so last chance for > opinions about the syntax ... Oh, I was looking at this one: http://archives.postgresql.org/pgsql-hackers/2010-12/msg01557.php Anyhoo, forcing the explicit ARRAY keyword in there seems like pretty cheap future-proofing to me. YMMV. -- 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] 9.1 (git head) does not compile using --with-libedit-preferred on Ubuntu 10.10
Mark Kirkwood writes: > Since libedit is getting some attention right now, I figured I'd try > using building with it instead of readline. configuring using: > ./configure --prefix=/usr/local/pgsql/9.1 --enable-debug > --enable-cassert --with-libedit-preferred > I get this linking postgres: > postmaster/postmaster.o: In function `PostmasterMain': > /home/postgres/develop/c/postgresql/src/backend/postmaster/postmaster.c:755: > undefined reference to `optreset' > tcop/postgres.o: In function `process_postgres_switches': > /home/postgres/develop/c/postgresql/src/backend/tcop/postgres.c:3457: > undefined reference to `optreset' > utils/misc/ps_status.o: In function `set_ps_display': > /home/postgres/develop/c/postgresql/src/backend/utils/misc/ps_status.c:314: > undefined reference to `setproctitle' > What seems to be going on is that libedit defines optreset, which then > messes up the various other bits of logic in utils/misc/ps_status.c so > it thinks it has setproctitle on Linux. It's pretty hard to see how those two things would be related. I think more likely libedit is providing a function named setproctitle, which seems like a rather stupid thing for them to have done. The optreset thing is probably the same ilk of issue, ie, libedit providing such a symbol and configure then being fooled into expecting it to be present in the standard libraries. We could possibly fix this by delaying the test for libedit till after we've tested for unrelated stuff, but that seems pretty klugy. > I'm not sure how important this is, given that even if I (bodge) these > errors away, I merely confirm that libedit multi-byte input is busted in > the version shipped with Ubuntu 10.10. There's that too :-( regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] 9.1 (git head) does not compile using --with-libedit-preferred on Ubuntu 10.10
Since libedit is getting some attention right now, I figured I'd try using building with it instead of readline. configuring using: ./configure --prefix=/usr/local/pgsql/9.1 --enable-debug --enable-cassert --with-libedit-preferred I get this linking postgres: postmaster/postmaster.o: In function `PostmasterMain': /home/postgres/develop/c/postgresql/src/backend/postmaster/postmaster.c:755: undefined reference to `optreset' tcop/postgres.o: In function `process_postgres_switches': /home/postgres/develop/c/postgresql/src/backend/tcop/postgres.c:3457: undefined reference to `optreset' utils/misc/ps_status.o: In function `set_ps_display': /home/postgres/develop/c/postgresql/src/backend/utils/misc/ps_status.c:314: undefined reference to `setproctitle' What seems to be going on is that libedit defines optreset, which then messes up the various other bits of logic in utils/misc/ps_status.c so it thinks it has setproctitle on Linux. The other files postmaster/postmaster.c and tcop/postgres.c get tricked into thinking they have optreset. I'm not sure how important this is, given that even if I (bodge) these errors away, I merely confirm that libedit multi-byte input is busted in the version shipped with Ubuntu 10.10. Cheers Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] updated patch for foreach stmt
Robert Haas writes: > On Tue, Feb 8, 2011 at 3:26 PM, Stephen Frost wrote: >> Alright, so, like I said, I really like this feature and would like to >> see it included. > Amen to that! > I think the syntax Tom suggested before was FOREACH thingy IN ARRAY > arr rather than just FOREACH thingy IN arr. Actually, I'm on record as saying the opposite: we shouldn't need to distinguish the exact data type at the syntax level, so long as the FOREACH construct is understood to mean "iterate through the members of the composite object produced by this expression": http://archives.postgresql.org/pgsql-hackers/2010-12/msg01579.php I am not, however, wedded to that position --- if people are happier with explicit use of ARRAY here, I won't fight hard to get rid of it. Anyway I'm going to start on this patch next, so last chance for opinions about the syntax ... 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] Usability tweaks for extension commands
Currently, ALTER EXTENSION UPDATE throws an error if there's nothing to do: regression=# create extension adminpack ; CREATE EXTENSION regression=# alter extension adminpack update; ERROR: version to install or update to must be different from old version On reflection it seems like this is overly paranoid, and it'd be more useful if the ALTER just reported a NOTICE along the lines of "version so-and-so is already installed". Any objections? Another thought is that it'd probably be useful for there to be a "CREATE OR REPLACE EXTENSION" syntax, with the behavior of "install the extension if it's not present, else make sure it's of the specified or default version"; this behavior parallels CREATE OR REPLACE LANGUAGE which is something we've been refining for awhile. I am not however entirely sure what to do with the SCHEMA option if the extension already exists --- we might be able to do SET SCHEMA, but perhaps that's too aggressive. 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] FOR KEY LOCK foreign keys
> How is such a determination made, exactly? It's Feb 15th, and portions of the patch need a rework according to the author. I'm with Robert on this one. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] contrib loose ends: 9.0 to 9.1 incompatibilities
I've been experimenting with dump/reload of 9.0 contrib-using databases into 9.1 and then applying CREATE EXTENSION FROM to update the contrib modules to extension style. There are some cases that fail :-(. Most of them are caused by the GIN extractQuery API changes. In particular, a 9.0 dump including intarray will fail altogether with "ginarrayextract(anyarray, internal) does not exist", and similarly reloading tsearch2 fails with "gin_extract_tsvector(pg_catalog.tsvector, internal) does not exist". The functions do still exist in core, but we added an extra argument to each. There seem to be two possible solutions to this: 1. We could just revert the pg_proc.h changes so that these two functions are still shown as taking only 2 arguments. Since GIN doesn't actually look at the signature claimed in pg_proc, this won't break anything functionally. It's pretty ugly though, and potentially will confuse people down the road. 2. We could add extra pg_proc.h entries matching the old signatures. For the moment these would be stub functions that call the same C code, though eventually perhaps they could be changed to throw errors. Preferences anyone? A related issue is that we similarly changed the signatures of GIN support functions that properly belong to intarray and tsearch2. That affects what the "unpackaged" conversion scripts need to expect. What I'm inclined to do there is just change the scripts to absorb the old functions as-is without trying to correct their signatures. Doing otherwise is a bit painful because they are operator class members, and there's no easy way to unhook them from the opclasses without dropping the opclasses. The only other fix I can think of is a direct UPDATE on pg_proc to fix the proargtypes entries, which would work but seems even uglier. There are some similar issues in pg_trgm as well. I believe we can fix these with the available facilities so long as we don't mind the fact that opclasses upgraded from 9.0 installations will be subtly different from ones installed fresh in 9.1, for example the new operators being considered "loose" in the opfamily instead of being bound into an operator class. While I don't immediately see any problems likely to arise from that, it's something that could perhaps bite us eventually. But there's no other answer except embarking on a project to materially upgrade the capabilities of ALTER OPERATOR CLASS/FAMILY, something I really don't want to be doing right now. Comments? BTW, none of these issues are new with the extensions patch; they are things we broke awhile ago. I'm thinking it's really past time that we set up some routine buildfarm-style testing to see if pg_upgrade from the previous version still works. 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_upgrade seems a tad broken
Tom Lane wrote: > 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. I have fixed the bug; patch attached and applied. Seems I introduced it during my pg_upgrade restructuring and didn't run my full regession test suite after that. My apologies. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c index dbbc143..0c518a2 100644 --- a/contrib/pg_upgrade/info.c +++ b/contrib/pg_upgrade/info.c @@ -48,7 +48,7 @@ gen_db_file_maps(DbInfo *old_db, DbInfo *new_db, for (relnum = 0; relnum < old_db->rel_arr.nrels; relnum++) { RelInfo*old_rel = &old_db->rel_arr.rels[relnum]; - RelInfo*new_rel = &old_db->rel_arr.rels[relnum]; + RelInfo*new_rel = &new_db->rel_arr.rels[relnum]; if (old_rel->reloid != new_rel->reloid) pg_log(PG_FATAL, "mismatch of relation id: database \"%s\", old relid %d, new relid %d\n", -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python do not delete function arguments
On 15/02/11 20:39, Peter Eisentraut wrote: > On tis, 2011-02-15 at 09:58 +0100, Jan Urbański wrote: >> Because the invocation that actually recurses sets up the scene for >> failure. > > That's what we're observing, but I can't figure out why it is. If you > can, could you explain it? > > It actually makes sense to me that the arguments should be deleted at > the end of the call. The data belongs to that call only, and > PLy_procedure_delete() that would otherwise clean it up is only called > rarely. > > Apparently, the recursive call ends up deleting the wrong arguments, but > it's not clear to me why that would affect the next top-level call, > because that would set up its own arguments again anyway. In any case, > perhaps the right fix is to fix PLy_function_delete_args() to delete the > args correctly. Aaah, ok, I got it (again). Let me write this in full before I forget and spend another hour chasing that bug (and boy, bugs that disappear because you're doing things in the debugger are so annoying). And actually, my patch doesn't fix it fully :| Let me demonstrate: CREATE FUNCTION rec(n integer) RETURNS integer AS $$ if n == 0: return plpy.notice("before n is %d" % n) plpy.execute("select rec(0)") plpy.notice("after n is %d" % n) $$ LANGUAGE plpythonu; Without the patch the second plpy.notice raises a NameError. With the patch the output is: NOTICE: before n is 4 CONTEXT: PL/Python function "rec" NOTICE: after n is 0 CONTEXT: PL/Python function "rec" What happens? In PLy_function_handler, PLy_function_build_args is called, and proc->globals is set. After that PLy_procedure_call is called, which starts executing Python code. The Python code does a call into C with plpy.execute, and PLy_function_handler gets called (a reentrant call). Then PLy_function_build_args is called again. It overwrites the "n" entry in proc->globals and then PLy_procedure_call gets called, which drops us back into Python (on the stack there's now C, Python, C, Python). This second invocation exits quickly because n == 0, and we're back in C. Now without my patch, the next thing to happen was deleting the arguments, which removed "n" from the proc->globals dict. The rest of C code runs and finally plpy.execute returns and we;re back in Python (the stack is C, Python). The second plpy.notice is run, which fetches "n" from the globals, and not finding it, raises a NameError. With the patch it simply fetches the overwritten value, namely 0. The KeyError was a red herring - that's how Python reacted when evaluating "n in (0, 1)", and if you look in the server log you'll see a RuntimeWarning complaining about something internal, that doesn't matter. The bottom line is that PLy_procedure_call is not reentrant because of proc->globals, and it has to be. Now when fixing this bug I tries copying the globals dict and restoring it, but ran into issues (I think the problem was that the function didn't like running with different globals then the one it has been compiled with). Not sure what to do with this :( Document it as a caveat (with or without my patch) and carry on? That sucks quite badly... 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] [COMMITTERS] pgsql: Adjust pg_upgrade error message, array freeing, and add error ch
Excerpts from Tom Lane's message of mar feb 15 18:05:59 -0300 2011: > Bruce Momjian writes: > > Adjust pg_upgrade error message, array freeing, and add error check. > > The buildfarm says this patch is broken. I have just pushed a fix for this. It's probably not the prettiest thing in the world, but there doesn't seem to be a place where the ClusterInfo structs are initialized. Maybe that merits more cleanup, not sure. -- Á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] Debian readline/libedit breakage
--On 15. Februar 2011 18:52:04 +0100 Stefan Kaltenbrunner wrote: well I have not actually tested - I was just reading the changelog on http://www.thrysoee.dk/editline/ which claims UTF8 "support" (whatever that means) in the current code drop. I tested it--enable-wc doesn't work as you might expect. As Greg already said, it will bother you with a strange behavior when deleting characters (e.g. it removes half of your prompt) and at least on my mac i still wasn't able to enter multibyte characters like german umlauts. -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XMin Hot Standby Feedback patch
On Tue, Feb 15, 2011 at 4:15 PM, Simon Riggs wrote: > On Tue, 2011-02-15 at 12:15 -0500, Robert Haas wrote: >> Looks pretty good to me, though I haven't tested it. I like some of >> the safety valves you put in there, but I don't understand this part > > Reworked logic covering all feedback, plus tests, plus docs. > > Last comments before commit please. What happens if someone has hot_standby_feedback on and then turns it off? I think in XLogWalRcvSendReply() you need if (hot_standby_feedback) { stuff } else { reply_message.xmin = InvaidXID; reply_message.epoch = 0; /* or something */ } Also this part looks kludgy to me: + GetNextXidAndEpoch(&nextXid, &nextEpoch); + if (nextXid < reply_message.xmin) + nextEpoch--; How about introducing a GetOldestXminAndEpoch function instead? Would it make sense to avoid grabbing the ProcArrayLock except when we truly need to update MyProc->xmin? ProcessStandbyReplyMessage gets called a lot... -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine writes: > Do we want to add such a query in the docs to help pgfoundry authors to > write their own 'from unpackaged' scripts? [ scratches head ... ] Why is your version generating so many unnecessary @extschema@ uses? 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] NULLs in array_cat vs array || array
On 15 February 2011 21:47, Tom Lane wrote: > Also, so far as I can see array_cat *is* ||, so I'm not sure what > discrepancy in behavior you're on about. You've confused me now. I had a case where I replaced || with , and surrounded it with array_cat, and the result differed, and now I can't recreate it. I think I should get an early night. -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
On 02/15/2011 04:49 PM, Dimitri Fontaine wrote: Ah well sed makes it simpler to read, but it won't be usable in windows. You can make perl do the same stuff (and perl has psed anyway), and perl is required for MSVC builds. 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] NULLs in array_cat vs array || array
On 15 February 2011 21:46, Cédric Villemain wrote: > 2011/2/15 Thom Brown : >> Hi all, >> >> I assumed array_cat would behave similarly to array || array, but it >> appears not when it comes to NULLs. Shouldn't these have identical >> functionality? The attached patch makes it so, although it would >> break existing code. > > There is bugreport and todo entry for that if it helps: > > http://archives.postgresql.org/pgsql-bugs/2008-11/msg00032.php Ah, I see. More to it than meets the eye. My bad. -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane writes: > Just for the archives' sake: the '@extschema@' business did turn out to > be important, at least for tsearch2 where it's necessary to distinguish > the objects it's dealing with from similarly-named objects in > pg_catalog. So this is what I used to generate the "unpackaged" > scripts. Some of them needed manual adjustment later to cover cases > where 9.1 had diverged from 9.0, but the script could hardly be expected > to know about that. Good to know that even contrib needs that! > #! /bin/sh > > MOD="$1" > > psql -d testdb -c "create extension $MOD" > > ( > echo "/* contrib/$MOD/$MOD--unpackaged--1.0.sql */" > echo > > psql -A -t -d testdb -c " > SELECT 'ALTER EXTENSION ' || E.extname || ' ADD ' > || replace(pg_describe_object(classid, objid, 0), > N.nspname, '@extschema@') > || ';' > FROM pg_depend D > JOIN pg_extension E ON D.refobjid = E.oid > AND D.refclassid = E.tableoid > JOIN pg_namespace N ON E.extnamespace = N.oid > WHERE deptype = 'e' AND E.extname = '$MOD' > ORDER BY D.objid > " | sed -e 's/ADD cast from \(.*\) to \(.*\);/ADD cast (\1 as \2);/' \ > -e 's/ for access method / using /' > ) > contrib/$MOD/$MOD--unpackaged--1.0.sql Ah well sed makes it simpler to read, but it won't be usable in windows. I now realize also that the second version of this query did some useless array type filtering. Adding a replace() step in the query would not be that ugly I guess, if we wanted to make it so. Do we want to add such a query in the docs to help pgfoundry authors to write their own 'from unpackaged' scripts? CREATE OR REPLACE FUNCTION extension_unpackaged_upgrade_script(text) RETURNS SETOF text LANGUAGE SQL AS $$ WITH objs AS ( SELECT 'ALTER EXTENSION ' || E.extname || ' ADD ' || replace(pg_describe_object(classid, objid, 0), N.nspname, '@extschema@') || ';' AS d FROM pg_depend D JOIN pg_extension E ON D.refobjid = E.oid AND D.refclassid = E.tableoid JOIN pg_namespace N ON E.extnamespace = N.oid WHERE deptype = 'e' AND E.extname = $1 ORDER BY D.objid ) SELECT regexp_replace(replace(d, ' for access method ', ' using '), 'ADD cast from (.*) to (.*);', E'ADD cast (\\1 as \\2);') FROM objs $$; dim=# select * from extension_unpackaged_upgrade_script('lo'); extension_unpackaged_upgrade_script - ALTER EXTENSION lo ADD type @extschema@.lo; ALTER EXTENSION lo ADD function @extschema@.lo_oid(@extschema@.lo); ALTER EXTENSION lo ADD function @extschema@.lo_manage(); (3 rows) 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] NULLs in array_cat vs array || array
Thom Brown writes: > I assumed array_cat would behave similarly to array || array, but it > appears not when it comes to NULLs. Shouldn't these have identical > functionality? The attached patch makes it so, although it would > break existing code. That patch is the hard way: the right change would be to remove the code altogether and mark the function strict in pg_proc. However, the fact that it's not like that already shows that we went out of our way to make it so. I don't think we should undo that decision just because somebody submits a patch to do so. Also, so far as I can see array_cat *is* ||, so I'm not sure what discrepancy in behavior you're on about. 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] NULLs in array_cat vs array || array
2011/2/15 Thom Brown : > Hi all, > > I assumed array_cat would behave similarly to array || array, but it > appears not when it comes to NULLs. Shouldn't these have identical > functionality? The attached patch makes it so, although it would > break existing code. There is bugreport and todo entry for that if it helps: http://archives.postgresql.org/pgsql-bugs/2008-11/msg00032.php > > Would such a change have any knock-on effect, or cause inconsistency > with other functions? > > Thanks > > Thom > > -- > 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 > > -- 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
[HACKERS] NULLs in array_cat vs array || array
Hi all, I assumed array_cat would behave similarly to array || array, but it appears not when it comes to NULLs. Shouldn't these have identical functionality? The attached patch makes it so, although it would break existing code. Would such a change have any knock-on effect, or cause inconsistency with other functions? Thanks Thom -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 array_cat_nulls.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] FOR KEY LOCK foreign keys
Excerpts from Robert Haas's message of mar feb 15 18:15:38 -0300 2011: > I am thinking that the statute of limitations has expired on this > patch, and that we should mark it Returned with Feedback and continue > working on it for 9.2. I know it's a valuable feature, but I think > we're out of time. Okay, I've marked it as such in the commitfest app. It'll be in 9.2's first commitfest. -- Á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] FOR KEY LOCK foreign keys
On Feb 15, 2011, at 1:15 PM, Robert Haas wrote: >> Yeah, that bug is fixed with the attached, though I am rethinking this >> bit. > > I am thinking that the statute of limitations has expired on this > patch, and that we should mark it Returned with Feedback and continue > working on it for 9.2. I know it's a valuable feature, but I think > we're out of time. How is such a determination made, exactly? 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] XMin Hot Standby Feedback patch
On Tue, 2011-02-15 at 12:15 -0500, Robert Haas wrote: > Looks pretty good to me, though I haven't tested it. I like some of > the safety valves you put in there, but I don't understand this part Reworked logic covering all feedback, plus tests, plus docs. Last comments before commit please. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 63c6283..30c33fb 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2029,6 +2029,10 @@ SET ENABLE_SEQSCAN TO OFF; This parameter can only be set in the postgresql.conf file or on the server command line. + +You should also consider setting hot_standby_feedback +as an alternative to using this parameter. + @@ -2121,6 +2125,22 @@ SET ENABLE_SEQSCAN TO OFF; + + hot_standby_feedback (boolean) + + hot_standby_feedback configuration parameter + + + +Specifies whether or not a hot standby will send feedback to the primary +about queries currently executing on the standby. This parameter can +be used to eliminate query cancels caused by cleanup records, though +it can cause database bloat on the primary for some workloads. +The default value is off. + + + + diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index a892969..6941e67 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -1486,23 +1486,6 @@ if (!triggered) -The most common reason for conflict between standby queries and WAL replay -is early cleanup. Normally, PostgreSQL allows -cleanup of old row versions when there are no transactions that need to -see them to ensure correct visibility of data according to MVCC rules. -However, this rule can only be applied for transactions executing on the -master. So it is possible that cleanup on the master will remove row -versions that are still visible to a transaction on the standby. - - - -Experienced users should note that both row version cleanup and row version -freezing will potentially conflict with standby queries. Running a manual -VACUUM FREEZE is likely to cause conflicts even on tables with -no updated or deleted rows. - - - Once the delay specified by max_standby_archive_delay or max_standby_streaming_delay has been exceeded, conflicting queries will be cancelled. This usually results just in a cancellation @@ -1529,6 +1512,23 @@ if (!triggered) +The most common reason for conflict between standby queries and WAL replay +is early cleanup. Normally, PostgreSQL allows +cleanup of old row versions when there are no transactions that need to +see them to ensure correct visibility of data according to MVCC rules. +However, this rule can only be applied for transactions executing on the +master. So it is possible that cleanup on the master will remove row +versions that are still visible to a transaction on the standby. + + + +Experienced users should note that both row version cleanup and row version +freezing will potentially conflict with standby queries. Running a manual +VACUUM FREEZE is likely to cause conflicts even on tables with +no updated or deleted rows. + + + Users should be clear that tables that are regularly and heavily updated on the primary server will quickly cause cancellation of longer running queries on the standby. In such cases the setting of a finite value for @@ -1539,12 +1539,10 @@ if (!triggered) Remedial possibilities exist if the number of standby-query cancellations -is found to be unacceptable. The first option is to connect to the -primary server and keep a query active for as long as needed to -run queries on the standby. This prevents VACUUM from removing -recently-dead rows and so cleanup conflicts do not occur. -This could be done using and -pg_sleep(), or via other mechanisms. If you do this, you +is found to be unacceptable. The first option is to set the parameter +hot_standby_feedback, which prevents VACUUM from +removing recently-dead rows and so cleanup conflicts do not occur. +If you do this, you should note that this will delay cleanup of dead rows on the primary, which may result in undesirable table bloat. However, the cleanup situation will be no worse than if the standby queries were running diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 3277da8..e23c4e5 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -45,6 +45,7 @@ #include "replicati
Re: [HACKERS] FOR KEY LOCK foreign keys
On Mon, Feb 14, 2011 at 6:49 PM, Alvaro Herrera wrote: > 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. I am thinking that the statute of limitations has expired on this patch, and that we should mark it Returned with Feedback and continue working on it for 9.2. I know it's a valuable feature, but I think we're out of time. -- 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 Tue, Feb 15, 2011 at 3:26 PM, marcin mank wrote: > On Tue, Feb 15, 2011 at 9:16 PM, Peter Eisentraut wrote: >> On mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: >>> I guess the real question is what's Peter's concrete objection to the >>> double-dash method? >> >> It just looks a bit silly and error prone. And other packaging systems >> have been doing without it for decades. > > how about : we use a single dash as the separator, and if the > extension author insists on having a dash in the name, as a punishment > he must duplicate the dash, i.e.: > uuid--ossp-1.0--5.5.sql That has a certain poetic justice to it. -- 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] review: FDW API
Heikki Linnakangas writes: > On 15.02.2011 21:13, Tom Lane wrote: >> Hmm. I don't have a problem with adding relkind to the planner's >> RelOptInfo, but it seems to me that if parse analysis needs to know >> this, you have put functionality into parse analysis that does not >> belong there. > Possibly. We throw the existing errors, for example if you try to do > "FOR UPDATE OF foo" where foo is a set-returning function, in > transformLockingClause(), so it seemed like the logical place to check > for foreign tables too. > Hmm, one approach would be to go ahead and create the RowMarkClauses for > all relations in the parse analysis phase, foreign or not, and throw the > error later, in preprocess_rowmarks(). I think moving the error check downstream would be a good thing. Consider for example the case of applying FOR UPDATE to a view. You can't know what that entails until after the rewriter expands the view. IIRC, at the moment we're basically duplicating the tests between parse analysis and the planner, but it's not clear what the value of that is. 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] review: FDW API
On 15.02.2011 21:00, Robert Haas wrote: On Tue, Feb 15, 2011 at 1:40 PM, Heikki Linnakangas wrote: I'm actually surprised we don't need to distinguish them in more places, but nevertheless it feels like we should have that info available more conveniently, and without requiring a catalog lookup like get_rel_relkind() does. At first I thought we should add a field to RelOptInfo, but that doesn't help transformLockingClause. Adding the field to RangeTblEntry seems quite invasive, and it doesn't feel like the right place to cache that kind of information anyway. Thoughts on that? Maybe it should be a new RTEKind. That would be even more invasive. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] review: FDW API
On 15.02.2011 21:13, Tom Lane wrote: Heikki Linnakangas writes: As the patch stands, we have to do get_rel_relkind() in a couple of places in parse analysis and the planner to distinguish a foreign table from a regular one. As the patch stands, there's nothing in RangeTblEntry (which is what we have in transformLockingClause) or RelOptInfo (for set_plain_rel_pathlist) to directly distinguish them. Hmm. I don't have a problem with adding relkind to the planner's RelOptInfo, but it seems to me that if parse analysis needs to know this, you have put functionality into parse analysis that does not belong there. Possibly. We throw the existing errors, for example if you try to do "FOR UPDATE OF foo" where foo is a set-returning function, in transformLockingClause(), so it seemed like the logical place to check for foreign tables too. Hmm, one approach would be to go ahead and create the RowMarkClauses for all relations in the parse analysis phase, foreign or not, and throw the error later, in preprocess_rowmarks(). preprocess_rowmarks() doesn't currently know if each RowMarkClause was created by "... FOR UPDATE" or "FOR UPDATE OF foo", but to be consistent with the logic in transformLockingClause() it would need to. For the former case, it would need to just ignore foreign tables but for the latter it would need to throw an error. I guess we could add an "explicit" flag to RowMarkClause for that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] why two dashes in extension load files
On Feb 15, 2011, at 12:32 PM, Tom Lane wrote: > Aside from the double-dash method, we kicked around using colons and > pluses as separators (and then forbidding just those characters in > extension and version names). Any of those would be workable, but it's > not clear to me that any of them have any particular cosmetic advantage > over any others. In any case, the time to be voting on them is past so > far as I'm concerned. The work is already done and I'm uneager to do it > over on one person's say-so. I'd prefer a single character, but can live with -- just fine. 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
Peter Eisentraut writes: > On mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: >> I guess the real question is what's Peter's concrete objection to the >> double-dash method? > It just looks a bit silly and error prone. And other packaging systems > have been doing without it for decades. I can't claim close familiarity with Debian's conventions in this matter, but I do know about RPM's, and I'm uneager to duplicate that silliness. Magic conversion of dots to underscores (sometimes), complete inability to determine which part of the package filename is which without external knowledge, etc. Aside from the double-dash method, we kicked around using colons and pluses as separators (and then forbidding just those characters in extension and version names). Any of those would be workable, but it's not clear to me that any of them have any particular cosmetic advantage over any others. In any case, the time to be voting on them is past so far as I'm concerned. The work is already done and I'm uneager to do it over on one person's say-so. 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 Tue, Feb 15, 2011 at 9:16 PM, Peter Eisentraut wrote: > On mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: >> I guess the real question is what's Peter's concrete objection to the >> double-dash method? > > It just looks a bit silly and error prone. And other packaging systems > have been doing without it for decades. how about : we use a single dash as the separator, and if the extension author insists on having a dash in the name, as a punishment he must duplicate the dash, i.e.: uuid--ossp-1.0--5.5.sql Greetings Marcin Mańk -- Sent 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 15:08 -0500, Tom Lane wrote: >> Umm ... we are not requiring version names to be numbers. > That's certainly interesting. Why? There isn't any packaging system anywhere on the planet that requires them to be purely numeric. By the time you get done allowing for multiple dots and "alpha" or "beta" and other such stuff, you might as well try to be agnostic about what they contain. 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 mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: > I guess the real question is what's Peter's concrete objection to the > double-dash method? It just looks a bit silly and error prone. And other packaging systems have been doing without it for decades. -- Sent 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 mån, 2011-02-14 at 15:08 -0500, Tom Lane wrote: > Umm ... we are not requiring version names to be numbers. That's certainly interesting. Why? -- Sent 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
> -Original Message- > From: Robert Haas [mailto:robertmh...@gmail.com] > Sent: 15 February 2011 16:52 > To: Tom Lane > Cc: Andrew Dunstan; Kohei Kaigai; Stephen Frost; KaiGai Kohei; PgHacker > Subject: Re: [HACKERS] sepgsql contrib module > > On Tue, Feb 15, 2011 at 11:41 AM, Tom Lane wrote: > > Robert Haas writes: > >> On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane wrote: > >>> Robert Haas writes: > Those are good points. My point was just that you can't actually > build that file at the time you RUN the regression tests, because you > have to build it first, then install it, then run the regression > tests. It could be a separate target, like 'make policy', but I don't > think it works to make it part of 'make installcheck'. > > > >>> So? Once you admit that you can do that, it's a matter of a couple > more > >>> lines to make the installcheck target depend on the policy target iff > >>> selinux was enabled. > > > >> Sure, you could do that, but I don't see what problem it would fix. > >> You'd still have to build and manually install the policy before you > >> could run make installcheck. And once you've done that, you don't > >> need to rebuild it every future time you run make installcheck. > > > > Oh, I see: you're pointing out the root-only "semodule" step that has > to > > be done in between there. Good point. But the current arrangement is > > still a mistake: the required contents of sepgsql-regtest.pp depend on > > the configuration of the test system, which can't be known at build > > time. > > > > So what we should do is offer a "make policy" target and alter the test > > instructions to say you should do that and then run semodule. Or maybe > > just put the whole "make -f /usr/share/selinux/devel/Makefile" dance > > into the instructions --- it doesn't look to me like our makefile > > infrastructure really has anything useful to add to that. > > Yeah, agreed. > I also agree with this direction. The policy type depends on individual installations, it is not easy to assume on build time. Please wait for a small patch to remove this rule from Makefile and update documentation. As a side note, we can have a build option that does not require selinux enabled. The reason why Makefile of selinux tries to /selinux/mls is that we don't specify MLS=1 or MLS=0 explicitly. IIRC, the specfile of RHEL/Fedora gives all the Makefile parameters explicitly, thus, selinux does not need to be enabled on the build server. However, it is not a solution in this case. It is not easy to estimate the required policy type and existence of MLS support on build time. Thanks, -- NEC Europe Ltd, Global Competence Center KaiGai Kohei -- Sent 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 tis, 2011-02-15 at 09:58 +0100, Jan Urbański wrote: > Because the invocation that actually recurses sets up the scene for > failure. That's what we're observing, but I can't figure out why it is. If you can, could you explain it? It actually makes sense to me that the arguments should be deleted at the end of the call. The data belongs to that call only, and PLy_procedure_delete() that would otherwise clean it up is only called rarely. Apparently, the recursive call ends up deleting the wrong arguments, but it's not clear to me why that would affect the next top-level call, because that would set up its own arguments again anyway. In any case, perhaps the right fix is to fix PLy_function_delete_args() to delete the args correctly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] review: FDW API
Heikki Linnakangas writes: > As the patch stands, we have to do get_rel_relkind() in a couple of > places in parse analysis and the planner to distinguish a foreign table > from a regular one. As the patch stands, there's nothing in > RangeTblEntry (which is what we have in transformLockingClause) or > RelOptInfo (for set_plain_rel_pathlist) to directly distinguish them. Hmm. I don't have a problem with adding relkind to the planner's RelOptInfo, but it seems to me that if parse analysis needs to know this, you have put functionality into parse analysis that does not belong there. (No, I haven't read the patch...) 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] review: FDW API
On Tue, Feb 15, 2011 at 1:40 PM, Heikki Linnakangas wrote: > I'm actually surprised we don't need to distinguish them in more places, but > nevertheless it feels like we should have that info available more > conveniently, and without requiring a catalog lookup like get_rel_relkind() > does. At first I thought we should add a field to RelOptInfo, but that > doesn't help transformLockingClause. Adding the field to RangeTblEntry seems > quite invasive, and it doesn't feel like the right place to cache that kind > of information anyway. Thoughts on that? Maybe it should be a new RTEKind. -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
I wrote: > Dimitri Fontaine writes: >> I think you'd be interested into this reworked SQL query. It should be >> providing exactly the script file you need as an upgrade from unpackaged. > This seems overly complicated. I have a version of it that I'll publish > as soon as I've tested it on all the contrib modules ... Just for the archives' sake: the '@extschema@' business did turn out to be important, at least for tsearch2 where it's necessary to distinguish the objects it's dealing with from similarly-named objects in pg_catalog. So this is what I used to generate the "unpackaged" scripts. Some of them needed manual adjustment later to cover cases where 9.1 had diverged from 9.0, but the script could hardly be expected to know about that. #! /bin/sh MOD="$1" psql -d testdb -c "create extension $MOD" ( echo "/* contrib/$MOD/$MOD--unpackaged--1.0.sql */" echo psql -A -t -d testdb -c " SELECT 'ALTER EXTENSION ' || E.extname || ' ADD ' || replace(pg_describe_object(classid, objid, 0), N.nspname, '@extschema@') || ';' FROM pg_depend D JOIN pg_extension E ON D.refobjid = E.oid AND D.refclassid = E.tableoid JOIN pg_namespace N ON E.extnamespace = N.oid WHERE deptype = 'e' AND E.extname = '$MOD' ORDER BY D.objid " | sed -e 's/ADD cast from \(.*\) to \(.*\);/ADD cast (\1 as \2);/' \ -e 's/ for access method / using /' ) > contrib/$MOD/$MOD--unpackaged--1.0.sql 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] Add support for logging the current role
* Andrew Dunstan (and...@dunslane.net) wrote: > On 02/15/2011 11:13 AM, Stephen Frost wrote: > >Think I suggested that at one point. I'm all for doing that on a major > >version change like this one, but I think we already had some concerns > >about that on this thread (Andrew maybe?). > > I could live with it for a release if I thought we had a clear path > ahead, but I think there are some design issues that we need to > think about before we start providing for header lines and variable > formats in CSV logs, particularly w.r.t. log rotation etc. So I'm > slightly nervous about going ahead with this right now. I believe the suggestion that Robert and I were talking about above was to just unilatterally change the CSV log file output format to include current_role. No header lines, no variable output format, etc. I do think we can make header lines and variable output work, if we can get agreement on what the semantics should be. > You don't really make your case any better by continuing this > argument from years ago. I can tell you from experience that the CSV > HEADER feature is distinctly useful as it is. If you want to add a > mode that uses the header line as a column list on import, then make > that case, and I'll support it in fact, but it's not an alternative > to having the header ignored, which is a feature I would vigorously > resist removing. I'm not really interested in removing it. I guess I have a vain hope that with arguing I'll convince someone to take up the mantle of implementing the 'use header' option. :) Not getting much traction though, so I expect I'll work on it this summer. > (Incidentally, I think it won't be trivial - the > COPY code expects to know the columns by the time it opens the > file). Thanks for that insight, I'll take a look at how things work and see if I can come up with a sensible proposal. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] XMin Hot Standby Feedback patch
On Tue, 2011-02-15 at 12:20 -0500, Robert Haas wrote: > On Tue, Feb 15, 2011 at 12:15 PM, Robert Haas wrote: > > On another disk, I think that those warning messages are a bad idea. > > That could fill up someone's disk really quickly. > > On another disk? What the heck am I talking about? > > On another point? On another note? Anyway, you get the idea... hopefully. Yeh. I quite liked it as a metaphorical turn of phrase. -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 9:49 AM, Alexey Klyukin wrote: >>> After I re-added the closing in plperl.sgml:235 these errors >>> disappeared, and the >>> resulting html looks fine too. v10 with just this single change is attached. >> >> So is this ready for committer? > > Yes. Awesom, thanks Alexey & Alex! 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] Debian readline/libedit breakage
On 02/15/2011 12:37 PM, Greg Stark wrote: On Tue, Feb 15, 2011 at 6:12 AM, Stefan Kaltenbrunner wrote: 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... There's utf8 support and then there's utf8 support. last I saw libedit didn't actually stop you from using utf8 and things kind of worked, but none of the editing commands understand what the multibyte characters were or understood what column position you were in so you could easily end up deleting half a character or with the insertion point in the middle of a character. well I have not actually tested - I was just reading the changelog on http://www.thrysoee.dk/editline/ which claims UTF8 "support" (whatever that means) in the current code drop. 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] Fix for Index Advisor related hooks
Gurjeet Singh writes: > On Tue, Feb 15, 2011 at 8:24 AM, Heikki Linnakangas < > heikki.linnakan...@enterprisedb.com> wrote: >> On 11.02.2011 22:44, Gurjeet Singh wrote: >>> One one hand get_actual_variable_range() expects that virtual indexes do >>> not >>> have an OID assigned, on the other hand explain_get_index_name_hook() is >>> handed just an index's OID to get its name back; IMHO these are based on >>> two >>> conflicting assumptions about whether a virtual index will have an OID >>> assigned. >> The new hook takes an index oid as argument, so I gather that you resolved >> the contradiction by deciding that fictitious indexes have OIDs. How do you >> assign those OIDs? Do fictitious indexes have entries in pg_index? > No, a fictitious index does not touch pg_index. The Index Advisor uses > GetNewOid(pg_class) to generate a new OID for the fictitious index. That seems like a very expensive, and lock-inducing, way of assigning a fictitious OID. They don't need to be globally unique. I suggest you consider the idea I suggested back in 2007: * In this toy example we just assign all hypothetical indexes * OID 0, and the explain_get_index_name hook just prints * . In a realistic situation we'd probably * assume that OIDs smaller than, say, 100 are never the OID of * any real index, allowing us to identify one of up to 100 * hypothetical indexes per plan. Then we'd need to save aside * some state data that would let the explain hooks print info * about the selected index. As far as the immediate problem goes, I agree that get_actual_variable_range is mistaken, but I think a cleaner and cheaper solution would be to add a bool "hypothetical" to IndexOptInfo. 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] psql -l doesn't process psqlrc
On Sun, Feb 13, 2011 at 7:52 AM, Peter Eisentraut wrote: > psql -l doesn't process psqlrc. Historically, this was probably not > useful, hence no one cared. But with the linestyle option it's useful. > So I propose the attached tweak. As a violent hater of the new linestyle, +1 from me. -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 7:45 PM, David E. Wheeler wrote: > On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: > >> After I re-added the closing in plperl.sgml:235 these errors >> disappeared, and the >> resulting html looks fine too. v10 with just this single change is attached. > > So is this ready for committer? Yes. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: > After I re-added the closing in plperl.sgml:235 these errors > disappeared, and the > resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? 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 and psql
Tom Lane writes: > Sure I did: \dx+ And I believe I did test that. Sorry for the noise, really. (shame) 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] Replication server timeout patch
On Mon, Feb 14, 2011 at 5:13 PM, 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 > > 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? Are you working on this? -- 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] CommitFest 2011-01 as of 2011-02-04
robertmh...@gmail.com (Robert Haas) writes: > 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. I had two interesting germane comments in my RSS feed this morning, both entitled "Please send a patch" http://www.lucas-nussbaum.net/blog/?p=630 Where Lucas suggests that, when someone requests an enhancement, the retort "Please send a patch" mayn't be the best idea, because the one receiving the requests may be many times better at contributing such changes than the one making the request. http://hezmatt.org/~mpalmer/blog/general/please_send_a_patch.html "On the other hand, Lucas, remember that each time you ask someone to take some time to implement your pet feature request, you take some time away from her that could be used to contribute something in an area where she gives a damn." These are *both* true statements, and, in order to grow the community that is capable of enhancing the system, there is merit to the careful application of both positions. There's stuff that Tom should do :-). And absent the general availability of cloning machines, we need to have people improving their skills so that there are more that are capable of submitting (and evaluating and committing) usable patches. -- wm(X,Y):-write(X),write('@'),write(Y). wm('cbbrowne','gmail.com'). http://linuxdatabases.info/info/languages.html Signs of a Klingon Programmer - 14. "Our competitors are without honor!" -- Sent 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 and psql
Dimitri Fontaine writes: > I realize that you didn't keep the \dx behavior I had, that when given > an extension name it would list all the objects contained in the > extension. Sure I did: \dx+ 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] XMin Hot Standby Feedback patch
On Tue, Feb 15, 2011 at 12:15 PM, Robert Haas wrote: > On another disk, I think that those warning messages are a bad idea. > That could fill up someone's disk really quickly. On another disk? What the heck am I talking about? On another point? On another note? Anyway, you get the idea... hopefully. -- 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] extensions and psql
On Tue, Feb 15, 2011 at 11:37 AM, Dimitri Fontaine wrote: > Do we want to get that back in, and in which psql command? It could > well be that having \dx list extension and \dx name list extension's > objects wasn't the best design around, and it could be that it's not > useful enough, but I know I liked to have a psql shortcut to do that. +1 for having a psql command to do that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XMin Hot Standby Feedback patch
On Tue, Feb 15, 2011 at 11:42 AM, Simon Riggs wrote: > Patch attached, no docs yet, but the patch is clear. > > I'm looking to commit this in next 24 hours barring objections and/or > test failures. Looks pretty good to me, though I haven't tested it. I like some of the safety valves you put in there, but I don't understand this part: + /* +* Feedback from standby should move us forwards, but not too far. +* Avoid grabbing XidGenLock here in case of waits, so use +* a heuristic instead. +*/ What's XIDGenLock got to do with it? On another disk, I think that those warning messages are a bad idea. That could fill up someone's disk really quickly. -- 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] CommitFest 2011-01 as of 2011-02-04
On 02/15/2011 06:55 AM, Robert Haas wrote: On Tue, Feb 15, 2011 at 3:31 AM, Itagaki Takahiro wrote: On Tue, Feb 15, 2011 at 01:27, Robert Haas wrote: 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. Will we include the copy API patch in 9.1 even if we won't have file_fdw? Personally, I want to include the APIs because someone can develop file_fdw as a third party extension for 9.1 using the infrastructure. The extension will lack of file encoding support, but still useful for many cases. I've been kind of wondering why you haven't already committed it. If you're confident that the code is in good shape, I don't particularly see any benefit to holding off. +10. The sooner the better. 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] Sync Rep for 2011CF1
On Mon, Feb 14, 2011 at 12:25 AM, Fujii Masao wrote: > On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas > wrote: >> I added a XLogWalRcvSendReply() call into XLogWalRcvFlush() so that it also >> sends a status update every time the WAL is flushed. If the walreceiver is >> busy receiving and flushing, that would happen once per WAL segment, which >> seems sensible. > > This change can make the callback function "WalRcvDie()" call ereport(ERROR) > via XLogWalRcvFlush(). This looks unsafe. Good catch. Is the cleanest solution to pass a boolean parameter to XLogWalRcvFlush() indicating whether we're in the midst of dying? -- 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] Sync Rep for 2011CF1
On Tue, Feb 15, 2011 at 1:11 AM, Fujii Masao wrote: > 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. Fixed. -- 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] Sync Rep for 2011CF1
On Mon, Feb 14, 2011 at 12:08 AM, 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. > > + * 'd' means a standby reply wrapped in a COPY BOTH packet. > + */ > > Typo: s/COPY BOTH/CopyData Fixed. > + msgtype = pq_getmsgbyte(&input_message); > + if (msgtype != 'r') > + ereport(COMMERROR, > + (errcode(ERRCODE_PROTOCOL_VIOLATION), > + errmsg("unexpected message type %c", msgtype))); > > I think that proc_exit(0) needs to be called in error case. Fixed. > + static StringInfoData input_message; > + StandbyReplyMessage reply; > + char msgtype; > + > + initStringInfo(&input_message); > > Doesn't the repeat of initStringInfo() cause the memory leak? Yeah. Fixed, I hope. > @@ -518,6 +584,7 @@ WalSndLoop(void) > { > if (!XLogSend(output_message, &caughtup)) > break; > + ProcessRepliesIfAny(); > > Why is ProcessRepliesIfAny() required there? I'm not sure if that's 100% necessary, but it seems harmless enough. > We added new columns "write_location", "flush_location" and > "apply_location". So, for the sake of consistency, the column > name "sent_location" should be changed to "send_location"? I was thinking about stream_location or streaming_location, per discussion on the other thread. -- 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] Fix for Index Advisor related hooks
Gurjeet Singh writes: > Also attached is the patch expose_IndexSupportInitialize.patch, that makes > the static function IndexSupportInitialize() global so that the Index > Advisor doesn't have to reinvent the wheel to prepare an index structure > with opfamilies and opclasses. We are *not* doing that. That's a private function and will remain so. 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] XMin Hot Standby Feedback patch
On Tue, 2011-02-15 at 18:49 +0200, Heikki Linnakangas wrote: > It would be wise to also transmit the epoch in addition to xmin, to > avoid confusion if the standby is > 2 billion transactions behind. Yes, good idea, thanks. That has to be the record for the fastest patch review. ;-) -- 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] Add support for logging the current role
On 02/15/2011 11:13 AM, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for this particular case. Think I suggested that at one point. I'm all for doing that on a major version change like this one, but I think we already had some concerns about that on this thread (Andrew maybe?). I could live with it for a release if I thought we had a clear path ahead, but I think there are some design issues that we need to think about before we start providing for header lines and variable formats in CSV logs, particularly w.r.t. log rotation etc. So I'm slightly nervous about going ahead with this right now. While I generally agree with the principal, I also wonder if it might be better to just add this field in log_line_prefix and wait for someone to complain about that as other than a theoretical matter. I might be working against myself, but I'll complain right now about the lack of any way to have a header on the CSV logs and that you don't get to control what fields are logged. That said, I'm not currently using them either, so my vote doesn't count for much. Of course, I'll also complain about the lack of any way to get PG to respect the header, forcing me to do fun things like: for file in *results*; do HEADER=`head -1 $file` sed -e 's:""::g'< $file | \ psql -d beac -h sauron -c \ "\copy my_table ($HEADER) from STDIN with csv header" done on a regular basis. How forcing me to do that rather than asking someone else to use 'tail -n +2' makes sense is beyond me.. You don't really make your case any better by continuing this argument from years ago. I can tell you from experience that the CSV HEADER feature is distinctly useful as it is. If you want to add a mode that uses the header line as a column list on import, then make that case, and I'll support it in fact, but it's not an alternative to having the header ignored, which is a feature I would vigorously resist removing. (Incidentally, I think it won't be trivial - the COPY code expects to know the columns by the time it opens the file). 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] XMin Hot Standby Feedback patch
On 15.02.2011 18:52, Robert Haas wrote: On Tue, Feb 15, 2011 at 11:49 AM, Heikki Linnakangas wrote: It would be wise to also transmit the epoch in addition to xmin, to avoid confusion if the standby is> 2 billion transactions behind. That case is probably hopelessly broken anyway. I don't expect the feedback to do anything too useful in case of xid wraparound, but at least the master would recognize the situation and know to ignore the bogus xmin from that standby. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XMin Hot Standby Feedback patch
On Tue, Feb 15, 2011 at 11:49 AM, Heikki Linnakangas wrote: > On 15.02.2011 18:42, Simon Riggs wrote: >> >> On Sat, 2011-02-12 at 14:11 -0800, Daniel Farina wrote: >>> >>> This is another bit of the syncrep patch split out. >>> >>> I will revisit the replication timeout one Real Soon, I promise -- but >>> I have a couple things to do today that may delay that until the >>> evening. >>> >>> >>> https://github.com/fdr/postgres/commit/ad3ce9ac62f0e128d7d1fd20d47184f867056af1 >>> >>> Context diff supplied here. >> >> Greg just tipped me off to this thread a few hours ago. I saw your other >> work on timeouts which looks good. >> >> I've reworked this feature myself, and its roughly the same thing you >> have posted, so I will just add on to this thread. The major change from >> my earlier patch is that the logic around setting xmin on the master is >> considerably tighter, and correctly uses locking. > > It would be wise to also transmit the epoch in addition to xmin, to avoid > confusion if the standby is > 2 billion transactions behind. That case is probably hopelessly broken anyway. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql contrib module
On Tue, Feb 15, 2011 at 11:41 AM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane wrote: >>> Robert Haas writes: Those are good points. My point was just that you can't actually build that file at the time you RUN the regression tests, because you have to build it first, then install it, then run the regression tests. It could be a separate target, like 'make policy', but I don't think it works to make it part of 'make installcheck'. > >>> So? Once you admit that you can do that, it's a matter of a couple more >>> lines to make the installcheck target depend on the policy target iff >>> selinux was enabled. > >> Sure, you could do that, but I don't see what problem it would fix. >> You'd still have to build and manually install the policy before you >> could run make installcheck. And once you've done that, you don't >> need to rebuild it every future time you run make installcheck. > > Oh, I see: you're pointing out the root-only "semodule" step that has to > be done in between there. Good point. But the current arrangement is > still a mistake: the required contents of sepgsql-regtest.pp depend on > the configuration of the test system, which can't be known at build > time. > > So what we should do is offer a "make policy" target and alter the test > instructions to say you should do that and then run semodule. Or maybe > just put the whole "make -f /usr/share/selinux/devel/Makefile" dance > into the instructions --- it doesn't look to me like our makefile > infrastructure really has anything useful to add to that. Yeah, agreed. -- 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] XMin Hot Standby Feedback patch
On 15.02.2011 18:42, Simon Riggs wrote: On Sat, 2011-02-12 at 14:11 -0800, Daniel Farina wrote: This is another bit of the syncrep patch split out. I will revisit the replication timeout one Real Soon, I promise -- but I have a couple things to do today that may delay that until the evening. https://github.com/fdr/postgres/commit/ad3ce9ac62f0e128d7d1fd20d47184f867056af1 Context diff supplied here. Greg just tipped me off to this thread a few hours ago. I saw your other work on timeouts which looks good. I've reworked this feature myself, and its roughly the same thing you have posted, so I will just add on to this thread. The major change from my earlier patch is that the logic around setting xmin on the master is considerably tighter, and correctly uses locking. It would be wise to also transmit the epoch in addition to xmin, to avoid confusion if the standby is > 2 billion transactions behind. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] XMin Hot Standby Feedback patch
On Sat, 2011-02-12 at 14:11 -0800, Daniel Farina wrote: > This is another bit of the syncrep patch split out. > > I will revisit the replication timeout one Real Soon, I promise -- but > I have a couple things to do today that may delay that until the > evening. > > https://github.com/fdr/postgres/commit/ad3ce9ac62f0e128d7d1fd20d47184f867056af1 > > Context diff supplied here. Greg just tipped me off to this thread a few hours ago. I saw your other work on timeouts which looks good. I've reworked this feature myself, and its roughly the same thing you have posted, so I will just add on to this thread. The major change from my earlier patch is that the logic around setting xmin on the master is considerably tighter, and correctly uses locking. Patch attached, no docs yet, but the patch is clear. I'm looking to commit this in next 24 hours barring objections and/or test failures. > Note that this information is not exposed via catalog in the original > syncrep patch, and is not here. Do we want that kind of reporting? Probably, but its a small change and will conflict with other work for now. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 3277da8..c4ebf97 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -45,6 +45,7 @@ #include "replication/walreceiver.h" #include "storage/ipc.h" #include "storage/pmsignal.h" +#include "storage/procarray.h" #include "utils/builtins.h" #include "utils/guc.h" #include "utils/memutils.h" @@ -56,6 +57,7 @@ bool am_walreceiver; /* GUC variable */ int wal_receiver_status_interval; +bool hot_standby_feedback; /* libpqreceiver hooks to these when loaded */ walrcv_connect_type walrcv_connect = NULL; @@ -610,10 +612,14 @@ XLogWalRcvSendReply(void) reply_message.apply = GetXLogReplayRecPtr(); reply_message.sendTime = now; - elog(DEBUG2, "sending write %X/%X flush %X/%X apply %X/%X", + if (hot_standby_feedback) + reply_message.xmin = GetOldestXmin(true, false); + + elog(DEBUG2, "sending write %X/%X flush %X/%X apply %X/%X xmin %u", reply_message.write.xlogid, reply_message.write.xrecoff, reply_message.flush.xlogid, reply_message.flush.xrecoff, - reply_message.apply.xlogid, reply_message.apply.xrecoff); + reply_message.apply.xlogid, reply_message.apply.xrecoff, + reply_message.xmin); /* Prepend with the message type and send it. */ buf[0] = 'r'; diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 3ad95b4..36eb3a9 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -53,6 +53,7 @@ #include "storage/ipc.h" #include "storage/pmsignal.h" #include "storage/proc.h" +#include "storage/procarray.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -498,6 +499,7 @@ ProcessStandbyReplyMessage(void) static StringInfoData input_message; StandbyReplyMessage reply; char msgtype; + TransactionId newxmin = InvalidTransactionId; initStringInfo(&input_message); @@ -524,10 +526,11 @@ ProcessStandbyReplyMessage(void) pq_copymsgbytes(&input_message, (char *) &reply, sizeof(StandbyReplyMessage)); - elog(DEBUG2, "write %X/%X flush %X/%X apply %X/%X ", + elog(DEBUG2, "write %X/%X flush %X/%X apply %X/%X xmin %u", reply.write.xlogid, reply.write.xrecoff, reply.flush.xlogid, reply.flush.xrecoff, - reply.apply.xlogid, reply.apply.xrecoff); + reply.apply.xlogid, reply.apply.xrecoff, + reply.xmin); /* * Update shared state for this WalSender process @@ -543,6 +546,74 @@ ProcessStandbyReplyMessage(void) walsnd->apply = reply.apply; SpinLockRelease(&walsnd->mutex); } + + /* + * Update the WalSender's proc xmin to allow it to be visible + * to snapshots. This will hold back the removal of dead rows + * and thereby prevent the generation of cleanup conflicts + * on the standby server. + */ + if (TransactionIdIsValid(reply.xmin)) + { + TransactionId safeLimit; +#define HOT_STANDBY_FEEDBACK_HORIZON 10 + + if (!TransactionIdIsValid(MyProc->xmin)) + { + /* + * Initialise MyProc->xmin from standby feedback. + * Don't allow use oldestXmin if it is too far back. + */ + safeLimit = ReadNewTransactionId() - HOT_STANDBY_FEEDBACK_HORIZON; + if (!TransactionIdIsNormal(safeLimit)) +safeLimit = FirstNormalTransactionId; + + if (TransactionIdPrecedes(safeLimit, reply.xmin)) + { +TransactionId oldestXmin = GetOldestXmin(true, true); + +if (TransactionIdPrecedes(oldestXmin, reply.xmin)) + newxmin = reply.xmin; +else + newxmin = oldestXmin; + } + else +ereport(WARNING, + (errmsg("standby xmin is far in the past"), + errhint("standby must catch up before hot standby feedback is effective."))); + } + else + { + /
Re: [HACKERS] sepgsql contrib module
Robert Haas writes: > On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane wrote: >> Robert Haas writes: >>> Those are good points. My point was just that you can't actually >>> build that file at the time you RUN the regression tests, because you >>> have to build it first, then install it, then run the regression >>> tests. It could be a separate target, like 'make policy', but I don't >>> think it works to make it part of 'make installcheck'. >> So? Once you admit that you can do that, it's a matter of a couple more >> lines to make the installcheck target depend on the policy target iff >> selinux was enabled. > Sure, you could do that, but I don't see what problem it would fix. > You'd still have to build and manually install the policy before you > could run make installcheck. And once you've done that, you don't > need to rebuild it every future time you run make installcheck. Oh, I see: you're pointing out the root-only "semodule" step that has to be done in between there. Good point. But the current arrangement is still a mistake: the required contents of sepgsql-regtest.pp depend on the configuration of the test system, which can't be known at build time. So what we should do is offer a "make policy" target and alter the test instructions to say you should do that and then run semodule. Or maybe just put the whole "make -f /usr/share/selinux/devel/Makefile" dance into the instructions --- it doesn't look to me like our makefile infrastructure really has anything useful to add to that. 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] extensions and psql
Hi, I realize that you didn't keep the \dx behavior I had, that when given an extension name it would list all the objects contained in the extension. Now that's a pretty simple query: select pg_describe_object(classid, objid, 0) from pg_depend d join pg_extension e on d.refclassid = e.tableoid and d.refobjid = e.oid and d.deptype = 'e' where e.extname = 'hstore' order by objid; Do we want to get that back in, and in which psql command? It could well be that having \dx list extension and \dx name list extension's objects wasn't the best design around, and it could be that it's not useful enough, but I know I liked to have a psql shortcut to do that. 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] Sync Rep for 2011CF1
On Tue, 2011-02-15 at 01:45 -0500, Jaime Casanova wrote: > 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 OK, thanks. > + 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 I think that code is being ripped out, so will check again later. -- 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] Add support for logging the current role
On Tue, Feb 15, 2011 at 11:13 AM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> Well, I guess the other option is to just add it to the format, full >> stop. But as someone pointed out previously, that's not a terribly >> scalable solution, but perhaps it could be judged adequate for this >> particular case. > > Think I suggested that at one point. I'm all for doing that on a major > version change like this one, but I think we already had some concerns > about that on this thread (Andrew maybe?). > >> While I generally agree with the principal, I also wonder if it might >> be better to just add this field in log_line_prefix and wait for >> someone to complain about that as other than a theoretical matter. > > I might be working against myself, but I'll complain right now about the > lack of any way to have a header on the CSV logs and that you don't get > to control what fields are logged. That said, I'm not currently using > them either, so my vote doesn't count for much. Of course, I'll also > complain about the lack of any way to get PG to respect the header, > forcing me to do fun things like: > > for file in *results*; do > HEADER=`head -1 $file` > sed -e 's:""::g' < $file | \ > psql -d beac -h sauron -c \ > "\copy my_table ($HEADER) from STDIN with csv header" > done > > on a regular basis. How forcing me to do that rather than asking > someone else to use 'tail -n +2' makes sense is beyond me.. It's not an either/or proposition. We could certainly support header on/off/ignore, with the new extensible COPY syntax. -- 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] pageinspect's infomask and infomask2 as smallint
On 15.02.2011 18:03, Tom Lane wrote: Robert Haas writes: On Tue, Feb 15, 2011 at 10:53 AM, Tom Lane wrote: What risk? And at least we'd be trying to do it cleanly, in a manner that should work for at least 99% of users. AFAICT, Heikki's proposal is "break it for everyone, and damn the torpedoes". I must be confused. I thought Heikki's proposal was "fix it in 9.1, because incompatibilities are an expected part of major release upgrades, but don't break it in 9.0 and prior, because it's not particularly important and we don't want to change behavior or risk breaking things in minor releases". Right, that's what I meant. No, nobody was proposing changing it before 9.1 (or at least I didn't think anybody was). What's under discussion is how much effort to put into making a 9.0-to-9.1 upgrade go smoothly for people who have the function installed. Oh, never mind then. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
* Robert Haas (robertmh...@gmail.com) wrote: > Well, I guess the other option is to just add it to the format, full > stop. But as someone pointed out previously, that's not a terribly > scalable solution, but perhaps it could be judged adequate for this > particular case. Think I suggested that at one point. I'm all for doing that on a major version change like this one, but I think we already had some concerns about that on this thread (Andrew maybe?). > While I generally agree with the principal, I also wonder if it might > be better to just add this field in log_line_prefix and wait for > someone to complain about that as other than a theoretical matter. I might be working against myself, but I'll complain right now about the lack of any way to have a header on the CSV logs and that you don't get to control what fields are logged. That said, I'm not currently using them either, so my vote doesn't count for much. Of course, I'll also complain about the lack of any way to get PG to respect the header, forcing me to do fun things like: for file in *results*; do HEADER=`head -1 $file` sed -e 's:""::g' < $file | \ psql -d beac -h sauron -c \ "\copy my_table ($HEADER) from STDIN with csv header" done on a regular basis. How forcing me to do that rather than asking someone else to use 'tail -n +2' makes sense is beyond me.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] sepgsql contrib module
On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane wrote: > Robert Haas writes: >> Those are good points. My point was just that you can't actually >> build that file at the time you RUN the regression tests, because you >> have to build it first, then install it, then run the regression >> tests. It could be a separate target, like 'make policy', but I don't >> think it works to make it part of 'make installcheck'. > > So? Once you admit that you can do that, it's a matter of a couple more > lines to make the installcheck target depend on the policy target iff > selinux was enabled. Sure, you could do that, but I don't see what problem it would fix. You'd still have to build and manually install the policy before you could run make installcheck. And once you've done that, you don't need to rebuild it every future time you run make installcheck. -- 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] pageinspect's infomask and infomask2 as smallint
On Tue, Feb 15, 2011 at 11:03 AM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Feb 15, 2011 at 10:53 AM, Tom Lane wrote: >>> What risk? And at least we'd be trying to do it cleanly, in a manner >>> that should work for at least 99% of users. AFAICT, Heikki's proposal >>> is "break it for everyone, and damn the torpedoes". > >> I must be confused. I thought Heikki's proposal was "fix it in 9.1, >> because incompatibilities are an expected part of major release >> upgrades, but don't break it in 9.0 and prior, because it's not >> particularly important and we don't want to change behavior or risk >> breaking things in minor releases". > > No, nobody was proposing changing it before 9.1 (or at least I didn't > think anybody was). What's under discussion is how much effort to put > into making a 9.0-to-9.1 upgrade go smoothly for people who have the > function installed. Oh, I see, never mind me then... feel free to make that go smoothly. :-) -- 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] Add support for logging the current role
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Given that this has been like this right along, I don't see why it's > all that urgent to force a half-baked solution into 9.1. I'm also > concerned that if we do do that, you'll lose motivation to work on > cleaning it up for 9.2 ;-) The addition to log_line_prefix is hardly 'half-baked' as a solution, to that problem (I just pulled the hunks from the rest of the patch as they were completely independent, and tested them). I've also gone and added the csvlog_fields/csvlog_header patch to the 2011-Next commitfest. :P I've also already started looking at changing syslogger to have it figure out if it should be writing a header out or not. If we can decide what semantics we should have when the log file exists and we're not planning to rotate it on startup, it won't be hard for me to implement them (well, I hope). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
On Tue, Feb 15, 2011 at 10:57 AM, Tom Lane wrote: > Robert Haas writes: >> Something along these lines would be OK with me (I haven't yet >> validated every detail), but there were previous objections to adding >> any new fields to log_line_prefix until we had a flexible CSV format. >> I think that's raising the bar a bit too high, personally, but I don't >> have the only vote around here... > > I think I was the one objecting. I don't necessarily say that we have > to have a "flexible" CSV format, but I do say that facilities that are > available in log_line_prefix and not in CSV logs are a bad thing. Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for this particular case. While I generally agree with the principal, I also wonder if it might be better to just add this field in log_line_prefix and wait for someone to complain about that as other than a theoretical matter. -- 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