wrapping up this CommitFest (was Re: knngist - 0.8)

Started by Robert Haasalmost 15 years ago103 messages
#1Robert Haas
robertmhaas@gmail.com

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

#2Josh Berkus
josh@agliodbs.com
In reply to: Robert Haas (#1)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#3Robert Haas
robertmhaas@gmail.com
In reply to: Josh Berkus (#2)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#4Magnus Hagander
magnus@hagander.net
In reply to: Robert Haas (#3)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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/

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#3)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#6Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#5)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#7Robert Haas
robertmhaas@gmail.com
In reply to: Magnus Hagander (#4)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#8Josh Berkus
josh@agliodbs.com
In reply to: Robert Haas (#7)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#9Robert Haas
robertmhaas@gmail.com
In reply to: Josh Berkus (#8)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#6)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#11Josh Berkus
josh@agliodbs.com
In reply to: Tom Lane (#10)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#9)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#13Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#12)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#14Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#10)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#15Joshua D. Drake
jd@commandprompt.com
In reply to: Robert Haas (#3)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#16Robert Haas
robertmhaas@gmail.com
In reply to: Josh Berkus (#11)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josh Berkus (#11)
Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#18Josh Berkus
josh@agliodbs.com
In reply to: Robert Haas (#16)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#19Robert Haas
robertmhaas@gmail.com
In reply to: Josh Berkus (#18)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#20Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#14)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#21Josh Berkus
josh@agliodbs.com
In reply to: Robert Haas (#19)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#22Josh Berkus
josh@agliodbs.com
In reply to: Tom Lane (#17)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#23David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#17)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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 am willing to help out with citext, especially if correlation support might make it better.

David

#24Jan Urbański
wulczer@wulczer.org
In reply to: Tom Lane (#17)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#25Peter Eisentraut
peter_e@gmx.net
In reply to: David E. Wheeler (#23)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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 course am 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.

#26Andrew Dunstan
andrew@dunslane.net
In reply to: Jan Urbański (#24)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#27Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#23)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

"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 course am 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

#28David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#27)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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_dump needs more collation-juju. Am I right?

Best,

David

#29Dimitri Fontaine
dimitri@2ndQuadrant.fr
In reply to: Tom Lane (#17)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#30Magnus Hagander
magnus@hagander.net
In reply to: Dimitri Fontaine (#29)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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/

#31Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#28)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

"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_dump needs 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

#32Jaime Casanova
jaime@2ndquadrant.com
In reply to: Robert Haas (#19)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#33Robert Haas
robertmhaas@gmail.com
In reply to: Jaime Casanova (#32)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#34Jaime Casanova
jaime@2ndquadrant.com
In reply to: Robert Haas (#33)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#35Jan Urbański
wulczer@wulczer.org
In reply to: Andrew Dunstan (#26)
1 attachment(s)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

Attachments:

incref-spiexceptions.patchtext/x-patch; name=incref-spiexceptions.patchDownload
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 4cc0708..a2ebd22 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -3936,6 +3936,7 @@ PLy_add_exceptions(PyObject *plpy)
 #endif
 	if (PyModule_AddObject(plpy, "spiexceptions", excmod) < 0)
 		PLy_elog(ERROR, "failed to add the spiexceptions module");
+	Py_INCREF(excmod);
 
 	PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL);
 	PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
#36Simon Riggs
simon@2ndQuadrant.com
In reply to: Josh Berkus (#21)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#37Andrew Dunstan
andrew@dunslane.net
In reply to: Jan Urbański (#35)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#38Jan Urbański
wulczer@wulczer.org
In reply to: Andrew Dunstan (#37)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#39Robert Haas
robertmhaas@gmail.com
In reply to: Jan Urbański (#38)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#40Jan Urbański
wulczer@wulczer.org
In reply to: Robert Haas (#39)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#41Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jan Urbański (#40)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= <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

#42Jan Urbański
wulczer@wulczer.org
In reply to: Tom Lane (#41)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

On 02/03/11 16:28, Tom Lane wrote:

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= <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

#43Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jan Urbański (#42)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

=?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= <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

#44Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#43)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#45David Fetter
david@fetter.org
In reply to: Andrew Dunstan (#44)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#46David E. Wheeler
david@kineticode.com
In reply to: Andrew Dunstan (#44)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#47Alvaro Herrera
alvherre@commandprompt.com
In reply to: Andrew Dunstan (#44)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#48Dave Page
dpage@pgadmin.org
In reply to: Alvaro Herrera (#47)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#49Andrew Dunstan
andrew@dunslane.net
In reply to: Alvaro Herrera (#47)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#50Andrew Dunstan
andrew@dunslane.net
In reply to: Dave Page (#48)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#51Dave Page
dpage@pgadmin.org
In reply to: Andrew Dunstan (#50)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#52Simon Riggs
simon@2ndQuadrant.com
In reply to: Simon Riggs (#36)
Re: wrapping up this CommitFest (was Re: knngist - 0.8)

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

#53Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#17)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#54Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#53)
1 attachment(s)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

Attachments:

alpha4.txttext/plain; charset=US-ASCII; name=alpha4.txtDownload
#55Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#53)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#56Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#53)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#57Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#54)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#58Alexander Korotkov
korotkov@intaro.ru
In reply to: Robert Haas (#54)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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.

----
With best regards,
Alexander Korotkov.

#59Robert Haas
robertmhaas@gmail.com
In reply to: Alexander Korotkov (#58)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#60Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#57)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#61Andy Colson
andy@squeakycode.net
In reply to: Robert Haas (#54)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#62Robert Haas
robertmhaas@gmail.com
In reply to: Andy Colson (#61)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#63Andy Colson
andy@squeakycode.net
In reply to: Robert Haas (#62)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#64Andy Colson
andy@squeakycode.net
In reply to: Robert Haas (#62)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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?

-Andy

#65Peter Eisentraut
peter_e@gmx.net
In reply to: Andy Colson (#64)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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?

#66Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#56)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#67Stefan Huehner
stefan@huehner.org
In reply to: Tom Lane (#66)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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 probable trigger.

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

#68Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#66)
1 attachment(s)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

Attachments:

0001-Attach-collation-information-to-several-FmgrInfo-str.patchtext/x-patch; charset=UTF-8; name=0001-Attach-collation-information-to-several-FmgrInfo-str.patchDownload
From 2c33f1bb9c363cbb34767ecc37c463e1162372ee Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 5 Mar 2011 18:30:04 +0100
Subject: [PATCH] Attach collation information to several FmgrInfo structs in selfuncs.c

---
 src/backend/utils/adt/selfuncs.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index f10110b..783ad7c 100644
*** a/src/backend/utils/adt/selfuncs.c
--- b/src/backend/utils/adt/selfuncs.c
*************** var_eq_const(VariableStatData *vardata,
*** 285,290 ****
--- 285,291 ----
  			FmgrInfo	eqproc;
  
  			fmgr_info(get_opcode(operator), &eqproc);
+ 			fmgr_info_collation(DEFAULT_COLLATION_OID, &eqproc);
  
  			for (i = 0; i < nvalues; i++)
  			{
*************** eqjoinsel_inner(Oid operator,
*** 2116,2121 ****
--- 2117,2123 ----
  					nmatches;
  
  		fmgr_info(get_opcode(operator), &eqproc);
+ 		fmgr_info_collation(DEFAULT_COLLATION_OID, &eqproc);
  		hasmatch1 = (bool *) palloc0(nvalues1 * sizeof(bool));
  		hasmatch2 = (bool *) palloc0(nvalues2 * sizeof(bool));
  
*************** eqjoinsel_semi(Oid operator,
*** 2338,2343 ****
--- 2340,2346 ----
  					nmatches;
  
  		fmgr_info(get_opcode(operator), &eqproc);
+ 		fmgr_info_collation(DEFAULT_COLLATION_OID, &eqproc);
  		hasmatch1 = (bool *) palloc0(nvalues1 * sizeof(bool));
  		hasmatch2 = (bool *) palloc0(nvalues2 * sizeof(bool));
  
*************** prefix_selectivity(PlannerInfo *root, Va
*** 5108,5115 ****
  								 BTGreaterEqualStrategyNumber);
  	if (cmpopr == InvalidOid)
  		elog(ERROR, "no >= operator for opfamily %u", opfamily);
- 	fmgr_info(get_opcode(cmpopr), &opproc);
  
  	prefixsel = ineq_histogram_selectivity(root, vardata, &opproc, true,
  										   prefixcon->constvalue,
  										   prefixcon->consttype);
--- 5111,5119 ----
  								 BTGreaterEqualStrategyNumber);
  	if (cmpopr == InvalidOid)
  		elog(ERROR, "no >= operator for opfamily %u", opfamily);
  
+ 	fmgr_info(get_opcode(cmpopr), &opproc);
+ 	fmgr_info_collation(DEFAULT_COLLATION_OID, &opproc);
  	prefixsel = ineq_histogram_selectivity(root, vardata, &opproc, true,
  										   prefixcon->constvalue,
  										   prefixcon->consttype);
*************** prefix_selectivity(PlannerInfo *root, Va
*** 5130,5135 ****
--- 5134,5140 ----
  	if (cmpopr == InvalidOid)
  		elog(ERROR, "no < operator for opfamily %u", opfamily);
  	fmgr_info(get_opcode(cmpopr), &opproc);
+ 	fmgr_info_collation(DEFAULT_COLLATION_OID, &opproc);
  
  	greaterstrcon = make_greater_string(prefixcon, &opproc);
  	if (greaterstrcon)
-- 
1.7.1

#69Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#68)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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) at fmgr.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

#70Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#68)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#71Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#70)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#72Robert Haas
robertmhaas@gmail.com
In reply to: Andy Colson (#63)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#73Robert Haas
robertmhaas@gmail.com
In reply to: Andy Colson (#64)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#74Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#70)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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.

#75David E. Wheeler
david@kineticode.com
In reply to: Robert Haas (#53)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#76Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#75)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

"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

#77Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#68)
1 attachment(s)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

Attachments:

0001-Fix-one-more-locale-operation-to-be-invoked-but-no-c.patchtext/x-patch; charset=UTF-8; name=0001-Fix-one-more-locale-operation-to-be-invoked-but-no-c.patchDownload
From 91cf261c2af263e0965f5b302129730a7530ff38 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 5 Mar 2011 23:45:22 +0100
Subject: [PATCH] Fix one more 'locale operation to be invoked, but no collation was derived' error

The error was caused by directly accessing Relation.rd_index.indcollation.
The latter is positioned after a variable length field and thus cannot be
accessed directly. Instead use Relation.indcollation which is provided for
that purpose.
---
 src/backend/access/index/indexam.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 6e6af18..f489400 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -872,7 +872,7 @@ index_getprocinfo(Relation irel,
 				 procnum, attnum, RelationGetRelationName(irel));
 
 		fmgr_info_cxt(procId, locinfo, irel->rd_indexcxt);
-		fmgr_info_collation(irel->rd_index->indcollation.values[attnum-1],
+		fmgr_info_collation(irel->rd_indcollation[attnum-1],
 							locinfo);
 	}
 
-- 
1.7.1

#78Josh Berkus
josh@agliodbs.com
In reply to: Andres Freund (#77)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#79Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#76)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#80Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#77)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#81Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#77)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#82Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#81)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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.

#83Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#82)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#84Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#83)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#85Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#84)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#86Josh Berkus
josh@agliodbs.com
In reply to: Robert Haas (#85)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#87Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#85)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#88Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#87)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#89Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Robert Haas (#88)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#90Robert Haas
robertmhaas@gmail.com
In reply to: Kevin Grittner (#89)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#91Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Robert Haas (#90)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#92Robert Haas
robertmhaas@gmail.com
In reply to: Kevin Grittner (#91)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#93Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#90)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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.

#94Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#93)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#95Devrim GÜNDÜZ
devrim@gunduz.org
In reply to: Josh Berkus (#86)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#96Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#83)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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.

#97Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#96)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#98Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#97)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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.

#99Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#68)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#100Alvaro Herrera
alvherre@commandprompt.com
In reply to: Dave Page (#51)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#101Dave Page
dpage@pgadmin.org
In reply to: Alvaro Herrera (#100)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#102Andrew Dunstan
andrew@dunslane.net
In reply to: Dave Page (#101)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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

#103Alvaro Herrera
alvherre@commandprompt.com
In reply to: Dave Page (#101)
Re: Alpha4 release blockers (was Re: wrapping up this CommitFest)

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