Re: It's June 1; do you know where your release is?

As of today we are three months behind the original plan for 8.4.0 release.
In a one-year release cycle that's already pretty bad slip; but there now
seems no chance of a release happening in less than a month, and if we
continue to let things drift it could easily stretch to five or six
months' slip. Given the slow pace of bug reports there is no reason to
be waiting. We need to refocus our energy on getting the release out.

The main thing that needs to happen now is to deal with the open items
listed at
http://wiki.postgresql.org/wiki/PostgreSQL_8.4_Open_Items
either by fixing them or by agreeing that it's okay to let them slide
to 8.5 or beyond.

regards, tom lane

Tom,

Let me start this out by voting the things I think we can drop until 8.5:

* gettext plurals patch needs some polishing
-- revert patch, save for 8.5

# adjust information_schema precision and scale fields?
-- save for 8.5

# instrument the Windows shared memory reattachment problem?
-- as much as I'd like to do this, the solution could be as bad as the
problem; we'd need more testing time for new instrumentation. Will have
to deal with via testing patched versions.

# tweak the ordering heuristics for parallel pg_restore?
-- beta2 version is "good enough"; further improvements should be saved
for 8.5.

# change or at least redocument from_collapse_limit?
-- should be doc patch only. Robert Haas should write it.

# revisit increase of default_statistics_target?
-- No. Still appears to be artifact of DBT2. Can't reproduce the issue
using pgbench, or any other test. Still investigating.

Other questions:

# cost_nestloop and cost_hashjoin are broken for SEMI and ANTI joins

* tgl says: I think this is mostly dealt with but it could use
performance testing.

Mark? Jignesh? Can you test this?

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

On Mon, 2009-06-01 at 11:09 -0700, Josh Berkus wrote:
> Tom,

> # adjust information_schema precision and scale fields?
> -- save for 8.5

I read this thread. It seems for the changes we can make we should just
make them. The actual amount of change is actually very nominal.

Joshua D. Drake

--
PostgreSQL - XMPP: jdrake(at)jabber(dot)postgresql(dot)org
Consulting, Development, Support, Training
503-667-4564 - http://www.commandprompt.com/
The PostgreSQL Company, serving since 1997

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> # cost_nestloop and cost_hashjoin are broken for SEMI and ANTI joins
> * tgl says: I think this is mostly dealt with but it could use performance testing.

> Mark? Jignesh? Can you test this?

Actually I'm hoping that Kevin Grittner will have something to report
on that one soon --- he's our best test case for complicated EXISTS
queries. I doubt that the standard benchmarks exercise this code
meaningfully at all.

regards, tom lane

On Mon, Jun 1, 2009 at 2:09 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Tom,
>
> Let me start this out by voting the things I think we can drop until 8.5:
>
> * gettext plurals patch needs some polishing
> -- revert patch, save for 8.5
>
> #  adjust information_schema precision and scale fields?
> -- save for 8.5
>
> # instrument the Windows shared memory reattachment problem?
> -- as much as I'd like to do this, the solution could be as bad as the
> problem; we'd need more testing time for new instrumentation.  Will have to
> deal with via testing patched versions.
>
> # tweak the ordering heuristics for parallel pg_restore?
> -- beta2 version is "good enough"; further improvements should be saved for
> 8.5.
>
> # change or at least redocument from_collapse_limit?
> -- should be doc patch only.  Robert Haas should write it.
>
> # revisit increase of default_statistics_target?
> -- No.  Still appears to be artifact of DBT2.  Can't reproduce the issue
> using pgbench, or any other test.  Still investigating.
>
>
> Other questions:
>
> #  cost_nestloop and cost_hashjoin are broken for SEMI and ANTI joins
>
>    * tgl says: I think this is mostly dealt with but it could use
> performance testing.
>
> Mark?  Jignesh?   Can you test this?

+1 to all of these. Will send extremely short doc patch tonight. I
recommend we create
http://wiki.postgresql.org/wiki/PostgreSQL_8.5_Open_Items and start
bumping things that can't be completed for 8.4.

Basically, my opinion is that if it's not a regression, it's too late
to work on it now. We should ship the release when we're confident
that the new features have had adequate testing and we've squashed all
the regressions.

...Robert

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> Let me start this out by voting the things I think we can drop until 8.5:

> * gettext plurals patch needs some polishing
> -- revert patch, save for 8.5

Peter might think differently about that ;-). My problem with it is
that we don't seem to have a polished API for how code uses the feature.
I wouldn't mind so much except that once we release we are going to be
stuck with the API indefinitely --- the usage will propagate into
third-party code very quickly and we won't want to break that code by
changing it.

