Thread: wrapping up this CommitFest (was Re: knngist - 0.8)
On Tue, Mar 1, 2011 at 1:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Mon, Feb 28, 2011 at 2:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Given that it is a contrib module, I personally wouldn't object to it >>> getting patched later, like during alpha or beta. But somebody's got >>> to do the work, and I've got a dozen higher-priority problems right now. > >> Well, we can argue about whether it's too late for 9.1 if and when a >> patch shows up. Right now we don't have that problem. > > We do now ... > http://archives.postgresql.org/pgsql-hackers/2011-03/msg00038.php > > Since we appear to be still holding the commitfest open for Sync Rep, > I guess this ought to get reviewed. Or else we should close the CommitFest and cut alpha4. Anyone have an opinion on which way to go? I think it's fair to say that Simon is working pretty actively on Sync Rep and that the bug count is probably dropping rapidly. It seems a shame to push sync rep out to 9.2 in that context. On the other hand, the patch wasn't done at the beginning of the CommitFest, it wasn't done at the scheduled end of the CommitFest, and it's still not done now two weeks after the scheduled end of the CommitFest. If it gets committed O(now), it's probably going to still have bugs and design problems that will take at least a few more weeks to shake out, which will directly add to the length of time that it takes to actually get the release out the door. I could go either way on this one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
>> Since we appear to be still holding the commitfest open for Sync Rep,
>> I guess this ought to get reviewed.
>
> Or else we should close the CommitFest and cut alpha4. Anyone have an
> opinion on which way to go?
I think we can give Sync Rep until the 15th, given the pace of work on
it. It is a major feature, and a complicated one.
We could even cut a pre-synch-rep Alpha4 *now*, and follow that with a
post-synch-rep Alpha5 sometime around April 1.
That'll put us in a good position for beta, and also to see what
specific issue SynR adds.
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
On Tue, Mar 1, 2011 at 2:12 PM, Josh Berkus <josh@agliodbs.com> wrote: > >>> Since we appear to be still holding the commitfest open for Sync Rep, >>> I guess this ought to get reviewed. >> >> Or else we should close the CommitFest and cut alpha4. Anyone have an >> opinion on which way to go? > > I think we can give Sync Rep until the 15th, given the pace of work on > it. It is a major feature, and a complicated one. Sure, but there are other features, major and minor, that we have postponed to 9.2. In the normal course of events, sync rep would have been marked Returned with Feedback a month ago. I like the feature, but I have to say I'm not very pleased that we seem to have fallen into a pattern of believing that some major features are somehow exempted from the scheduling deadline and others are not. I am sure there are plenty of other people who would have liked a six week extension of the usual CommitFest deadlines too, but they didn't get it (and for the most part, were pretty gracious about that). > We could even cut a pre-synch-rep Alpha4 *now*, and follow that with a > post-synch-rep Alpha5 sometime around April 1. > > That'll put us in a good position for beta, and also to see what > specific issue SynR adds. Frankly, I think we should be aiming to get a beta out in April, not another alpha. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Mar 1, 2011 at 20:26, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Mar 1, 2011 at 2:12 PM, Josh Berkus <josh@agliodbs.com> wrote: >> >>>> Since we appear to be still holding the commitfest open for Sync Rep, >>>> I guess this ought to get reviewed. >>> >>> Or else we should close the CommitFest and cut alpha4. Anyone have an >>> opinion on which way to go? >> >> I think we can give Sync Rep until the 15th, given the pace of work on >> it. It is a major feature, and a complicated one. > > Sure, but there are other features, major and minor, that we have > postponed to 9.2. In the normal course of events, sync rep would have > been marked Returned with Feedback a month ago. I like the feature, > but I have to say I'm not very pleased that we seem to have fallen > into a pattern of believing that some major features are somehow > exempted from the scheduling deadline and others are not. I am sure > there are plenty of other people who would have liked a six week > extension of the usual CommitFest deadlines too, but they didn't get > it (and for the most part, were pretty gracious about that). > >> We could even cut a pre-synch-rep Alpha4 *now*, and follow that with a >> post-synch-rep Alpha5 sometime around April 1. >> >> That'll put us in a good position for beta, and also to see what >> specific issue SynR adds. > > Frankly, I think we should be aiming to get a beta out in April, not > another alpha. That would be good, but in order to get there, +1 for cutting a new alpha even if syncrep isn't ready for it yet. That way we can at least get some more testing on all the non-syncrep code. That is assuming that cutting an alpha release isn't all that much work, but IIRC it's not. (Hey, I know it's not much work for me, but someone who actually does the work should comment on the total amount...) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Mar 1, 2011 at 2:12 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> I think we can give Sync Rep until the 15th, given the pace of work on
>> it. �It is a major feature, and a complicated one.
> Sure, but there are other features, major and minor, that we have
> postponed to 9.2. In the normal course of events, sync rep would have
> been marked Returned with Feedback a month ago. I like the feature,
> but I have to say I'm not very pleased that we seem to have fallen
> into a pattern of believing that some major features are somehow
> exempted from the scheduling deadline and others are not.
Yes. What are the rest of us supposed to do for the next two weeks,
twiddle our thumbs?
Personally I've got a couple of days' worth of cleanup tasks before I'd
want to see us cut an alpha anyway, especially if we're going to try
to accept the btree_gist KNNgist patch. Two weeks is too much though.
I'd say that if there's a plausible chance that Sync Rep will be
committable by the end of *this* week (and I mean Friday not Sunday),
I'm willing to wait that long for it. Otherwise, it's 9.2 material.
> Frankly, I think we should be aiming to get a beta out in April, not
> another alpha.
Quite.
regards, tom lane
On Tue, Mar 1, 2011 at 2:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Mar 1, 2011 at 2:12 PM, Josh Berkus <josh@agliodbs.com> wrote: >>> I think we can give Sync Rep until the 15th, given the pace of work on >>> it. It is a major feature, and a complicated one. > >> Sure, but there are other features, major and minor, that we have >> postponed to 9.2. In the normal course of events, sync rep would have >> been marked Returned with Feedback a month ago. I like the feature, >> but I have to say I'm not very pleased that we seem to have fallen >> into a pattern of believing that some major features are somehow >> exempted from the scheduling deadline and others are not. > > Yes. What are the rest of us supposed to do for the next two weeks, > twiddle our thumbs? > > Personally I've got a couple of days' worth of cleanup tasks before I'd > want to see us cut an alpha anyway, especially if we're going to try > to accept the btree_gist KNNgist patch. Two weeks is too much though. > > I'd say that if there's a plausible chance that Sync Rep will be > committable by the end of *this* week (and I mean Friday not Sunday), > I'm willing to wait that long for it. Otherwise, it's 9.2 material. I am quite sure that Simon will be able to get something committed ahead of whatever deadline we choose to set. Whether that commit will be up to our usual standards is another question altogether. The last version posted to the list was trivial to break, and that was several weeks ago. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Mar 1, 2011 at 2:38 PM, Magnus Hagander <magnus@hagander.net> wrote: >> Frankly, I think we should be aiming to get a beta out in April, not >> another alpha. > > That would be good, but in order to get there, +1 for cutting a new > alpha even if syncrep isn't ready for it yet. That way we can at least > get some more testing on all the non-syncrep code. > > That is assuming that cutting an alpha release isn't all that much > work, but IIRC it's not. (Hey, I know it's not much work for me, but > someone who actually does the work should comment on the total > amount...) As I understand it, it requires only the steps described here: http://wiki.postgresql.org/wiki/Alpha_release_process That all looks pretty straightforward, assuming you can log into developer.postgresql.org, which I can't. I think I remember having an ftp account at some point, but it's not accepting connections on port 22, so there is doubtless some secret sauce I am missing here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> As I understand it, it requires only the steps described here: > > http://wiki.postgresql.org/wiki/Alpha_release_process > > That all looks pretty straightforward, assuming you can log into > developer.postgresql.org, which I can't. I think I remember having an > ftp account at some point, but it's not accepting connections on port > 22, so there is doubtless some secret sauce I am missing here. Ideally, we want to have some binaries/packages for the "final alpha". Those broaden testing considerably. We don't need them for all platforms, of course. Really, the critical ones for testing are Windows and OSX. Linux/BSD/Solaris users are pretty good at make/make install. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.com
On Tue, Mar 1, 2011 at 2:52 PM, Josh Berkus <josh@agliodbs.com> wrote: > Ideally, we want to have some binaries/packages for the "final alpha". > Those broaden testing considerably. When we have a version that needs that treatment, we can simply call it beta1. If it's too half-baked for that, then I don't see the point in going to a lot of trouble to build packages. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Mar 1, 2011 at 2:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'd say that if there's a plausible chance that Sync Rep will be
>> committable by the end of *this* week (and I mean Friday not Sunday),
>> I'm willing to wait that long for it. �Otherwise, it's 9.2 material.
> I am quite sure that Simon will be able to get something committed
> ahead of whatever deadline we choose to set. Whether that commit will
> be up to our usual standards is another question altogether. The last
> version posted to the list was trivial to break, and that was several
> weeks ago.
Yeah, there's that. It's difficult to believe that anything committed
in the very short term wouldn't be rushed to completion rather than
really ready. That holds whether the deadline is this week or two
weeks out.
The other issue is that, as Robert says, we have already cut Sync Rep
more slack than any other patch in the commitfest. It does not seem
fair to hold up the release process another week or two for it, even
assuming that we get a high-quality feature at the end of that.
If we do hold up the release, I'll probably go back and reopen the
postgresql_fdw patch as well as btree_gist. So I won't run out of
things to do. But I'm not terribly satisfied with the decision-making
process here.
regards, tom lane
On 3/1/11 11:58 AM, Tom Lane wrote:
> If we do hold up the release, I'll probably go back and reopen the
> postgresql_fdw patch as well as btree_gist. So I won't run out of
> things to do. But I'm not terribly satisfied with the decision-making
> process here.
OK, well, we gave it an extra 15 days, which was all we promised.
I'm ok with closing things as of the end of the 15 days, say Thursday or
Friday.
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Mar 1, 2011 at 2:52 PM, Josh Berkus <josh@agliodbs.com> wrote: >> Ideally, we want to have some binaries/packages for the "final alpha". >> Those broaden testing considerably. > When we have a version that needs that treatment, we can simply call > it beta1. If it's too half-baked for that, then I don't see the point > in going to a lot of trouble to build packages. We (or more precisely EDB) made Windows installers for alpha1: http://www.enterprisedb.com/products-services-training/pgdevdownload And IIRC they did installers for alphas in the 9.0 cycle too. And certainly Devrim and others have been building binary packages for alphas. If this alpha is so much less baked than the previous ones that that's not worthwhile, there's something very wrong with the process. The last alpha ought to be in testable condition. regards, tom lane
On Tue, Mar 1, 2011 at 3:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Mar 1, 2011 at 2:52 PM, Josh Berkus <josh@agliodbs.com> wrote: >>> Ideally, we want to have some binaries/packages for the "final alpha". >>> Those broaden testing considerably. > >> When we have a version that needs that treatment, we can simply call >> it beta1. If it's too half-baked for that, then I don't see the point >> in going to a lot of trouble to build packages. > > We (or more precisely EDB) made Windows installers for alpha1: > http://www.enterprisedb.com/products-services-training/pgdevdownload > And IIRC they did installers for alphas in the 9.0 cycle too. And > certainly Devrim and others have been building binary packages for > alphas. If this alpha is so much less baked than the previous ones > that that's not worthwhile, there's something very wrong with the > process. The last alpha ought to be in testable condition. Oh, really? OK. I wasn't aware that alphas got installers. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Mar 1, 2011 at 2:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Mar 1, 2011 at 2:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> I'd say that if there's a plausible chance that Sync Rep will be >>> committable by the end of *this* week (and I mean Friday not Sunday), >>> I'm willing to wait that long for it. Otherwise, it's 9.2 material. > >> I am quite sure that Simon will be able to get something committed >> ahead of whatever deadline we choose to set. Whether that commit will >> be up to our usual standards is another question altogether. The last >> version posted to the list was trivial to break, and that was several >> weeks ago. > > Yeah, there's that. It's difficult to believe that anything committed > in the very short term wouldn't be rushed to completion rather than > really ready. That holds whether the deadline is this week or two > weeks out. Yep. > The other issue is that, as Robert says, we have already cut Sync Rep > more slack than any other patch in the commitfest. It does not seem > fair to hold up the release process another week or two for it, even > assuming that we get a high-quality feature at the end of that. Yep. > If we do hold up the release, I'll probably go back and reopen the > postgresql_fdw patch as well as btree_gist. So I won't run out of > things to do. But I'm not terribly satisfied with the decision-making > process here. Well, we haven't actually made a decision here yet. We're just talking about what decision we ought to make. Frankly, I avoided trying to mark Sync Rep Returned with Feedback mostly for the reason that I knew Simon would object, and his commit bit gives him a certain degree of latitude to ignore the CF process anyway. But I am really not that keen on having Sync Rep go in and then spending another month fixing all the bugs, and that's what I think will happen. Bruce has been going through the open items for the past several weeks (at least) and tells me that he hasn't found very much. I'm not sure what your thought is on what's required to get us from here to beta, but I am thinking it could be done in a few weeks. With a concerted effort and some sustained focus, I don't see why we could get this release out the door in, say, three months. Taking in a feature that's going to take another month to sort out is going to push that out, and I am really not excited about another round of spend-all-summer-waiting-for-people-to-get-back-from-vacation-and-release-in-September. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, 2011-03-01 at 14:26 -0500, Robert Haas wrote: > On Tue, Mar 1, 2011 at 2:12 PM, Josh Berkus <josh@agliodbs.com> wrote: > > > >>> Since we appear to be still holding the commitfest open for Sync Rep, > >>> I guess this ought to get reviewed. > >> > >> Or else we should close the CommitFest and cut alpha4. Anyone have an > >> opinion on which way to go? > > > > I think we can give Sync Rep until the 15th, given the pace of work on > > it. It is a major feature, and a complicated one. > > Sure, but there are other features, major and minor, that we have > postponed to 9.2. In the normal course of events, sync rep would have > been marked Returned with Feedback a month ago. I like the feature, > but I have to say I'm not very pleased that we seem to have fallen > into a pattern of believing that some major features are somehow > exempted from the scheduling deadline and others are not. Neither am I, I mean, we were actively fixing and bringing Fk-Locks up to date and we got pushed but sync rep gets in? We may have had fk-locks baked by now. I want syncrep as much as the next person, but this isn't really fair to any of the other submitters. JD -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt
On Tue, Mar 1, 2011 at 3:01 PM, Josh Berkus <josh@agliodbs.com> wrote: > On 3/1/11 11:58 AM, Tom Lane wrote: >> If we do hold up the release, I'll probably go back and reopen the >> postgresql_fdw patch as well as btree_gist. So I won't run out of >> things to do. But I'm not terribly satisfied with the decision-making >> process here. > > OK, well, we gave it an extra 15 days, which was all we promised. > > I'm ok with closing things as of the end of the 15 days, say Thursday or > Friday. That's still missing the point, which is that the code is unlikely to be up to our usual standards at that point. So far I don't hear anyone arguing strongly that we should accept sync rep in 9.1, and several people arguing the reverse. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Josh Berkus <josh@agliodbs.com> writes: > I'm ok with closing things as of the end of the 15 days, say Thursday or > Friday. It might be a good idea to make a list of what we have left to do before we can wrap an alpha. Here are some things on my list. Not all of them are necessarily release blockers, but we need to discuss which ones are: * Regression test failures from recent plpython patches. These are affecting enough machines to make them "must fix before alpha", IMO. There are some variations in error message wording, which are not too terrible but also not exactly hard to fix. The python assert failure that some Fedora machines are reporting is considerably more disturbing. * Collation-related regression failure on buildfarm member pika. This is clearly a bug we need to identify, but maybe we can ship the alpha without a fix --- for one thing, getting more than one report of the problem would be helpful. * Collation-related changes still needed in contrib/citext. If we don't have this done before alpha4, I'm afraid we'll have to generate a 1.1 update script to fix it for alpha4 users. I'd just as soon not deal with that overhead. * Rearrange pg_proc and pg_operator comments as suggested here: http://archives.postgresql.org/pgsql-docs/2010-10/msg00041.php While this isn't a "must fix", the very end of a development cycle is clearly the best time for such a patch, since at any other time there are going to be a larger number of pending patches that would have to be adjusted. So I'd kind of like to get this done, if I can spare a day for it. * Generate alpha release notes. This is at least half a day's work for somebody, I think, even with our fairly low standards for alpha release notes. There are other things I'd like to do, like review and perhaps commit the btree_gist patch, but they're not at the level of "must fix". Any other "must fix" items on people's minds? regards, tom lane
> That's still missing the point, which is that the code is unlikely to
> be up to our usual standards at that point.
I was talking about "when do we close the CF and start building an
alpha", not synch rep particularly. Tom said he wants until Friday
anyway just for some cleanup.
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
On Tue, Mar 1, 2011 at 3:36 PM, Josh Berkus <josh@agliodbs.com> wrote: > >> That's still missing the point, which is that the code is unlikely to >> be up to our usual standards at that point. > > I was talking about "when do we close the CF and start building an > alpha", not synch rep particularly. Tom said he wants until Friday > anyway just for some cleanup. That response is just dodging the hard question, so whatever. Tom's cleanup is not going to break things, or at least it's going to fix more than it breaks on net. Sync rep, on the other hand, is going to do the opposite, probably by a large margin. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> Bruce has been going through the open items for the past several weeks
> (at least) and tells me that he hasn't found very much. I'm not sure
> what your thought is on what's required to get us from here to beta,
> but I am thinking it could be done in a few weeks. With a concerted
> effort and some sustained focus, I don't see why we could get this
> release out the door in, say, three months. Taking in a feature
> that's going to take another month to sort out is going to push that
> out, and I am really not excited about another round of
> spend-all-summer-waiting-for-people-to-get-back-from-vacation-and-release-in-September.
Yeah, it would be really nice to get the release out in June rather than
September. If we wait any longer for Sync Rep I'm pretty sure it's
going to be the latter not the former.
See my nearby message for a start at a list of what we "must" do to
get to alpha4. Any features we want to cram in at this stage go on
top of that.
regards, tom lane
> That response is just dodging the hard question, so whatever. Tom's
> cleanup is not going to break things, or at least it's going to fix
> more than it breaks on net. Sync rep, on the other hand, is going to
> do the opposite, probably by a large margin.
Well, we need to at least give Simon and Fujii a chance to respond
during their respective daytime hours.
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
On 3/1/11 12:35 PM, Tom Lane wrote:
> * Generate alpha release notes. This is at least half a day's work
> for somebody, I think, even with our fairly low standards for alpha
> release notes.
I can help with this. Possibly Selena can too.
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
On Mar 1, 2011, at 12:35 PM, Tom Lane wrote: > * Collation-related changes still needed in contrib/citext. If we don't > have this done before alpha4, I'm afraid we'll have to generate a 1.1 > update script to fix it for alpha4 users. I'd just as soon not deal > with that overhead. What needs to happen here, exactly? I'm ignorant about about the correlation changes, but need to learn, and of course amwilling to help out with citext, especially if correlation support might make it better. David
On 01/03/11 21:35, Tom Lane wrote: > Josh Berkus <josh@agliodbs.com> writes: >> I'm ok with closing things as of the end of the 15 days, say Thursday or >> Friday. > > It might be a good idea to make a list of what we have left to do before > we can wrap an alpha. Here are some things on my list. Not all of them > are necessarily release blockers, but we need to discuss which ones are: > > * Regression test failures from recent plpython patches. These are > affecting enough machines to make them "must fix before alpha", IMO. > There are some variations in error message wording, which are not too > terrible but also not exactly hard to fix. The python assert failure > that some Fedora machines are reporting is considerably more disturbing. I'm looking into the crash, no luck so long. Jan
On 03/01/2011 03:53 PM, Jan Urbański wrote: > On 01/03/11 21:35, Tom Lane wrote: >> Josh Berkus<josh@agliodbs.com> writes: >>> I'm ok with closing things as of the end of the 15 days, say Thursday or >>> Friday. >> It might be a good idea to make a list of what we have left to do before >> we can wrap an alpha. Here are some things on my list. Not all of them >> are necessarily release blockers, but we need to discuss which ones are: >> >> * Regression test failures from recent plpython patches. These are >> affecting enough machines to make them "must fix before alpha", IMO. >> There are some variations in error message wording, which are not too >> terrible but also not exactly hard to fix. The python assert failure >> that some Fedora machines are reporting is considerably more disturbing. I agree. > I'm looking into the crash, no luck so long. > > Is there anything you need that would help you? cheers andrew
On tis, 2011-03-01 at 12:50 -0800, David E. Wheeler wrote: > On Mar 1, 2011, at 12:35 PM, Tom Lane wrote: > > > * Collation-related changes still needed in contrib/citext. If we don't > > have this done before alpha4, I'm afraid we'll have to generate a 1.1 > > update script to fix it for alpha4 users. I'd just as soon not deal > > with that overhead. > > What needs to happen here, exactly? I'm ignorant about about the correlation changes, but need to learn, and of courseam willing to help out with citext, especially if correlation support might make it better. I think you just need to put something like UPDATE pg_type SET typcollation = (SELECT oid FROM pg_collation WHERE collname = 'default') WHERE typname = 'citext'; into the upgrade script. The discussion last time was whether there should be ALTER TYPE support for that, but I guess we settled on not bothering.
"David E. Wheeler" <david@kineticode.com> writes: > On Mar 1, 2011, at 12:35 PM, Tom Lane wrote: >> * Collation-related changes still needed in contrib/citext. If we don't >> have this done before alpha4, I'm afraid we'll have to generate a 1.1 >> update script to fix it for alpha4 users. I'd just as soon not deal >> with that overhead. > What needs to happen here, exactly? I'm ignorant about about the correlation changes, but need to learn, and of courseam willing to help out with citext, especially if correlation support might make it better. Collation, not correlation. The question is what collation property the citext type needs to have, and how we get it to have that setting during an upgrade from 9.0. See http://archives.postgresql.org/message-id/11548.1297983024@sss.pgh.pa.us regards, tom lane
On Mar 1, 2011, at 1:12 PM, Tom Lane wrote: > Collation, not correlation. Yeah, I'm fat-fingered today. > The question is what collation property the > citext type needs to have, and how we get it to have that setting during > an upgrade from 9.0. See > http://archives.postgresql.org/message-id/11548.1297983024@sss.pgh.pa.us Ah, I remember now. That lead to this: http://archives.postgresql.org/pgsql-hackers/2011-02/msg01824.php So it looks like what Peter said about updating the catalog is what needs to be done, and is simple, yes? But then pg_dumpneeds more collation-juju. Am I right? Best, David
Tom Lane <tgl@sss.pgh.pa.us> writes: > Any other "must fix" items on people's minds? I'd like it if Magnus could commit his last round of work on pg_basebackup to stream the WALs in a subprocess. It's been about ready and waiting for more tests and code review while I've been ill. I should be able to get there by Friday, on the parts where I can help. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Tue, Mar 1, 2011 at 22:20, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Tom Lane <tgl@sss.pgh.pa.us> writes: >> Any other "must fix" items on people's minds? > > I'd like it if Magnus could commit his last round of work on > pg_basebackup to stream the WALs in a subprocess. It's been about ready > and waiting for more tests and code review while I've been ill. I > should be able to get there by Friday, on the parts where I can help. I'd love to, but there's at least one bug still in there that needs to be tracked down. And I'm doing training most of this week, so unless someone else can pitch in, it may not be done on time. And while it's nice, I don't think it falls under "*must* fix". -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
"David E. Wheeler" <david@kineticode.com> writes: > On Mar 1, 2011, at 1:12 PM, Tom Lane wrote: >> The question is what collation property the >> citext type needs to have, and how we get it to have that setting during >> an upgrade from 9.0. See >> http://archives.postgresql.org/message-id/11548.1297983024@sss.pgh.pa.us > Ah, I remember now. That lead to this: > http://archives.postgresql.org/pgsql-hackers/2011-02/msg01824.php > So it looks like what Peter said about updating the catalog is what needs to be done, and is simple, yes? But then pg_dumpneeds more collation-juju. Am I right? Yeah, the real problem in my mind is not so much citext as whether the current representation of a type's collation property is sane in the first place. Once we've thrashed that out, I'll be happy with a simple "UPDATE pg_type" kluge in the citext upgrade script. Doing anything cleaner-looking than that is a future project (and might never happen, seeing that we've never felt a need for ALTER TYPE commands for other properties of a basic type). So really I guess the release-blocker issue is http://archives.postgresql.org/message-id/27152.1299015062@sss.pgh.pa.us regards, tom lane
On Tue, Mar 1, 2011 at 3:40 PM, Robert Haas <robertmhaas@gmail.com> wrote: > Sync rep, on the other hand, is going to > do the opposite, probably by a large margin. > Are you sure of that? the code is very centralized and the bugs we found last week weren't that difficult to address, the prove for that is that Simon fixed them in one day... And for the open items, most of them are definitions about the behaviour... -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte y capacitación de PostgreSQL
On Tue, Mar 1, 2011 at 4:55 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Tue, Mar 1, 2011 at 3:40 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> Sync rep, on the other hand, is going to >> do the opposite, probably by a large margin. >> > > Are you sure of that? the code is very centralized and the bugs we > found last week weren't that difficult to address, the prove for that > is that Simon fixed them in one day... So where is the new patch, and the test reports from all the people who found problems in the last version? > And for the open items, most of them are definitions about the behaviour... Like what? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Mar 1, 2011 at 4:58 PM, Robert Haas <robertmhaas@gmail.com> wrote: > > So where is the new patch, and the test reports from all the people > who found problems in the last version? > Simon do this in his public repo here: git://github.com/simon2ndQuadrant/postgres.git It has been just one day from this so i expect he will post a patch soon, meanwhile i'm following his repo and testing and seems i'm not the only one because Yeb Havinga has added two columns in pg_stat_replication to know which standby is the synch standby and which ones are potential synch standbys -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte y capacitación de PostgreSQL
On 01/03/11 22:07, Andrew Dunstan wrote: > > > On 03/01/2011 03:53 PM, Jan Urbański wrote: >> On 01/03/11 21:35, Tom Lane wrote: >>> Josh Berkus<josh@agliodbs.com> writes: >>>> I'm ok with closing things as of the end of the 15 days, say >>>> Thursday or >>>> Friday. >>> It might be a good idea to make a list of what we have left to do before >>> we can wrap an alpha. Here are some things on my list. Not all of them >>> are necessarily release blockers, but we need to discuss which ones are: >>> >>> * Regression test failures from recent plpython patches. These are >>> affecting enough machines to make them "must fix before alpha", IMO. >>> There are some variations in error message wording, which are not too >>> terrible but also not exactly hard to fix. The python assert failure >>> that some Fedora machines are reporting is considerably more disturbing. > > I agree. > >> I'm looking into the crash, no luck so long. >> >> > > Is there anything you need that would help you? Could you try this patch and see if it fixes the failures? I'm at a loss as to why this happens, but judging from the traceback the spiexceptions module is getting unreffed somewhere and when garbage collection kicks it it barfs on an object with refcount 0. So I'm forcing an incref of the module to confirm that. I tried various tricks on 32 bit Debian, with Python 2.6, 2.7, Python compiled from Fedora's SRPM and I never saw anything wrong. Will keep on trying, but tommorrow evening, time to sleep :( Cheers, Jan
Attachment
On Tue, 2011-03-01 at 12:42 -0800, Josh Berkus wrote: > > That response is just dodging the hard question, so whatever. Tom's > > cleanup is not going to break things, or at least it's going to fix > > more than it breaks on net. Sync rep, on the other hand, is going to > > do the opposite, probably by a large margin. > > Well, we need to at least give Simon and Fujii a chance to respond > during their respective daytime hours. Friday is a reasonable deadline. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
On 03/01/2011 05:19 PM, Jan Urbański wrote: > On 01/03/11 22:07, Andrew Dunstan wrote: >> >> On 03/01/2011 03:53 PM, Jan Urbański wrote: >>> On 01/03/11 21:35, Tom Lane wrote: >>>> Josh Berkus<josh@agliodbs.com> writes: >>>>> I'm ok with closing things as of the end of the 15 days, say >>>>> Thursday or >>>>> Friday. >>>> It might be a good idea to make a list of what we have left to do before >>>> we can wrap an alpha. Here are some things on my list. Not all of them >>>> are necessarily release blockers, but we need to discuss which ones are: >>>> >>>> * Regression test failures from recent plpython patches. These are >>>> affecting enough machines to make them "must fix before alpha", IMO. >>>> There are some variations in error message wording, which are not too >>>> terrible but also not exactly hard to fix. The python assert failure >>>> that some Fedora machines are reporting is considerably more disturbing. >> I agree. >> >>> I'm looking into the crash, no luck so long. >>> >>> >> Is there anything you need that would help you? > Could you try this patch and see if it fixes the failures? > > I'm at a loss as to why this happens, but judging from the traceback the > spiexceptions module is getting unreffed somewhere and when garbage > collection kicks it it barfs on an object with refcount 0. So I'm > forcing an incref of the module to confirm that. > > I tried various tricks on 32 bit Debian, with Python 2.6, 2.7, Python > compiled from Fedora's SRPM and I never saw anything wrong. Will keep on > trying, but tommorrow evening, time to sleep :( > > Thanks. That seems to have fixed it, so I have applied the patch. Would you like to supply some comments to got with it? cheers andrew
On 02/03/11 01:05, Andrew Dunstan wrote: > > > On 03/01/2011 05:19 PM, Jan Urbański wrote: >> On 01/03/11 22:07, Andrew Dunstan wrote: >>> >>> On 03/01/2011 03:53 PM, Jan Urbański wrote: >>>> On 01/03/11 21:35, Tom Lane wrote: >>>>> Josh Berkus<josh@agliodbs.com> writes: >>>>>> I'm ok with closing things as of the end of the 15 days, say >>>>>> Thursday or >>>>>> Friday. >>>>> It might be a good idea to make a list of what we have left to do >>>>> before >>>>> we can wrap an alpha. Here are some things on my list. Not all of >>>>> them >>>>> are necessarily release blockers, but we need to discuss which ones >>>>> are: >>>>> >>>>> * Regression test failures from recent plpython patches. These are >>>>> affecting enough machines to make them "must fix before alpha", IMO. >>>>> There are some variations in error message wording, which are not too >>>>> terrible but also not exactly hard to fix. The python assert failure >>>>> that some Fedora machines are reporting is considerably more >>>>> disturbing. >>> I agree. >>> >>>> I'm looking into the crash, no luck so long. >>>> >>>> >>> Is there anything you need that would help you? >> Could you try this patch and see if it fixes the failures? >> >> I'm at a loss as to why this happens, but judging from the traceback the >> spiexceptions module is getting unreffed somewhere and when garbage >> collection kicks it it barfs on an object with refcount 0. So I'm >> forcing an incref of the module to confirm that. >> >> I tried various tricks on 32 bit Debian, with Python 2.6, 2.7, Python >> compiled from Fedora's SRPM and I never saw anything wrong. Will keep on >> trying, but tommorrow evening, time to sleep :( > > Thanks. > > That seems to have fixed it, so I have applied the patch. Would you like > to supply some comments to got with it? The comment would be something like /* XXX it appears that in some circumstantes the reference count of the spiexceptions module drops to zero causing a Python assert failure when the garbage collector visits the module. This has been observed on the buildfarm. To fix this, add an additional ref for the module here. */ I have no idea why the refcount of the module becomes zero, debug prints I added on my system were always showing 1. Jan
On Wed, Mar 2, 2011 at 6:14 AM, Jan Urbański <wulczer@wulczer.org> wrote: >> That seems to have fixed it, so I have applied the patch. Would you like >> to supply some comments to got with it? > > The comment would be something like > > /* XXX it appears that in some circumstantes the reference count of the > spiexceptions module drops to zero causing a Python assert failure when > the garbage collector visits the module. This has been observed on the > buildfarm. To fix this, add an additional ref for the module here. */ > > I have no idea why the refcount of the module becomes zero, debug prints > I added on my system were always showing 1. But does bumping the ref count then create a leak the rest of the time? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 02/03/11 14:25, Robert Haas wrote: > On Wed, Mar 2, 2011 at 6:14 AM, Jan Urbański <wulczer@wulczer.org> wrote: >>> That seems to have fixed it, so I have applied the patch. Would you like >>> to supply some comments to got with it? >> >> The comment would be something like >> >> /* XXX it appears that in some circumstantes the reference count of the >> spiexceptions module drops to zero causing a Python assert failure when >> the garbage collector visits the module. This has been observed on the >> buildfarm. To fix this, add an additional ref for the module here. */ >> >> I have no idea why the refcount of the module becomes zero, debug prints >> I added on my system were always showing 1. > > But does bumping the ref count then create a leak the rest of the time? Not really, because you never want to garbage collect the spiexceptions module (just like you don't want to GC th plpy module, or the plpy.info function etc.). So the reference count of that module should never drop to zero, but apparently on some machines it does. So just reffing artificailly is kind of a valid solution, I'm just uneasy with not knowing why it fails on some machines and does not on others. Jan
Jan Urbański <wulczer@wulczer.org> writes: > On 02/03/11 14:25, Robert Haas wrote: >> But does bumping the ref count then create a leak the rest of the time? > Not really, because you never want to garbage collect the spiexceptions > module (just like you don't want to GC th plpy module, or the plpy.info > function etc.). So the reference count of that module should never drop > to zero, but apparently on some machines it does. So just reffing > artificailly is kind of a valid solution, I'm just uneasy with not > knowing why it fails on some machines and does not on others. Yeah, that last point makes me nervous too. A look into the Fedora repository shows that the python version shipped in F13 is rather heavily patched: http://pkgs.fedoraproject.org/gitweb/?p=python.git;a=tree;h=refs/heads/f13/master;hb=refs/heads/f13/master It's not clear to me which of their changes from a stock build might be at issue, though, and even less clear whether they introduced a bug or did something to expose a bug of ours. regards, tom lane
On 02/03/11 16:28, Tom Lane wrote: > Jan Urbański <wulczer@wulczer.org> writes: >> On 02/03/11 14:25, Robert Haas wrote: >>> But does bumping the ref count then create a leak the rest of the time? > >> Not really, because you never want to garbage collect the spiexceptions >> module (just like you don't want to GC th plpy module, or the plpy.info >> function etc.). So the reference count of that module should never drop >> to zero, but apparently on some machines it does. So just reffing >> artificailly is kind of a valid solution, I'm just uneasy with not >> knowing why it fails on some machines and does not on others. > > Yeah, that last point makes me nervous too. A look into the Fedora > repository shows that the python version shipped in F13 is rather > heavily patched: > http://pkgs.fedoraproject.org/gitweb/?p=python.git;a=tree;h=refs/heads/f13/master;hb=refs/heads/f13/master > It's not clear to me which of their changes from a stock build might > be at issue, though, and even less clear whether they introduced a > bug or did something to expose a bug of ours. FWIW I looked at these patches yesterday when I was trying to reproduce the bug, but did not find anything interesting. It's mostly for stuff in the standard library. I haven't tried building Python with all of of these patches though, and did not find an easy way to rebuild a SRPM on a Debian system. I'm also wondering if it can be a 32 vs 64 bit issue?... Jan
Jan Urbański <wulczer@wulczer.org> writes:
> FWIW I looked at these patches yesterday when I was trying to reproduce
> the bug, but did not find anything interesting. It's mostly for stuff in
> the standard library. I haven't tried building Python with all of of
> these patches though, and did not find an easy way to rebuild a SRPM on
> a Debian system. I'm also wondering if it can be a 32 vs 64 bit issue?...
Well, we can eliminate that last theory, because there were both 32 and
64 bit buildfarm machines showing the crash, cf bobcat and crake.
BTW, I see the former is now running F14, not F13 as claimed on the
buildfarm dashboard, so it doesn't look to be specific to a particular
Fedora version either. I'm a bit tempted to install F15 alpha and see
if it's still there ...
regards, tom lane
On 03/02/2011 11:49 AM, Tom Lane wrote: > Well, we can eliminate that last theory, because there were both 32 and > 64 bit buildfarm machines showing the crash, cf bobcat and crake. > BTW, I see the former is now running F14, not F13 as claimed on the > buildfarm dashboard, That's because David apparently hasn't run update_personality.pl, although he has in the past. cheers andrew
On Wed, Mar 02, 2011 at 12:02:30PM -0500, Andrew Dunstan wrote: > On 03/02/2011 11:49 AM, Tom Lane wrote: > >Well, we can eliminate that last theory, because there were both 32 and > >64 bit buildfarm machines showing the crash, cf bobcat and crake. > >BTW, I see the former is now running F14, not F13 as claimed on the > >buildfarm dashboard, > > That's because David apparently hasn't run update_personality.pl, > although he has in the past. Oops! Sorry about that! Done :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Mar 2, 2011, at 9:02 AM, Andrew Dunstan wrote: > That's because David apparently hasn't run update_personality.pl, although he has in the past. Is this something we can run against crazier community members? Best, David
Excerpts from Andrew Dunstan's message of mié mar 02 14:02:30 -0300 2011: > > On 03/02/2011 11:49 AM, Tom Lane wrote: > > Well, we can eliminate that last theory, because there were both 32 and > > 64 bit buildfarm machines showing the crash, cf bobcat and crake. > > BTW, I see the former is now running F14, not F13 as claimed on the > > buildfarm dashboard, > > > That's because David apparently hasn't run update_personality.pl, > although he has in the past. Does this also explain that "moa" reports being GCC while it's actually Sun Studio? -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Thu, Mar 3, 2011 at 12:42 AM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Andrew Dunstan's message of mié mar 02 14:02:30 -0300 2011: >> >> On 03/02/2011 11:49 AM, Tom Lane wrote: >> > Well, we can eliminate that last theory, because there were both 32 and >> > 64 bit buildfarm machines showing the crash, cf bobcat and crake. >> > BTW, I see the former is now running F14, not F13 as claimed on the >> > buildfarm dashboard, >> >> >> That's because David apparently hasn't run update_personality.pl, >> although he has in the past. > > Does this also explain that "moa" reports being GCC while it's actually > Sun Studio? moa has never changed - but there was a mixup with huia's keys when they were first registered on the buildfarm. I wonder if it wasn't the keys, but the rest of the info that was actually confused. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 03/02/2011 02:12 PM, Alvaro Herrera wrote: > Excerpts from Andrew Dunstan's message of mié mar 02 14:02:30 -0300 2011: >> On 03/02/2011 11:49 AM, Tom Lane wrote: >>> Well, we can eliminate that last theory, because there were both 32 and >>> 64 bit buildfarm machines showing the crash, cf bobcat and crake. >>> BTW, I see the former is now running F14, not F13 as claimed on the >>> buildfarm dashboard, >> >> That's because David apparently hasn't run update_personality.pl, >> although he has in the past. > Does this also explain that "moa" reports being GCC while it's actually > Sun Studio? No, that's just plain wrong. update_personality lets you change the OS and compiler version, but not the name or either. If you change OS or compiler you need a new animal. In this case it just looks like it was misdescribed from the start, so I'll fix it in the database. cheers andrew
On 03/02/2011 02:16 PM, Dave Page wrote: > On Thu, Mar 3, 2011 at 12:42 AM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: >> Excerpts from Andrew Dunstan's message of mié mar 02 14:02:30 -0300 2011: >>> On 03/02/2011 11:49 AM, Tom Lane wrote: >>>> Well, we can eliminate that last theory, because there were both 32 and >>>> 64 bit buildfarm machines showing the crash, cf bobcat and crake. >>>> BTW, I see the former is now running F14, not F13 as claimed on the >>>> buildfarm dashboard, >>> >>> That's because David apparently hasn't run update_personality.pl, >>> although he has in the past. >> Does this also explain that "moa" reports being GCC while it's actually >> Sun Studio? > moa has never changed - but there was a mixup with huia's keys when > they were first registered on the buildfarm. I wonder if it wasn't the > keys, but the rest of the info that was actually confused. Oh, ugh. So if I switch the compiler names and versions on these two they will be correct? cheers andrew
On Thu, Mar 3, 2011 at 12:54 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > > > On 03/02/2011 02:16 PM, Dave Page wrote: >> >> On Thu, Mar 3, 2011 at 12:42 AM, Alvaro Herrera >> <alvherre@commandprompt.com> wrote: >>> >>> Excerpts from Andrew Dunstan's message of mié mar 02 14:02:30 -0300 2011: >>>> >>>> On 03/02/2011 11:49 AM, Tom Lane wrote: >>>>> >>>>> Well, we can eliminate that last theory, because there were both 32 and >>>>> 64 bit buildfarm machines showing the crash, cf bobcat and crake. >>>>> BTW, I see the former is now running F14, not F13 as claimed on the >>>>> buildfarm dashboard, >>>> >>>> That's because David apparently hasn't run update_personality.pl, >>>> although he has in the past. >>> >>> Does this also explain that "moa" reports being GCC while it's actually >>> Sun Studio? >> >> moa has never changed - but there was a mixup with huia's keys when >> they were first registered on the buildfarm. I wonder if it wasn't the >> keys, but the rest of the info that was actually confused. > > Oh, ugh. So if I switch the compiler names and versions on these two they > will be correct? Should be. Moa is definitely Sun Studio: -bash-3.00$ /opt/sunstudio12.1/bin/cc -V cc: Sun C 5.10 SunOS_i386 2009/06/03 usage: cc [ options] files. Use 'cc -flags' for details And Huia is GCC: -bash-3.00$ /usr/sfw/bin/gcc --version gcc (GCC) 3.4.3 (csl-sol210-3_4-20050802) Thanks. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, 2011-03-01 at 22:56 +0000, Simon Riggs wrote: > On Tue, 2011-03-01 at 12:42 -0800, Josh Berkus wrote: > > > That response is just dodging the hard question, so whatever. Tom's > > > cleanup is not going to break things, or at least it's going to fix > > > more than it breaks on net. Sync rep, on the other hand, is going to > > > do the opposite, probably by a large margin. > > > > Well, we need to at least give Simon and Fujii a chance to respond > > during their respective daytime hours. > > Friday is a reasonable deadline. I'm not going to be committing Sync Rep today. It's technically in very good shape, but some of the conceptual issues aren't clear enough on a Friday night to make it sensible for me to commit to PostgreSQL, given we must support it for 5 years if I do. I believe those things are resolvable for 9.1, probably fairly soon, but I'm not going to rush a commit just to hit a deadline. That isn't the Way of Quality that we have followed for so long. I'll let y'all discuss what to do next. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
Let's review where we are. On Tue, Mar 1, 2011 at 3:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > * Regression test failures from recent plpython patches. These are > affecting enough machines to make them "must fix before alpha", IMO. > There are some variations in error message wording, which are not too > terrible but also not exactly hard to fix. The python assert failure > that some Fedora machines are reporting is considerably more disturbing. I believe this is fixed by commit 4c966d920fb75a5d0366b887c2ef28e6d87c1eda, although it seems no one really understands why. > * Collation-related regression failure on buildfarm member pika. This > is clearly a bug we need to identify, but maybe we can ship the alpha > without a fix --- for one thing, getting more than one report of the > problem would be helpful. pika is still red, but it claims not to have updated in 13 days, so I'm not sure what that means. > * Collation-related changes still needed in contrib/citext. If we don't > have this done before alpha4, I'm afraid we'll have to generate a 1.1 > update script to fix it for alpha4 users. I'd just as soon not deal > with that overhead. I believe that this was fixed by commit 94be9e3f0ca9e7ced66168397eb586565bced9ca. > * Rearrange pg_proc and pg_operator comments as suggested here: > http://archives.postgresql.org/pgsql-docs/2010-10/msg00041.php > While this isn't a "must fix", the very end of a development cycle > is clearly the best time for such a patch, since at any other time > there are going to be a larger number of pending patches that would > have to be adjusted. So I'd kind of like to get this done, if I can > spare a day for it. I believe this was fixed by commits 94133a935414407920a47d06a6e22734c974c3b8 and 908ab80286401bb20a519fa7dc7a837631f20369. > * Generate alpha release notes. This is at least half a day's work > for somebody, I think, even with our fairly low standards for alpha > release notes. This is all kinds of not done, AFAIK. > There are other things I'd like to do, like review and perhaps commit > the btree_gist patch, but they're not at the level of "must fix". btree_gist was commited as 8436489c81c23af637696ac69cdaafddcc907ee1 So it seems like the only thing that is an absolute must-do is write some release notes. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Mar 4, 2011 at 10:44 PM, Robert Haas <robertmhaas@gmail.com> wrote: > So it seems like the only thing that is an absolute must-do is write > some release notes. Here's a rough attempt at filtering the post-alpha3 commit log down to approximately the set of things worth adding to the alpha4 release notes. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
Robert Haas <robertmhaas@gmail.com> writes:
> So it seems like the only thing that is an absolute must-do is write
> some release notes.
The buildfarm is showing that I broke MSVC builds, but other than that,
yeah.
What needs to happen for MSVC is that the rules for installing DATA
files need to be applied for the pl directories not just contrib.
I looked around but couldn't see exactly where the pl stuff gets
installed at all in those scripts. Maybe somebody who knows those
scripts better than me can fix it.
regards, tom lane
On Saturday 05 March 2011 04:44:20 Robert Haas wrote: > > * Collation-related regression failure on buildfarm member pika. This > > is clearly a bug we need to identify, but maybe we can ship the alpha > > without a fix --- for one thing, getting more than one report of the > > problem would be helpful. > > pika is still red, but it claims not to have updated in 13 days, so > I'm not sure what that means. I have just yesterday hit the same bug while testing some application on then HEAD, so its not just PIKA. Will try to find the bug or file some somewhat reproducible recipe. Andres
On Fri, Mar 4, 2011 at 11:22 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Mar 4, 2011 at 10:44 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> So it seems like the only thing that is an absolute must-do is write >> some release notes. > > Here's a rough attempt at filtering the post-alpha3 commit log down to > approximately the set of things worth adding to the alpha4 release > notes. Working on SGML-ifying this... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sat, Mar 5, 2011 at 8:48 AM, Alexander Korotkov <korotkov@intaro.ru> wrote: > On Sat, Mar 5, 2011 at 7:22 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> >> Here's a rough attempt at filtering the post-alpha3 commit log down to >> approximately the set of things worth adding to the alpha4 release >> notes. > > > Seems that support LIKE and ILIKE index searches via contrib/pg_trgm indexes > is not mentioned here. Good catch, thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sat, Mar 5, 2011 at 8:41 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Mar 4, 2011 at 11:22 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Fri, Mar 4, 2011 at 10:44 PM, Robert Haas <robertmhaas@gmail.com> wrote: >>> So it seems like the only thing that is an absolute must-do is write >>> some release notes. >> >> Here's a rough attempt at filtering the post-alpha3 commit log down to >> approximately the set of things worth adding to the alpha4 release >> notes. > > Working on SGML-ifying this... OK, first attempt committed. Proof-reading and double-checking that I haven't left things out is very welcome. I'm not entirely convinced the sections this is divided up into are the best possible choices, but neither am I sure exactly what would be better. Suggestions welcome. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 03/04/2011 10:22 PM, Robert Haas wrote: > On Fri, Mar 4, 2011 at 10:44 PM, Robert Haas<robertmhaas@gmail.com> wrote: >> So it seems like the only thing that is an absolute must-do is write >> some release notes. > > Here's a rough attempt at filtering the post-alpha3 commit log down to > approximately the set of things worth adding to the alpha4 release > notes. > > > > > Support unlogged tables. The contents of an unlogged table are WAL-logged; um.. are _not_ WAL-logged? -Andy
On Sat, Mar 5, 2011 at 9:44 AM, Andy Colson <andy@squeakycode.net> wrote: >> Support unlogged tables. The contents of an unlogged table are >> WAL-logged; > > um.. are _not_ WAL-logged? Uh, yeah. It looks like I fixed that in the version I committed, but introduced another, similar mistake which I have now also fixed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 03/05/2011 08:54 AM, Robert Haas wrote: > On Sat, Mar 5, 2011 at 9:44 AM, Andy Colson<andy@squeakycode.net> wrote: >>> Support unlogged tables. The contents of an unlogged table are >>> WAL-logged; >> >> um.. are _not_ WAL-logged? > > Uh, yeah. It looks like I fixed that in the version I committed, but > introduced another, similar mistake which I have now also fixed. > I think you have this section twice: "When an autovacuum worker..." but it is doubly cool... so... :-) -Andy
On 03/05/2011 08:54 AM, Robert Haas wrote: > On Sat, Mar 5, 2011 at 9:44 AM, Andy Colson<andy@squeakycode.net> wrote: >>> Support unlogged tables. The contents of an unlogged table are >>> WAL-logged; >> >> um.. are _not_ WAL-logged? > > Uh, yeah. It looks like I fixed that in the version I committed, but > introduced another, similar mistake which I have now also fixed. > Improved support for parallel make, make -k, and make -q Can we add a line saying "-j still doesnt work, dont use it yet" or "make -j2 works great now". I admit I've never triedto use -j before... is this telling me its ok to use now? -Andy
On lör, 2011-03-05 at 09:33 -0600, Andy Colson wrote: > Can we add a line saying "-j still doesnt work, dont use it yet" or > "make -j2 works great now". I admit I've never tried to use -j > before... is this telling me its ok to use now? Has make -j ever made any sense? Other than for locking up your system?
Andres Freund <andres@anarazel.de> writes:
> On Saturday 05 March 2011 04:44:20 Robert Haas wrote:
>>> * Collation-related regression failure on buildfarm member pika. This
>>> is clearly a bug we need to identify, but maybe we can ship the alpha
>>> without a fix --- for one thing, getting more than one report of the
>>> problem would be helpful.
> I have just yesterday hit the same bug while testing some application on then
> HEAD, so its not just PIKA.
What platform, and what locale/encoding environment?
regards, tom lane
On Sat, Mar 05, 2011 at 11:46:13AM -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On Saturday 05 March 2011 04:44:20 Robert Haas wrote:
> >>> * Collation-related regression failure on buildfarm member pika. This
> >>> is clearly a bug we need to identify, but maybe we can ship the alpha
> >>> without a fix --- for one thing, getting more than one report of the
> >>> problem would be helpful.
>
> > I have just yesterday hit the same bug while testing some application on then
> > HEAD, so its not just PIKA.
>
> What platform, and what locale/encoding environment?
Hi,
i got the same issue using git head and managed to isolate a small testcase.
git head on debian/sid x86_64, gcc45
Reproducible with:
bin/initdb -D data/ --encoding=UTF-8 --locale=en_US
bin/postgres -B16384 -D data/ -p 5555 -h localhost
create database test with encoding='UTF8'
create table ad_window (ad_client_id varchar(32) not null , name varchar(60) not null, ad_module_id varchar(32) not
null);
create UNIQUE INDEX ad_window_name on ad_window(ad_client_id, name, ad_module_id);
insert into ad_window values ('0','Tables and Columns','0');
insert into ad_window values ('0','Reference','0');
The second insert fails consistently with:
WARNING: text_cmp 1-1-<0'Tables and Columns0>-<0Reference0>
WARNING: called from varlena: <0'Tables and Columns0> <0Reference0> for collid: (null)
ERROR: locale operation to be invoked, but no collation was derived
The added elogs show the compared values on the second insert which first pointed me to the unique index as the
probabletrigger.
Backtrace of the failing text_cmp (not sure if useful):
#1 0x000000000077054b in bttextcmp (fcinfo=0x7fffbc1f9980) at varlena.c:1595
#2 0x00000000007cade4 in FunctionCall2 (flinfo=0x254e7b8, arg1=140410956296568, arg2=39118720) at fmgr.c:1387
#3 0x0000000000492330 in _bt_compare ()
#4 0x0000000000491e6a in _bt_binsrch ()
#5 0x000000000048a381 in _bt_doinsert ()
#6 0x000000000049077d in btinsert ()
#7 0x00000000007cb230 in FunctionCall6 (flinfo=0x247bc60, arg1=140411111761456, arg2=140736349578208,
arg3=140736349578176, arg4=39118460, arg5=140411111755632, arg6=1) at fmgr.c:1500
#8 0x0000000000488ef0 in index_insert (indexRelation=0x7fb402706630, values=0x7fffbc1fa3e0, isnull=0x7fffbc1fa3c0 "",
heap_t_ctid=0x254e67c, heapRelation=0x7fb402704f70, checkUnique=UNIQUE_CHECK_YES) at indexam.c:205
#9 0x00000000005cce68 in ExecInsertIndexTuples ()
#10 0x00000000005ddafc in ExecInsert ()
#11 0x00000000005de9e1 in ExecModifyTable ()
#12 0x00000000005c032a in ExecProcNode ()
#13 0x00000000005be204 in ExecutePlan ()
#14 0x00000000005bcabe in standard_ExecutorRun ()
#15 0x00000000005bc9b2 in ExecutorRun ()
#16 0x00000000006d2d9b in ProcessQuery (plan=0x2527350, sourceText=0x245ebd0 "insert into ad_window values
('0','Reference','0');",params=0x0, dest=0x2527430, completionTag=0x7fffbc1faa30 "") at pquery.c:187
Regards,
Stefan
On Saturday 05 March 2011 17:46:13 Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On Saturday 05 March 2011 04:44:20 Robert Haas wrote:
> >>> * Collation-related regression failure on buildfarm member pika. This
> >>> is clearly a bug we need to identify, but maybe we can ship the alpha
> >>> without a fix --- for one thing, getting more than one report of the
> >>> problem would be helpful.
> >
> > I have just yesterday hit the same bug while testing some application on
> > then HEAD, so its not just PIKA.
>
> What platform, and what locale/encoding environment?
One debian and one ubuntu x64 box.
I have a WIP patch fixing one of the two issues.
Several places in selfuncs.c didn't setup collations. That lead for example to
errors during patternsel.
The eqproc don't actually currently need the collation information, but I
think it is more consistent.
(attached)
I am currently looking at the other one. Its quite strange:
I can trigger it reliably on my machines.
with gdb attached:
b indexam.c:875 (thats index_getprocinfo)
...
test=# CREATE TABLE foo(a int, b text, c int);
CREATE TABLE
Time: 65.535 ms
test=# INSERT INTO foo VALUES (1, '1', 2);
INSERT 0 1
Time: 66.777 ms
test=# INSERT INTO foo VALUES (1, '2', 2);
INSERT 0 1
Time: 40.687 ms
#play with gdb
test=# CREATE INDEX foo_abc ON foo (a, b,c);
ERROR: locale operation to be invoked, but no collation was derived
...
(gdb) print irel->rd_indcollation[0]
$8 = 0
(gdb) print irel->rd_indcollation[1]
$9 = 100
(gdb) print irel->rd_indcollation[2]
$10 = 0
(gdb)
$11 = 0
(gdb) print irel->rd_index->indcollation.values[0]
$12 = 0
(gdb) print irel->rd_index->indcollation.values[1]
$13 = 0
(gdb) print irel->rd_index->indcollation.values[2]
$14 = 100
(gdb) print irel->rd_index->indcollation.values[3]
$15 = 0
(gdb) print irel->rd_index->indcollation
$16 = {vl_len_ = 8, ndim = 144, dataoffset = 1, elemtype = 0, dim1 = 26,
lbound1 = 3, values = {0}}
The piece of code using indcollation assumes that dataoffset = 0 which seems to
be an valid assumption...
... fmgr_info_collation(irel->rd_index->indcollation.values[attnum-1], locinfo);
...
Thus no valid collation is attached for the text column but a useless one for
the last int column.
Andres
On Saturday 05 March 2011 18:37:30 Andres Freund wrote: > On Saturday 05 March 2011 17:46:13 Tom Lane wrote: > > Andres Freund <andres@anarazel.de> writes: > I am currently looking at the other one. Its quite strange: The backtrace during the operations described earlier: 0 index_getprocinfo (irel=0x7f5426cbc820, attnum=1, procnum=1) at indexam.c:875 #1 0x000000000048ab14 in _bt_mkscankey_nodata (rel=0x7f5426cbc820) at nbtutils.c:123 #2 0x00000000007eb628 in tuplesort_begin_index_btree (indexRel=0x7f5426cbc820, enforceUnique=0 '\000', workMem=1048576, randomAccess=0 '\000') at tuplesort.c:753 #3 0x000000000048c94c in _bt_spoolinit (index=0x7f5426cbc820, isunique=0 '\000', isdead=0 '\000') at nbtsort.c:165 #4 0x0000000000487074 in btbuild (fcinfo=0x7fff68ab2220) at nbtree.c:116 #5 0x00000000007ca99a in OidFunctionCall3 (functionId=338, arg1=139999404896344, arg2=139999404869664, arg3=30080592) atfmgr.c:1711 #6 0x00000000004d598d in index_build (heapRelation=0x7f5426cc3058, indexRelation=0x7f5426cbc820, indexInfo=0x1cafe50, isprimary=0'\000') at index.c:1729 #7 0x00000000004d4c5c in index_create (heapRelation=0x7f5426cc3058, indexRelationName=0x1d41c90 "foo_abc", indexRelationId=626424, indexInfo=0x1cafe50, indexColNames=0x1cb0318, accessMethodObjectId=403, tableSpaceId=0, collationObjectId=0x1cb03b8, classObjectId=0x1cb03d8, coloptions=0x1cb03f8, reloptions=0, isprimary=0 '\000', isconstraint=0'\000', deferrable=0 '\000', initdeferred=0 '\000', allow_system_table_mods=0 '\000', skip_build=0 '\000',concurrent=0 '\000') at index.c:1063 #8 0x000000000057397f in DefineIndex (heapRelation=0x1d41ca8, indexRelationName=0x1d41c90 "foo_abc", indexRelationId=0, accessMethodName=0x1d41d10 "btree", tableSpaceName=0x0, attributeList=0x1d41d28, predicate=0x0, options=0x0,exclusionOpNames=0x0, unique=0 '\000', primary=0 '\000', isconstraint=0 '\000', deferrable=0 '\000', initdeferred=0'\000', is_alter_table=0 '\000', check_rights=1 '\001', skip_build=0 '\000', quiet=0 '\000', concurrent=0'\000') at indexcmds.c:395 #9 0x00000000006d3d74 in standard_ProcessUtility (parsetree=0x1d30ff0, queryString=0x1d303c0 "CREATE INDEX foo_abc ON foo(a, b,c);", params=0x0, isTopLevel=1 '\001', dest=0x1d31390, completionTag=0x7fff68ab2dc0 "") at utility.c:954 #10 0x00000000006d2db3 in ProcessUtility (parsetree=0x1d30ff0, queryString=0x1d303c0 "CREATE INDEX foo_abc ON foo (a, b,c);",params=0x0, isTopLevel=1 '\001', dest=0x1d31390, completionTag=0x7fff68ab2dc0 "") at utility.c:334 #11 0x00000000006d1e82 in PortalRunUtility (portal=0x1cade40, utilityStmt=0x1d30ff0, isTopLevel=1 '\001', dest=0x1d31390, completionTag=0x7fff68ab2dc0 "") at pquery.c:1184 #12 0x00000000006d2029 in PortalRunMulti (portal=0x1cade40, isTopLevel=1 '\001', dest=0x1d31390, altdest=0x1d31390, completionTag=0x7fff68ab2dc0"") at pquery.c:1315 #13 0x00000000006d15ff in PortalRun (portal=0x1cade40, count=9223372036854775807, isTopLevel=1 '\001', dest=0x1d31390, altdest=0x1d31390, completionTag=0x7fff68ab2dc0 "") at pquery.c:813 #14 0x00000000006cb935 in exec_simple_query (query_string=0x1d303c0 "CREATE INDEX foo_abc ON foo (a, b,c);") at postgres.c:1018 #15 0x00000000006cfa47 in PostgresMain (argc=2, argv=0x1c79da8, username=0x1c79c40 "andres") at postgres.c:3902 #16 0x000000000068444e in BackendRun (port=0x1cb1e20) at postmaster.c:3590 #17 0x0000000000683b1a in BackendStartup (port=0x1cb1e20) at postmaster.c:3275 #18 0x0000000000680e9a in ServerLoop () at postmaster.c:1449 #19 0x0000000000680677 in PostmasterMain (argc=3, argv=0x1c76ef0) at postmaster.c:1110 #20 0x00000000005fb12d in main (argc=3, argv=0x1c76ef0) at main.c:199 (gdb) Andres
Andres Freund <andres@anarazel.de> writes:
> I have a WIP patch fixing one of the two issues.
> Several places in selfuncs.c didn't setup collations. That lead for example to
> errors during patternsel.
Hmm. I have to say that this seems like quite the wrong way to go.
If everyplace in the system that could be calling a collation-sensitive
function has to be modified like this, we'll be fighting bugs of
omission till h*ll freezes over. Why aren't we just setting
finfo.fn_collation to DEFAULT_COLLATION_OID by default, or maybe better
letting places that inspect it take zero as meaning default collation?
Call sites should only need to call fmgr_info_collation() if they have
an explicit non-default collation to pass in.
regards, tom lane
On Saturday 05 March 2011 18:43:31 Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > I have a WIP patch fixing one of the two issues. > > > > Several places in selfuncs.c didn't setup collations. That lead for > > example to errors during patternsel. > > Hmm. I have to say that this seems like quite the wrong way to go. > If everyplace in the system that could be calling a collation-sensitive > function has to be modified like this, we'll be fighting bugs of > omission till h*ll freezes over. Why aren't we just setting > finfo.fn_collation to DEFAULT_COLLATION_OID by default, or maybe better > letting places that inspect it take zero as meaning default collation? > Call sites should only need to call fmgr_info_collation() if they have > an explicit non-default collation to pass in. I wondered the same. On the other hand it makes errors like the one during index build way much harder to catch... Andres
On Sat, Mar 5, 2011 at 10:17 AM, Andy Colson <andy@squeakycode.net> wrote: > On 03/05/2011 08:54 AM, Robert Haas wrote: >> >> On Sat, Mar 5, 2011 at 9:44 AM, Andy Colson<andy@squeakycode.net> wrote: >>>> >>>> Support unlogged tables. The contents of an unlogged table are >>>> WAL-logged; >>> >>> um.. are _not_ WAL-logged? >> >> Uh, yeah. It looks like I fixed that in the version I committed, but >> introduced another, similar mistake which I have now also fixed. >> > > I think you have this section twice: > > "When an autovacuum worker..." > > > but it is doubly cool... so... :-) Thanks, fixed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sat, Mar 5, 2011 at 10:33 AM, Andy Colson <andy@squeakycode.net> wrote: > On 03/05/2011 08:54 AM, Robert Haas wrote: >> >> On Sat, Mar 5, 2011 at 9:44 AM, Andy Colson<andy@squeakycode.net> wrote: >>>> >>>> Support unlogged tables. The contents of an unlogged table are >>>> WAL-logged; >>> >>> um.. are _not_ WAL-logged? >> >> Uh, yeah. It looks like I fixed that in the version I committed, but >> introduced another, similar mistake which I have now also fixed. >> > > Improved support for parallel make, make -k, and make -q > > Can we add a line saying "-j still doesnt work, dont use it yet" or "make > -j2 works great now". I admit I've never tried to use -j before... is this > telling me its ok to use now? I've been using -j3 for a long time. These changes just make it able to parallelize a bit better. As Peter says, -j without arguments is mostly just a footgun. I can't imagine why that option hasn't been removed long since. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On lör, 2011-03-05 at 12:43 -0500, Tom Lane wrote: > Why aren't we just setting finfo.fn_collation to DEFAULT_COLLATION_OID > by default, or maybe better letting places that inspect it take zero > as meaning default collation? Because then you'd just get silently wrong results instead of an error.
On Mar 4, 2011, at 7:44 PM, Robert Haas wrote: > So it seems like the only thing that is an absolute must-do is write > some release notes. What about this? > Yeah, the real problem in my mind is not so much citext as whether the > current representation of a type's collation property is sane in the > first place. Once we've thrashed that out, I'll be happy with a simple > "UPDATE pg_type" kluge in the citext upgrade script. Doing anything > cleaner-looking than that is a future project (and might never happen, > seeing that we've never felt a need for ALTER TYPE commands for other > properties of a basic type). > > So really I guess the release-blocker issue is > > http://archives.postgresql.org/message-id/27152(dot)1299015062(at)sss(dot)pgh(dot)pa(dot)us Best, David
"David E. Wheeler" <david@kineticode.com> writes: > On Mar 4, 2011, at 7:44 PM, Robert Haas wrote: >> So it seems like the only thing that is an absolute must-do is write >> some release notes. > What about this? >> http://archives.postgresql.org/message-id/27152(dot)1299015062(at)sss(dot)pgh(dot)pa(dot)us Well, nobody else seems to be excited about it. And it's not like there isn't a clear upgrade path if we decide to change CREATE TYPE later --- it'll just be a bit clunkier. regards, tom lane
Ah. Finally after trying to stare down the code for some more time the issue
is pretty simple.
index_getprocinfo did this:
/* Initialize the lookup info if first time through */if (locinfo->fn_oid == InvalidOid){
... fmgr_info_cxt(procId, locinfo, irel->rd_indexcxt);
fmgr_info_collation(irel->rd_index->indcollation.values[attnum-1], locinfo);}
which is not a good idea because irel->rd_index->indcollation is field after
the variable lenght indkey field.
Indkey is of int2vector type which is important because it means that there is
enough space for a second key without making direct access to indcollation
wrong (which is 4byte alligned). That explains why the issue could only be
triggered by a composite index covering at least 3 columns...
RelationInitIndexAccessInfo already fills the more convenient
Relation.rd_indcollation which makes the fix trivial.
It was a good bug though, because now I understand the relcache to some degree
which I formerly definitely did not ;-)
On Saturday 05 March 2011 18:37:30 Andres Freund wrote:
> test=# CREATE TABLE foo(a int, b text, c int);
> CREATE TABLE
> Time: 65.535 ms
>
> test=# INSERT INTO foo VALUES (1, '1', 2);
> INSERT 0 1
> Time: 66.777 ms
>
> test=# INSERT INTO foo VALUES (1, '2', 2);
> INSERT 0 1
> Time: 40.687 ms
> test=# CREATE INDEX foo_abc ON foo (a, b,c);
> ERROR: locale operation to be invoked, but no collation was derived
Fixed after applying the patch.
Andres
Robert,
Some minor fixes:
197 <listitem>
198 <para>
199 <emphasis>Implement a truly serializable isolation
level</emphasis>
200 </para>
201 </listitem>
Should be:
<emphasis>Implement Serializable Snapshot Isolation, in order to provide
a more robust serializable transaction mode</emphasis>
212 <emphasis>Allow multiple collations to be used within a single
213 database</emphasis>
Should be:
Allow collations to be declared at the column level, permitting multiple
collations within a single database.
I agree that the sections you have are not ideal, but I don't think it's
necessary to fix them for an alpha release.
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
On Sat, Mar 5, 2011 at 5:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "David E. Wheeler" <david@kineticode.com> writes: >> On Mar 4, 2011, at 7:44 PM, Robert Haas wrote: >>> So it seems like the only thing that is an absolute must-do is write >>> some release notes. > >> What about this? >>> http://archives.postgresql.org/message-id/27152(dot)1299015062(at)sss(dot)pgh(dot)pa(dot)us > > Well, nobody else seems to be excited about it. And it's not like there > isn't a clear upgrade path if we decide to change CREATE TYPE later --- > it'll just be a bit clunkier. I am pretty suspicious that it's not a good design, but I don't understand it well enough to propose a better one (if there is a better one). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sat, Mar 5, 2011 at 7:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:
Seems that support LIKE and ILIKE index searches via contrib/pg_trgm indexes is not mentioned here.
Here's a rough attempt at filtering the post-alpha3 commit log down toapproximately the set of things worth adding to the alpha4 releasenotes.
----
With best regards,
Alexander Korotkov.
With best regards,
Alexander Korotkov.
Andres Freund <andres@anarazel.de> writes:
> Ah. Finally after trying to stare down the code for some more time the issue
> is pretty simple.
> - fmgr_info_collation(irel->rd_index->indcollation.values[attnum-1],
> + fmgr_info_collation(irel->rd_indcollation[attnum-1],
Good catch ... but while I was poking around to make sure that there
were no other similar errors, I started to get pretty desperately
unhappy with the state of this code altogether. What exactly is
indcollation supposed to mean? Does it have any meaning for unordered
indexes? If it means something about the index's sort order, why is it
that it's not consulted while building the pathkeys that represent the
sort order? (It isn't.)
regards, tom lane
Andres Freund <andres@anarazel.de> writes:
> - fmgr_info_collation(irel->rd_index->indcollation.values[attnum-1],
> + fmgr_info_collation(irel->rd_indcollation[attnum-1],
> locinfo);
BTW, I went ahead and committed this part, since the bug and the fix are
clear. I'm still not thrilled with the plan of sprinkling the code with
random fmgr_info_collation() calls to make up for the lack of a sane
default. IMO, that *is* a default, just a badly implemented one.
regards, tom lane
On sön, 2011-03-06 at 12:16 -0500, Tom Lane wrote: > I'm still not thrilled with the plan of sprinkling the code with > random fmgr_info_collation() calls to make up for the lack of a sane > default. IMO, that *is* a default, just a badly implemented one. We have touched upon this point several times during the development of this patch. The main problem is that you need to distinguish no collation from the default collation, so they can't both be OID zero. Another problem is that if you assume OID zero means default, finding bugs in the expression tree analysis would be really hard. It would be like saying, oh, we have type OID zero, let's make it text and proceed.
Peter Eisentraut <peter_e@gmx.net> writes:
> On sön, 2011-03-06 at 12:16 -0500, Tom Lane wrote:
>> I'm still not thrilled with the plan of sprinkling the code with
>> random fmgr_info_collation() calls to make up for the lack of a sane
>> default. IMO, that *is* a default, just a badly implemented one.
> We have touched upon this point several times during the development of
> this patch. The main problem is that you need to distinguish no
> collation from the default collation, so they can't both be OID zero.
Fair enough, but throwing in fmgr_info_collation(DEFAULT_COLLATION)
anytime we have a problem seems to me to introduce the exact same issue.
Who's to say that that's really the appropriate value to use?
regards, tom lane
Hi,
On Monday, March 07, 2011 06:40:55 PM Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On sön, 2011-03-06 at 12:16 -0500, Tom Lane wrote:
> >> I'm still not thrilled with the plan of sprinkling the code with
> >> random fmgr_info_collation() calls to make up for the lack of a sane
> >> default. IMO, that *is* a default, just a badly implemented one.
> >
> > We have touched upon this point several times during the development of
> > this patch. The main problem is that you need to distinguish no
> > collation from the default collation, so they can't both be OID zero.
>
> Fair enough, but throwing in fmgr_info_collation(DEFAULT_COLLATION)
> anytime we have a problem seems to me to introduce the exact same issue.
Its comparatively easier to grep for when you notice something itchy.
> Who's to say that that's really the appropriate value to use?
I actually am quite doubtfull that thats the correct value for patternsel and
the other places I added it in the patch. I think that should that be C. On
the other hand its not likely to be that influential.
that looks like it should be corrected btw:
src/backend/tsearch/{wparser_def.c,ts_locale.c}
Oid collation = DEFAULT_COLLATION_OID; /*TODO*/
Thats occuring 6 times in there...
Andres
On Mon, Mar 7, 2011 at 1:42 PM, Andres Freund <andres@anarazel.de> wrote:
> that looks like it should be corrected btw:
> src/backend/tsearch/{wparser_def.c,ts_locale.c}
> Oid collation = DEFAULT_COLLATION_OID; /*TODO*/
>
> Thats occuring 6 times in there...
At the risk of hijacking this thread to talk about the subject of this
thread, when are we going to cut alpha4?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 3/7/11 10:59 AM, Robert Haas wrote:
> On Mon, Mar 7, 2011 at 1:42 PM, Andres Freund <andres@anarazel.de> wrote:
>> that looks like it should be corrected btw:
>> src/backend/tsearch/{wparser_def.c,ts_locale.c}
>> Oid collation = DEFAULT_COLLATION_OID; /*TODO*/
>>
>> Thats occuring 6 times in there...
>
> At the risk of hijacking this thread to talk about the subject of this
> thread, when are we going to cut alpha4?
Any reason not to release it this week, like Thursday?
Peter?
-- -- Josh Berkus PostgreSQL Experts Inc.
http://www.pgexperts.com
Robert Haas <robertmhaas@gmail.com> writes:
> At the risk of hijacking this thread to talk about the subject of this
> thread, when are we going to cut alpha4?
Well, a prerequisite for cutting an alpha is closing the commitfest,
which at this point reduces to deciding what we are going to do with
the plpython traceback patch.
Other than that, I think we're pretty close.
regards, tom lane
On Mon, Mar 7, 2011 at 2:43 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> At the risk of hijacking this thread to talk about the subject of this >> thread, when are we going to cut alpha4? > > Well, a prerequisite for cutting an alpha is closing the commitfest, > which at this point reduces to deciding what we are going to do with > the plpython traceback patch. > > Other than that, I think we're pretty close. AFAIK, there's nothing particularly special about that patch, other than that the author chose to move it back from Returned with Feedback to some other status. I think we should just pick a time to wrap some time in the next day or two, and it either makes it or it doesn't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> wrote: > Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Well, a prerequisite for cutting an alpha is closing the >> commitfest, which at this point reduces to deciding what we are >> going to do with the plpython traceback patch. > AFAIK, there's nothing particularly special about that patch, > other than that the author chose to move it back from Returned > with Feedback to some other status. I don't see it having been in Returned with Feedback. The reviewer moved it to Ready for Committer, committers raised issues and moved it to Waiting for Author, and the author submitted a new patch and moved it back to Ready for Committer. -Kevin
On Mon, Mar 7, 2011 at 3:15 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > Robert Haas <robertmhaas@gmail.com> wrote: >> Tom Lane <tgl@sss.pgh.pa.us> wrote: > >>> Well, a prerequisite for cutting an alpha is closing the >>> commitfest, which at this point reduces to deciding what we are >>> going to do with the plpython traceback patch. > >> AFAIK, there's nothing particularly special about that patch, >> other than that the author chose to move it back from Returned >> with Feedback to some other status. > > I don't see it having been in Returned with Feedback. The reviewer > moved it to Ready for Committer, committers raised issues and moved > it to Waiting for Author, and the author submitted a new patch and > moved it back to Ready for Committer. Sorry, you're right. Still, as happy as I am that we've made so much progress with PL/python (and other things) this CommitFest, I think it's time to pick a date and time to ship alpha4 and call this one good. If the patch makes it, great; if not, oh well. We can't keep letting this drag out. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> wrote: > Still, as happy as I am that we've made so much progress with > PL/python (and other things) this CommitFest, I think it's time to > pick a date and time to ship alpha4 and call this one good. If > the patch makes it, great; if not, oh well. We can't keep letting > this drag out. Oh, I agree, all around. I just didn't want there to be a misunderstanding. -Kevin
On Mon, Mar 7, 2011 at 3:28 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > Robert Haas <robertmhaas@gmail.com> wrote: > >> Still, as happy as I am that we've made so much progress with >> PL/python (and other things) this CommitFest, I think it's time to >> pick a date and time to ship alpha4 and call this one good. If >> the patch makes it, great; if not, oh well. We can't keep letting >> this drag out. > > Oh, I agree, all around. I just didn't want there to be a > misunderstanding. Agreed, and I appreciate the correction. I mis-remembered the history of that patch; yeah for the audit log. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On mån, 2011-03-07 at 15:19 -0500, Robert Haas wrote: > Sorry, you're right. Still, as happy as I am that we've made so much > progress with PL/python (and other things) this CommitFest, I think > it's time to pick a date and time to ship alpha4 and call this one > good. If the patch makes it, great; if not, oh well. We can't keep > letting this drag out. Go for it. We might add the traceback patch afterwards, but it's not ready at the moment.
On Mon, Mar 7, 2011 at 4:59 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On mån, 2011-03-07 at 15:19 -0500, Robert Haas wrote: >> Sorry, you're right. Still, as happy as I am that we've made so much >> progress with PL/python (and other things) this CommitFest, I think >> it's time to pick a date and time to ship alpha4 and call this one >> good. If the patch makes it, great; if not, oh well. We can't keep >> letting this drag out. > > Go for it. > > We might add the traceback patch afterwards, but it's not ready at the > moment. OK, then I propose we tag and cut the tarball on Wednesday morning, say around 9am Eastern. Objections? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, 2011-03-07 at 11:00 -0800, Josh Berkus wrote: > > > > At the risk of hijacking this thread to talk about the subject of > this > > thread, when are we going to cut alpha4? > > Any reason not to release it this week, like Thursday? Let's release it before PGEast, please -- I will be there, and won't be able to spend time on packaging. -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz
On mån, 2011-03-07 at 12:40 -0500, Tom Lane wrote: > Fair enough, but throwing in fmgr_info_collation(DEFAULT_COLLATION) > anytime we have a problem seems to me to introduce the exact same > issue. Who's to say that that's really the appropriate value to use? It normally isn't the appropriate value to use. It's only appropriate if either that particular part of the code doesn't support collations yet or it's dealing with some hardcoded catalog lookups or some similar case. In most other cases you should be getting the collation passed in from somewhere, and if you aren't there is probably some work to do to get it there. That's at least sort of the experience from developing this.
Peter Eisentraut <peter_e@gmx.net> writes:
> On mån, 2011-03-07 at 12:40 -0500, Tom Lane wrote:
>> Fair enough, but throwing in fmgr_info_collation(DEFAULT_COLLATION)
>> anytime we have a problem seems to me to introduce the exact same
>> issue. Who's to say that that's really the appropriate value to use?
> It normally isn't the appropriate value to use. It's only appropriate
> if either that particular part of the code doesn't support collations
> yet or it's dealing with some hardcoded catalog lookups or some similar
> case. In most other cases you should be getting the collation passed in
> from somewhere, and if you aren't there is probably some work to do to
> get it there. That's at least sort of the experience from developing
> this.
Mph. Well, I guess in the case of the pg_statistic stats we can declare
by fiat that we calculate the stats according to the default collation.
They'll be a bit off when used for a query that is comparing according
to some other collation, but it's probably no worse than any other
approximation we make in the selectivity code.
regards, tom lane
On tis, 2011-03-08 at 17:43 -0500, Tom Lane wrote: > Mph. Well, I guess in the case of the pg_statistic stats we can > declare by fiat that we calculate the stats according to the default > collation. They'll be a bit off when used for a query that is > comparing according to some other collation, but it's probably no > worse than any other approximation we make in the selectivity code. That was my assumption for the time being. Unfortunately, it's hard to create test coverage for these types of issues.
Andres Freund <andres@anarazel.de> writes:
> On Saturday 05 March 2011 17:46:13 Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>>> * Collation-related regression failure on buildfarm member pika. This
>>>> is clearly a bug we need to identify, but maybe we can ship the alpha
>>>> without a fix --- for one thing, getting more than one report of the
>>>> problem would be helpful.
>>> I have just yesterday hit the same bug while testing some application on
>>> then HEAD, so its not just PIKA.
>> What platform, and what locale/encoding environment?
> One debian and one ubuntu x64 box.
> I have a WIP patch fixing one of the two issues.
> Several places in selfuncs.c didn't setup collations. That lead for example to
> errors during patternsel.
I've applied these changes as part of my last commit. However, I now
believe that this has nothing to do with pika's problem and we're not
going to see it go green :-(
I'm pretty well convinced that what is happening on pika is that the
first two rows of histogram_bounds data processed by the max() aggregate
happen to be of collatable types, but since the aggregate is over
anyarray it has no collation to pass to the array_larger() function,
so it fails with that error instead of the expected one. I don't know
exactly why we see that fail consistently on that machine and not any
other buildfarm machines, but it doesn't matter --- the outcome is
clearly possible.
We could add a variant expected-output file for the polymorphism test,
but I don't really want to take on the added maintenance burden.
I think it'd be best to just add a WHERE clause to the failing query
that will restrict the set of rows considered so that they don't include
any columns of collatable types. If pika's next run doesn't show green
then I'll do that.
regards, tom lane
Excerpts from Dave Page's message of mié mar 02 16:38:00 -0300 2011: > Should be. Moa is definitely Sun Studio: > > -bash-3.00$ /opt/sunstudio12.1/bin/cc -V > cc: Sun C 5.10 SunOS_i386 2009/06/03 > usage: cc [ options] files. Use 'cc -flags' for details > > And Huia is GCC: > > -bash-3.00$ /usr/sfw/bin/gcc --version > gcc (GCC) 3.4.3 (csl-sol210-3_4-20050802) BTW I just swapped the compiler details for those two animals in the buildfarm database. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Wed, Apr 27, 2011 at 8:12 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Dave Page's message of mié mar 02 16:38:00 -0300 2011: > >> Should be. Moa is definitely Sun Studio: >> >> -bash-3.00$ /opt/sunstudio12.1/bin/cc -V >> cc: Sun C 5.10 SunOS_i386 2009/06/03 >> usage: cc [ options] files. Use 'cc -flags' for details >> >> And Huia is GCC: >> >> -bash-3.00$ /usr/sfw/bin/gcc --version >> gcc (GCC) 3.4.3 (csl-sol210-3_4-20050802) > > BTW I just swapped the compiler details for those two animals in the > buildfarm database. I thought Andrew did that already? -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 04/27/2011 03:15 PM, Dave Page wrote: > On Wed, Apr 27, 2011 at 8:12 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: >> Excerpts from Dave Page's message of mié mar 02 16:38:00 -0300 2011: >> >>> Should be. Moa is definitely Sun Studio: >>> >>> -bash-3.00$ /opt/sunstudio12.1/bin/cc -V >>> cc: Sun C 5.10 SunOS_i386 2009/06/03 >>> usage: cc [ options] files. Use 'cc -flags' for details >>> >>> And Huia is GCC: >>> >>> -bash-3.00$ /usr/sfw/bin/gcc --version >>> gcc (GCC) 3.4.3 (csl-sol210-3_4-20050802) >> BTW I just swapped the compiler details for those two animals in the >> buildfarm database. > I thought Andrew did that already? > I think I forgot. Thanks Alvaro. cheers andrew
Excerpts from Dave Page's message of mié abr 27 16:15:33 -0300 2011: > On Wed, Apr 27, 2011 at 8:12 PM, Alvaro Herrera > <alvherre@commandprompt.com> wrote: > > BTW I just swapped the compiler details for those two animals in the > > buildfarm database. > > I thought Andrew did that already? He hadn't gotten around to actually doing it, apparently. Moa was still listed as GCC. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support