Personally I'd prefer to fix it rather than revert it.

> # adjust information_schema precision and scale fields?
> -- save for 8.5

No objection here. I mainly wanted to make sure the issue doesn't get
forgotten.

> # instrument the Windows shared memory reattachment problem?
> -- as much as I'd like to do this, the solution could be as bad as the
> problem; we'd need more testing time for new instrumentation. Will have
> to deal with via testing patched versions.

Yeah. That was put on the list a month ago, when there was still plenty
of time to do something about it; but since we missed getting it into
beta2 I think it will have to wait.

> # tweak the ordering heuristics for parallel pg_restore?
> -- beta2 version is "good enough"; further improvements should be saved
> for 8.5.

OK, particularly if no one is in a position to test it right away.

regards, tom lane

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I recommend we create
> http://wiki.postgresql.org/wiki/PostgreSQL_8.5_Open_Items and start
> bumping things that can't be completed for 8.4.

Why not just the regular TODO list?

Stuff that has plausible patches attached can go directly to the
CommitFest_2009-First page, but otherwise I don't see a need for
special treatment. If we kick something off the 8.4 open items
list we're essentially saying it doesn't deserve special treatment.

regards, tom lane

On Mon, Jun 1, 2009 at 3:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I recommend we create
>> http://wiki.postgresql.org/wiki/PostgreSQL_8.5_Open_Items and start
>> bumping things that can't be completed for 8.4.
>
> Why not just the regular TODO list?
>
> Stuff that has plausible patches attached can go directly to the
> CommitFest_2009-First page, but otherwise I don't see a need for
> special treatment.  If we kick something off the 8.4 open items
> list we're essentially saying it doesn't deserve special treatment.

That would be fine too...

...Robert

Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> # instrument the Windows shared memory reattachment problem?
>> -- as much as I'd like to do this, the solution could be as bad as the
>> problem; we'd need more testing time for new instrumentation. Will have
>> to deal with via testing patched versions.
>
> Yeah. That was put on the list a month ago, when there was still plenty
> of time to do something about it; but since we missed getting it into
> beta2 I think it will have to wait.

Agreed. I did some mucking around with it, but the parts I found
reasonably easy to do were also reasonably useless :(

//Magnus

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> Tom Lane wrote:
>>> # instrument the Windows shared memory reattachment problem?
>>
>> Yeah. That was put on the list a month ago, when there was still plenty
>> of time to do something about it; but since we missed getting it into
>> beta2 I think it will have to wait.

> Agreed. I did some mucking around with it, but the parts I found
> reasonably easy to do were also reasonably useless :(

OK, pushed that item to TODO.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> cost_nestloop and cost_hashjoin are broken for SEMI and ANTI joins

> Actually I'm hoping that Kevin Grittner will have something to
> report on that one soon

So far, I haven't found any performance regressions in the beta2
release compared to the snapshot from March 2nd on our challenging
queries. They both perform as well or better than 8.3 for our
queries, although there is a slight increase in planning time to get
to the better plans.

Since there are new plans for most of these, I have had to actually
run them to confirm that there is no regression, and that takes some
time. I'm still chipping away, but I would say that it looks good to
me; unless someone else has a query that's behaving badly, I wouldn't
hold it up for this.

I'll post right away if a subsequent test shows a problem.

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>>> cost_nestloop and cost_hashjoin are broken for SEMI and ANTI joins

>> Actually I'm hoping that Kevin Grittner will have something to
>> report on that one soon

> So far, I haven't found any performance regressions in the beta2
> release compared to the snapshot from March 2nd on our challenging
> queries. They both perform as well or better than 8.3 for our
> queries, although there is a slight increase in planning time to get
> to the better plans.

Thanks; I'll remove this from the Open Items list for the moment.

regards, tom lane

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> On Mon, 2009-06-01 at 11:09 -0700, Josh Berkus wrote:
>> # adjust information_schema precision and scale fields?
>> -- save for 8.5

> I read this thread. It seems for the changes we can make we should just
> make them. The actual amount of change is actually very nominal.

I think the major argument against "just change it" is that we do not
wish to force an initdb now for beta testers, but if we don't there
is always going to be this niggling doubt about how an alleged "8.4"
installation will actually behave. Although I previously suggested
we could live with that, on reflection I don't think that the problem
is important enough to justify it. The information_schema has had
this issue since day one, and we hadn't gotten complaints before.
So pushing it to 8.5 seems the best decision to me.

regards, tom lane

On Mon, Jun 1, 2009 at 12:32 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> As of today we are three months behind the original plan for 8.4.0 release.
> In a one-year release cycle that's already pretty bad slip; but there now
> seems no chance of a release happening in less than a month, and if we
> continue to let things drift it could easily stretch to five or six
> months' slip.  Given the slow pace of bug reports there is no reason to
> be waiting.  We need to refocus our energy on getting the release out.
>
> The main thing that needs to happen now is to deal with the open items
> listed at
> http://wiki.postgresql.org/wiki/PostgreSQL_8.4_Open_Items
> either by fixing them or by agreeing that it's okay to let them slide
> to 8.5 or beyond.

Regarding this item:

* Consider reverting preventing regular users from base type creation

You raise this point:

tgl says: whether or not we think PL/Java is bulletproof, there are
other problems, for instance this one
http://archives.postgresql.org/message-id/87zlnwnvjg.fsf@news-spur.riddles.org.uk

That's a pretty overwhelming argument for leaving it as-is. I think
we should remove this from the list of open items.

...Robert

On Mon, 1 Jun 2009, Robert Haas wrote:

> Regarding this item:
>
> * Consider reverting preventing regular users from base type creation
>
> You raise this point:
>
> tgl says: whether or not we think PL/Java is bulletproof, there are
> other problems, for instance this one
> http://archives.postgresql.org/message-id/87zlnwnvjg.fsf@news-spur.riddles.org.uk
>
> That's a pretty overwhelming argument for leaving it as-is. I think
> we should remove this from the list of open items.

Yes, that makes sense to me as the original requester of this open item.
I thought it had been taken off a while ago.

Kris Jurka

Kris Jurka <books(at)ejurka(dot)com> writes:
> On Mon, 1 Jun 2009, Robert Haas wrote:
>> tgl says: whether or not we think PL/Java is bulletproof, there are
>> other problems, for instance this one
>> http://archives.postgresql.org/message-id/87zlnwnvjg.fsf@news-spur.riddles.org.uk
>>
>> That's a pretty overwhelming argument for leaving it as-is. I think
>> we should remove this from the list of open items.

> Yes, that makes sense to me as the original requester of this open item.
> I thought it had been taken off a while ago.

Removed now.

regards, tom lane

Tom, all,

More suggested dispositions:

* plperl fails with Perl 5.10 on Windows
o tgl says: no reports of this with pre-8.4 Postgres, but I
bet that's just because no one tried it
o dpage says: I'm rolling back the Windows installers to use
5.8 for now. Would appreciate help from anyone familiar with Perl
internals to try to debug this further!

-- Dunstan, Wheeler, Sabino-Mullaine, 'lil help please?

* contrib/seg and contrib/cube GiST index support have performance
issues
o proposed (incomplete) patch here

-- If it's just a performance issue, I don't think this issue should
block 8.4.0; it can be fixed in 8.4.1 if necessary, since we'll probably
want to backpatch the fix anyway.

* possible bug in cover density ranking?

-- From Teodor's response, this is maybe a doc patch and not a code
patch. Teodor? Oleg?

* localeconv encoding issues
o proposed patch here

-- Any reason not to apply patch?

* BUG #4622: xpath only work in utf-8 server encoding
o tgl says: there's a proposed patch for this, but I don't
think it fixes it

-- I think this is a doc patch. Since libxml (as I understand it) only
supports UTF, this is not something we can fix without major engineering
anyway, certainly not before release. I just think we need big warnings
in the docs in several places.

* contrib/intarray opclass definition needs updating
o tgl says: done, but there's another problem; see also bug
#4806

-- This is a serious issue which I'm not sure how we can resolve in the
next couple weeks. Simply throwing a warning is inadequate (although if
we can't fix it in time, we'll have to do that). Having the planner
refuse to use the index if '{}' is involved is problematic from a
performance standpoint. And changing GIN and GiST so they index empty
arrays seems likely to have other side effects. Ideas, anyone?

* Path separator consistency on Windows

-- This discussion does not appear to have concluded. Magnus, Dave?

* autovacuum can run rebuild_database_list unreasonably often

-- A possible quick workaround would be to put a lower limit of naptime
at 1s. This would save most people (those with 10 or less database)
from triggering rebuild_database_list too often. However, given that
it's precisely the people with 100's of databases who would want to
lower naptime to very low levels, this isn't much of a solution.
On the other-other hand, this is enough of a corner case that I think
we can put in a documentation warning and put a fix for this in the TODO
list. Unless Alvaro can get in a patch which prevents
rebuild_database_list from running more often than once per minute this
week?

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh Berkus wrote:
>
>
> * plperl fails with Perl 5.10 on Windows
> o tgl says: no reports of this with pre-8.4 Postgres, but I
> bet that's just because no one tried it
> o dpage says: I'm rolling back the Windows installers to use
> 5.8 for now. Would appreciate help from anyone familiar with Perl
> internals to try to debug this further!
>
> -- Dunstan, Wheeler, Sabino-Mullaine, 'lil help please?
>
>

I'm working on it.

cheers

andrew

On Tue, Jun 2, 2009 at 8:26 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>    * autovacuum can run rebuild_database_list unreasonably often
>
> -- A possible quick workaround would be to put a lower limit of naptime at
> 1s.  This would save most people (those with 10 or less database) from
> triggering rebuild_database_list too often.  However, given that it's
> precisely the people with 100's of databases who would want to lower naptime
> to very low levels, this isn't much of a solution.
>  On the other-other hand, this is enough of a corner case that I think we
> can put in a documentation warning and put a fix for this in the TODO list.
>  Unless Alvaro can get in a patch which prevents rebuild_database_list from
> running more often than once per minute this week?

Assuming I have parsed this correctly, +1. I don't think this
particular issue deserves special treatment compared to anything else.

...Robert

On Jun 2, 2009, at 5:26 PM, Josh Berkus wrote:

> Tom, all,
>
> More suggested dispositions:
>
> * plperl fails with Perl 5.10 on Windows
> o tgl says: no reports of this with pre-8.4 Postgres, but I
> bet that's just because no one tried it
> o dpage says: I'm rolling back the Windows installers to
> use 5.8 for now. Would appreciate help from anyone familiar with
> Perl internals to try to debug this further!
>
> -- Dunstan, Wheeler, Sabino-Mullaine, 'lil help please?

I don't think that any of use uses Perl on Windows. But do remember
that there are (at least) two Perls for Windows: ActivePerl and
Strawberry. The latter is the community's choice going forward, and I
suspect it'd be easier to use with Pg, since it's more like the
standard Unix Perl than ActivePerl is.

But someone who's familiar with Perl on Windows needs to look into
this; I don't even have a Windows box.

Best,

David

>

On Tue, 2009-06-02 at 17:26 -0700, Josh Berkus wrote:
>
> * possible bug in cover density ranking?
>
> -- From Teodor's response, this is maybe a doc patch and not a code
> patch. Teodor? Oleg?

I personally think that this is a bug, because we are assigning very
high rank when we are not sure about the positional information. This is
not a show stopper though.

-Sushant.

Andrew Dunstan wrote:
>
>
> Josh Berkus wrote:
>>
>>
>> * plperl fails with Perl 5.10 on Windows
>> o tgl says: no reports of this with pre-8.4 Postgres, but I
>> bet that's just because no one tried it
>> o dpage says: I'm rolling back the Windows installers to
>> use 5.8 for now. Would appreciate help from anyone familiar with Perl
>> internals to try to debug this further!
>>
>> -- Dunstan, Wheeler, Sabino-Mullaine, 'lil help please?
>>
>>
>
> I'm working on it.

Well, the first problem we encounter is this gem in the GNUMakefile (and
there is an equivalent in the MSVC build setup):

ifeq ($(*PORTNAME*), win32)
perl_archlibexp := $(*subst \,/,$(perl_archlibexp)*)
perl_privlibexp := $(*subst \,/,$(perl_privlibexp)*)
perl_embed_ldflags = -L$(*perl_archlibexp*)/CORE -lperl58
override CPPFLAGS += -DPLPERL_HAVE_UID_GID
/# Perl on win32 contains /* within comment all over the header file,
//# so disable this warning.
/override CFLAGS += -Wno-comment
endif

Note the hardcoded perl58 library name.

If we fix that then we get the previously reported crash. I'll try to
put some traces in to see exactly why that might be happening.

cheers

andrew

Josh Berkus wrote:
> Tom, all,
>
> * Path separator consistency on Windows
>
> -- This discussion does not appear to have concluded. Magnus, Dave?

We've lived with this for a long time, it's nothing new. And it's not
critical. Bump to 8.5.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> More suggested dispositions:

> * contrib/seg and contrib/cube GiST index support have performance issues

> -- If it's just a performance issue, I don't think this issue should
> block 8.4.0; it can be fixed in 8.4.1 if necessary, since we'll probably
> want to backpatch the fix anyway.

I might be mistaken, but I think that any fix would invalidate existing
indexes of these types; which would make it problematic to apply a fix
in a minor release. This item is actually my single biggest concern
of the Open Items list. I'd really like to see it resolved. It's not
so much that seg and cube themselves are a big deal, as that I think
people have copied that logic in custom opclasses. We need to be
shipping an example that's not buggy.

> * localeconv encoding issues
> o proposed patch here

> -- Any reason not to apply patch?

It still needs work, per my review the other day. In any case, it
being a Windows-specific patch, I'd want one of the Windows folk
to take responsibility for testing/committing it --- I'm not going to.

> * BUG #4622: xpath only work in utf-8 server encoding

> -- I think this is a doc patch. Since libxml (as I understand it) only
> supports UTF, this is not something we can fix without major engineering
> anyway, certainly not before release. I just think we need big warnings
> in the docs in several places.

This resolution is okay with me. I'm not sure how "major" the
engineering really is, but with no one stepping up to do the work,
I can't see letting this item block the release indefinitely.

> * autovacuum can run rebuild_database_list unreasonably often

> -- A possible quick workaround would be to put a lower limit of naptime
> at 1s.

I'm assuming that Alvaro can fix this in a reasonable way as soon as he
gets a little time to spend on it.

regards, tom lane

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> Josh Berkus wrote:
>> * Path separator consistency on Windows
>>
>> -- This discussion does not appear to have concluded. Magnus, Dave?

> We've lived with this for a long time, it's nothing new. And it's not
> critical. Bump to 8.5.

This is partly my fault, since I objected to the proposed refactoring
of path.c but didn't get around to offering a concrete alternative.
But I agree it's noncritical and a bit too late for 8.4.

I moved the entry to TODO, but maybe it should be on the next CommitFest
page instead? There were proposed patches submitted.

regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> Josh Berkus wrote:
>>> * Path separator consistency on Windows
>>>
>>> -- This discussion does not appear to have concluded. Magnus, Dave?
>
>> We've lived with this for a long time, it's nothing new. And it's not
>> critical. Bump to 8.5.
>
> This is partly my fault, since I objected to the proposed refactoring
> of path.c but didn't get around to offering a concrete alternative.
> But I agree it's noncritical and a bit too late for 8.4.
>
> I moved the entry to TODO, but maybe it should be on the next CommitFest
> page instead? There were proposed patches submitted.

CommitFest sounds best to me.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Andrew Dunstan wrote:
>
>
> Andrew Dunstan wrote:
>>
>>
>> Josh Berkus wrote:
>>>
>>>
>>> * plperl fails with Perl 5.10 on Windows
>>> o tgl says: no reports of this with pre-8.4 Postgres, but
>>> I bet that's just because no one tried it
>>> o dpage says: I'm rolling back the Windows installers to
>>> use 5.8 for now. Would appreciate help from anyone familiar with
>>> Perl internals to try to debug this further!
>>>
>>> -- Dunstan, Wheeler, Sabino-Mullaine, 'lil help please?
>>>
>>>
>>
>> I'm working on it.
>
> Well, the first problem we encounter is this gem in the GNUMakefile
> (and there is an equivalent in the MSVC build setup):
>
> ifeq ($(*PORTNAME*), win32)
> perl_archlibexp := $(*subst \,/,$(perl_archlibexp)*)
> perl_privlibexp := $(*subst \,/,$(perl_privlibexp)*)
> perl_embed_ldflags = -L$(*perl_archlibexp*)/CORE -lperl58
> override CPPFLAGS += -DPLPERL_HAVE_UID_GID
> /# Perl on win32 contains /* within comment all over the header file,
> //# so disable this warning.
> /override CFLAGS += -Wno-comment
> endif
>
> Note the hardcoded perl58 library name.
>
> If we fix that then we get the previously reported crash. I'll try to
> put some traces in to see exactly why that might be happening.
>
>

Update: the problem is apparently occurring during the call to
perl_parse() in plperl_init_interp().

Not sure why yet, but that might help other people to concentrate on
where the problem is.

cheers

andrew

On Wed, Jun 3, 2009 at 11:54 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> Update: the problem is apparently occurring during the call to perl_parse()
> in plperl_init_interp().
>

Not sure if you saw the previous thread on this, but here's a
backtrace that gives some more detail:
http://archives.postgresql.org/pgsql-bugs/2009-05/msg00198.php

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page wrote:
> On Wed, Jun 3, 2009 at 11:54 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>> Update: the problem is apparently occurring during the call to perl_parse()
>> in plperl_init_interp().
>>
>>
>
> Not sure if you saw the previous thread on this, but here's a
> backtrace that gives some more detail:
> http://archives.postgresql.org/pgsql-bugs/2009-05/msg00198.php
>

Well, that's not too easy to read, but anyway, I got to the bottom of
it. Turns out there's a change in the Perl embedding API that nobody
noticed. The attached patch worked for me on Windows. We'll need
something like this on other platforms I believe. I see via Google that
Debian is objecting to packages that call perl_parse without calling
PERL_SYS_INIT (or PERL_SYS_INIT3). I'm even wondering if we should
backpatch it.

Of course, that leaves the issue of the library name to be fixed, but
sufficient unto the day ...

cheers

andrew

On Thu, Jun 4, 2009 at 12:40 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> Well, that's not too easy to read, but anyway, I got to the bottom of it.
> Turns out there's a change in the Perl embedding API that nobody noticed.
> The attached patch worked for me on Windows. We'll need something like this
> on other platforms I believe. I see via Google that Debian is objecting to
> packages that call perl_parse without calling PERL_SYS_INIT (or
> PERL_SYS_INIT3). I'm even wondering if we should backpatch it.

Looks good to me too - thanks!

> Of course, that leaves the issue of the library name to be fixed, but
> sufficient unto the day ...

Yeah - I chatted briefly with Magnus about that a while back. I added
a quick hack to Mkvcbuild.pm to allow it to use either 5.10 or 5.8,
but didn't even think about the msys build.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page wrote:
> On Thu, Jun 4, 2009 at 12:40 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> Of course, that leaves the issue of the library name to be fixed, but
>> sufficient unto the day ...
>
> Yeah - I chatted briefly with Magnus about that a while back. I added
> a quick hack to Mkvcbuild.pm to allow it to use either 5.10 or 5.8,
> but didn't even think about the msys build.
>

Yeah, this is something we need to look at for 8.5. I think it's too
late in the cycle to start messing with it for 8.4.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander wrote:
> Dave Page wrote:
>
>> On Thu, Jun 4, 2009 at 12:40 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>
>>> Of course, that leaves the issue of the library name to be fixed, but
>>> sufficient unto the day ...
>>>
>> Yeah - I chatted briefly with Magnus about that a while back. I added
>> a quick hack to Mkvcbuild.pm to allow it to use either 5.10 or 5.8,
>> but didn't even think about the msys build.
>>
>>
>
> Yeah, this is something we need to look at for 8.5. I think it's too
> late in the cycle to start messing with it for 8.4.
>

I don't see why. Perl 5.10 is out there and people will want to use it.
I have a simple fix for this in GNUMakefile (see below) , and putting
something similar in the MSVC build stuff will be simple too. If we
don't want to support Perl 5.10 why was the item on the open issues list
at all (and why did I just spend hours finding a solution to the hard part)?

I have cleaned up the patch from yesterday slightly, too. I'm wondering
how far back we should backpatch all this. This API was actually
documented in Perl (just) before 7.4 was released, so in theory the API
adjustment should go all the way back. However, in practice it hasn't
been a problem until now, and we aren't supporting Windows releases
before 8.2, so I think 8.2 is probably the right spot.

cheers

andrew

Index: GNUmakefile
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plperl/GNUmakefile,v
retrieving revision 1.36
diff -u -r1.36 GNUmakefile
--- GNUmakefile 2 Oct 2008 08:11:11 -0000 1.36
+++ GNUmakefile 4 Jun 2009 12:28:31 -0000
@@ -19,7 +19,8 @@
ifeq ($(PORTNAME), win32)
perl_archlibexp := $(subst \,/,$(perl_archlibexp))
perl_privlibexp := $(subst \,/,$(perl_privlibexp))
-perl_embed_ldflags = -L$(perl_archlibexp)/CORE -lperl58
+perl_lib := $(basename $(notdir $(wildcard
$(perl_archlibexp)/CORE/perl*.lib)))
+perl_embed_ldflags = -L$(perl_archlibexp)/CORE -l$(perl_lib)
override CPPFLAGS += -DPLPERL_HAVE_UID_GID
# Perl on win32 contains /* within comment all over the header file,
# so disable this warning.

Andrew Dunstan wrote:
>
>
> Magnus Hagander wrote:
>> Dave Page wrote:
>>
>>> On Thu, Jun 4, 2009 at 12:40 AM, Andrew Dunstan <andrew(at)dunslane(dot)net>
>>> wrote:
>>>
>>>> Of course, that leaves the issue of the library name to be fixed, but
>>>> sufficient unto the day ...
>>>>
>>> Yeah - I chatted briefly with Magnus about that a while back. I added
>>> a quick hack to Mkvcbuild.pm to allow it to use either 5.10 or 5.8,
>>> but didn't even think about the msys build.
>>>
>>>
>>
>> Yeah, this is something we need to look at for 8.5. I think it's too
>> late in the cycle to start messing with it for 8.4.
>>
>
> I don't see why. Perl 5.10 is out there and people will want to use it.
> I have a simple fix for this in GNUMakefile (see below) , and putting
> something similar in the MSVC build stuff will be simple too. If we
> don't want to support Perl 5.10 why was the item on the open issues list
> at all (and why did I just spend hours finding a solution to the hard
> part)?

I mostly meant a general make-it-version-independent fix. The fix that's
in the MSVC build system now is really just a hack that only supports
5.8 and 5.10, not any other versions.

But sure, if it's that easy, go for it :-)

> I have cleaned up the patch from yesterday slightly, too. I'm wondering
> how far back we should backpatch all this. This API was actually
> documented in Perl (just) before 7.4 was released, so in theory the API
> adjustment should go all the way back. However, in practice it hasn't
> been a problem until now, and we aren't supporting Windows releases
> before 8.2, so I think 8.2 is probably the right spot.

IIRC, all the complaints we've seen about it is from people using the
binary installer (which would be the vast majority of the Windows
users). And we're not going to switch to 5.10 in any of the backbranches
there, ever. Based on that argument, backpatching doesn't need to be
done at all.

For the sake of those who do build from source, going back to 8.2 seems
reasonable. There's certainly no point in going past it - we have
stopped backpatching much more important fixes there a long time ago.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I have cleaned up the patch from yesterday slightly, too. I'm wondering
> how far back we should backpatch all this. This API was actually
> documented in Perl (just) before 7.4 was released, so in theory the API
> adjustment should go all the way back. However, in practice it hasn't
> been a problem until now, and we aren't supporting Windows releases
> before 8.2, so I think 8.2 is probably the right spot.

My inclination is "backpatch all the way". Now that we know the problem
is we are failing to adhere to Perl's published API, I bet we could find
some other problems it causes, if we poked into the right nooks and
crannies. And not just on Windows.

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> I have cleaned up the patch from yesterday slightly, too. I'm wondering
>> how far back we should backpatch all this. This API was actually
>> documented in Perl (just) before 7.4 was released, so in theory the API
>> adjustment should go all the way back. However, in practice it hasn't
>> been a problem until now, and we aren't supporting Windows releases
>> before 8.2, so I think 8.2 is probably the right spot.
>>
>
> My inclination is "backpatch all the way". Now that we know the problem
> is we are failing to adhere to Perl's published API, I bet we could find
> some other problems it causes, if we poked into the right nooks and
> crannies. And not just on Windows.
>
>

Yeah, the debian bug specifically mentions hppa. I'll go with your instinct.

cheers

andrew

Magnus Hagander wrote:
> Andrew Dunstan wrote:
>
>> Magnus Hagander wrote:
>>
>>> Dave Page wrote:
>>>
>>>
>>>> On Thu, Jun 4, 2009 at 12:40 AM, Andrew Dunstan <andrew(at)dunslane(dot)net>
>>>> wrote:
>>>>
>>>>
>>>>> Of course, that leaves the issue of the library name to be fixed, but
>>>>> sufficient unto the day ...
>>>>>
>>>>>
>>>> Yeah - I chatted briefly with Magnus about that a while back. I added
>>>> a quick hack to Mkvcbuild.pm to allow it to use either 5.10 or 5.8,
>>>> but didn't even think about the msys build.
>>>>
>>>>
>>>>
>>> Yeah, this is something we need to look at for 8.5. I think it's too
>>> late in the cycle to start messing with it for 8.4.
>>>
>>>
>> I don't see why. Perl 5.10 is out there and people will want to use it.
>> I have a simple fix for this in GNUMakefile (see below) , and putting
>> something similar in the MSVC build stuff will be simple too. If we
>> don't want to support Perl 5.10 why was the item on the open issues list
>> at all (and why did I just spend hours finding a solution to the hard
>> part)?
>>
>
> I mostly meant a general make-it-version-independent fix. The fix that's
> in the MSVC build system now is really just a hack that only supports
> 5.8 and 5.10, not any other versions.
>
> But sure, if it's that easy, go for it :-)
>

Yeah. The patch below should be far more future-proof (and past-proof,
for that matter).

Dave, any chance you can test it?

cheers

andrew

Index: src/tools/msvc/Mkvcbuild.pm
===================================================================
RCS file: /cvsroot/pgsql/src/tools/msvc/Mkvcbuild.pm,v
retrieving revision 1.39
diff -c -r1.39 Mkvcbuild.pm
*** src/tools/msvc/Mkvcbuild.pm 7 Apr 2009 19:35:57 -0000 1.39
--- src/tools/msvc/Mkvcbuild.pm 4 Jun 2009 16:19:29 -0000
***************
*** 104,116 ****
}
}
$plperl->AddReference($postgres);
! if (-e $solution->{options}->{perl} . '\lib\CORE\perl510.lib')
{
! $plperl->AddLibrary($solution->{options}->{perl} .
'\lib\CORE\perl510.lib');
! }
! else
! {
! $plperl->AddLibrary($solution->{options}->{perl} .
'\lib\CORE\perl58.lib');
}
}

--- 104,114 ----
}
}
$plperl->AddReference($postgres);
! my @perl_libs =
! glob($solution->{options}->{perl} .
'\lib\CORE\perl*.lib');
! if (@perl_libs == 1)
{
! $plperl->AddLibrary($perl_libs[0]);
}
}

One of the other open items is:

>> contrib/intarray opclass definition needs updating
>> tgl says: done, but there's another problem; see also bug #4806

The state of play on this is:

* GIN cannot find items that generate no index entries, such as empty
arrays or empty text documents.

* This is the Wrong Thing for certain operators, notably array
inclusion: "x <@ array[1]" ought to find empty-array values of x.

* Fixing this is quite nontrivial, although it seems doable (basically,
"empty" values need to be entered with some sort of placeholder key).
Behavior for, eg, arrays containing NULL elements also needs to be
thought about.

* However, it's been broken for awhile (since 8.2) and nobody complained
until recently. So it seems acceptable to just put the issue on TODO
and leave it unfixed for 8.4.

* But ... the patch I applied to fix this issue
http://archives.postgresql.org/message-id/17021.1234474178@sss.pgh.pa.us
means that users of contrib/intarray might be subject to this
inconsistency where they were not before. The contrib/intarray
operators were defined to not succeed for empty arrays, which might be
mathematically wrong but it made them consistent with the index
behavior. Of course, they were also *in*consistent with the
identically-named core array operators.

It seems we have two reasonable possibilities for 8.4: revert that patch
and keep contrib/intarray's behavior the same as it was, or leave the
patch in and document that there was a behavioral change.

Thoughts?

regards, tom lane

On Jun 4, 2009, at 3:58 PM, Tom Lane wrote:

> It seems we have two reasonable possibilities for 8.4: revert that
> patch
> and keep contrib/intarray's behavior the same as it was, or leave the
> patch in and document that there was a behavioral change.

You can't keep the patch and update intarray to behave like it did
before?

Best,

David

"David E. Wheeler" <david(at)kineticode(dot)com> writes:
> On Jun 4, 2009, at 3:58 PM, Tom Lane wrote:
>> It seems we have two reasonable possibilities for 8.4: revert that
>> patch and keep contrib/intarray's behavior the same as it was, or leave the
>> patch in and document that there was a behavioral change.

> You can't keep the patch and update intarray to behave like it did
> before?

Uh, no. The point is that intarray's operators behave(d) differently
from the core operators.

regards, tom lane

On Thu, Jun 4, 2009 at 5:23 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

> Yeah. The patch below should be far more future-proof (and past-proof, for
> that matter).
>
> Dave, any chance you can test it?

Doesn't work here, presumably because there is a PerlEz.lib in the
same directory as Perl510.lib, causing the glob to match more than 1
file.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page wrote:
> On Thu, Jun 4, 2009 at 5:23 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
>> Yeah. The patch below should be far more future-proof (and past-proof, for
>> that matter).
>>
>> Dave, any chance you can test it?
>>
>
> Doesn't work here, presumably because there is a PerlEz.lib in the
> same directory as Perl510.lib, causing the glob to match more than 1
> file.
>
>

OK. try with this please:

my @perl_libs = grep { /perl\d+.lib$/ }
glob($solution->{options}->{perl} . '\lib\CORE\perl*.lib');

cheers

andrew

On Fri, Jun 5, 2009 at 1:19 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> OK. try with this please:
>
>       my @perl_libs = grep { /perl\d+.lib$/ }
>           glob($solution->{options}->{perl} . '\lib\CORE\perl*.lib');

That looks good.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com