Beta2 Wrap Up ...

On Thu, 2005-15-09 at 21:09 -0300, Marc G. Fournier wrote:

Tomorrow afternoon, we are planning on packaging up Beta2 .. if anyone is
sitting on something that should get in before that happens, or has a bug
they are sitting on, please let us know ...

One change that I would like to get into beta2 is the proposed
refactoring of some of the new system info / administration functions.
Since these will require an initdb, it would be nice to get them into
the tree before beta2 (since we're requiring an initdb for beta2
anyway).

I've got the patch finished, but I'm just waiting for a consensus on the
right API for some of these functions (see the -hackers thread on the
subject). I'll check something into CVS by this evening EST...

-Neil

Neil Conway <neilc@samurai.com> writes:

One change that I would like to get into beta2 is the proposed
refactoring of some of the new system info / administration functions.

I thought we'd more or less dropped that idea based on Andreas'
responses.

regards, tom lane

On Thu, 2005-15-09 at 22:31 -0400, Tom Lane wrote:

I thought we'd more or less dropped that idea based on Andreas'
responses.

I've heard no argument against renaming pg_complete_relation_size() to
pg_total_relation_size() and changing the functions that return an
integer status code to make them return a boolean (but I'm content with
not making them return void and report errors via elog).

-Neil

On Thu, 2005-15-09 at 22:06 -0400, Neil Conway wrote:

One change that I would like to get into beta2 is the proposed
refactoring of some of the new system info / administration functions.

Ok, this is done: the changes have been committed to CVS HEAD and the
catalog version number has been bumped.

-Neil

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Neil Conway
Sent: 16 September 2005 03:48
To: Tom Lane
Cc: Marc G. Fournier; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Beta2 Wrap Up ...

On Thu, 2005-15-09 at 22:31 -0400, Tom Lane wrote:

I thought we'd more or less dropped that idea based on Andreas'
responses.

I've heard no argument against renaming pg_complete_relation_size() to
pg_total_relation_size()

Having spent days, no, weeks deciding on that name on list I do not want
to see it change this late, especially as we'll now need to go and
update pgAdmin again!

and changing the functions that return an
integer status code to make them return a boolean (but I'm
content with
not making them return void and report errors via elog).

Similarly I'm not overly keen on seeing these changed again. These
functions were discussed to death earlier in the cycle, and this is what
everyone finally agreed on.

Regards, Dave.

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Neil Conway
Sent: 16 September 2005 06:38
To: Marc G. Fournier
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Beta2 Wrap Up ...

On Thu, 2005-15-09 at 22:06 -0400, Neil Conway wrote:

One change that I would like to get into beta2 is the proposed
refactoring of some of the new system info / administration

functions.

Ok, this is done: the changes have been committed to CVS HEAD and the
catalog version number has been bumped.

Hi Neil,

I appreciate that you want to get things done, but you realise that the
timeframe between your 'I'll commit this later post' and when you
actually committed was almost precisely when those of us that argued to
get these functions included in the first place were all asleep?

Perhaps you could allow 24 hours before committing potentially
controversial changes in future?

Regards, Dave

-----Original Message-----
From: Neil Conway [mailto:neilc@samurai.com]
Sent: 16 September 2005 14:57
To: Dave Page
Cc: Marc G. Fournier; pgsql-hackers@postgresql.org
Subject: RE: [HACKERS] Beta2 Wrap Up ...

On Fri, 2005-16-09 at 08:47 +0100, Dave Page wrote:

Perhaps you could allow 24 hours before committing potentially
controversial changes in future?

My apologies for the rush -- I normally would wait longer for a
consensus. In fact, I _was_ waiting for a consensus until I
saw that the
wrap for beta2 was imminent, and therefore I couldn't afford
to wait any
longer. I talked to Bruce, and he recommended essentially what I did.

FWIW, you did have more than 24 hours to object or comment on the
proposed changes, as the original email on the subject was sent on
Tuesday:

Yes, I did see that, but then I saw Andreas' response and left it at
that as Tom did.

Oh well...

Regards, Dave

On Fri, 2005-16-09 at 08:47 +0100, Dave Page wrote:

Perhaps you could allow 24 hours before committing potentially
controversial changes in future?

My apologies for the rush -- I normally would wait longer for a
consensus. In fact, I _was_ waiting for a consensus until I saw that the
wrap for beta2 was imminent, and therefore I couldn't afford to wait any
longer. I talked to Bruce, and he recommended essentially what I did.

FWIW, you did have more than 24 hours to object or comment on the
proposed changes, as the original email on the subject was sent on
Tuesday:

http://archives.postgresql.org/pgsql-hackers/2005-09/msg00635.php

-Neil

I thought we'd more or less dropped that idea based on Andreas'
responses.

I've heard no argument against renaming

pg_complete_relation_size() to

pg_total_relation_size()

Having spent days, no, weeks deciding on that name on list I
do not want to see it change this late, especially as we'll
now need to go and update pgAdmin again!

and changing the functions that return an integer status

code to make

them return a boolean (but I'm content with not making them return
void and report errors via elog).

Similarly I'm not overly keen on seeing these changed again.
These functions were discussed to death earlier in the cycle,
and this is what everyone finally agreed on.

(Sorry I haven't spoken up on this earlier, been out of town)

I agree in not really liking the change-of-names during beta. Then
again, if it's ever going to get done, better to do it now than for 8.2.

Also, the change to pg_cancel_backend breaks backwards compatibility
with 8.0, which is a whole lot worse than breaking it with 8.1-beta1.

For example, it'll break one of my scripts. I can easily code around it,
sure, but it's still breaking backwards compatibility for what I can see
as only cosmetic reasons.

Somewhat hacked up query example of something that breaks:
SELECT sum(pg_cancel_backend(procpid)) FROM pg_stat_activity WHERE
......

Sure, can be fairly easily recoded with CASE, but... If nothing else
this needs to go in as a "backwards incompatible change" in the release
notes.

Also, please do *not* make it return void and elog(ERROR) in the future!
That will break exactly the above kind of applications in a way that
cannot be coded around. IIRC this specific scenario was discussed back
when the function was originally added.

//Magnus

Magnus Hagander wrote:

I thought we'd more or less dropped that idea based on Andreas'
responses.

I've heard no argument against renaming

pg_complete_relation_size() to

pg_total_relation_size()

Having spent days, no, weeks deciding on that name on list I
do not want to see it change this late, especially as we'll
now need to go and update pgAdmin again!

Fortunately, pgAdmin doesn't use that function, but only the basic
pg_relation_size(). Phew!

Also, the change to pg_cancel_backend breaks backwards compatibility
with 8.0, which is a whole lot worse than breaking it with 8.1-beta1.

Unfortunately, core doesn't see this as backward compatibility break,
instead it's regarded as adjustment of a new function. Anything that's
not in core isn't worth a single thought....

Also, please do *not* make it return void and elog(ERROR) in the future!
That will break exactly the above kind of applications in a way that
cannot be coded around. IIRC this specific scenario was discussed back
when the function was originally added.

Seems we got around this; call us lucky...

Regards,
Andreas

Having spent days, no, weeks deciding on that name on list I do not
want to see it change this late, especially as we'll now need to go
and update pgAdmin again!

Fortunately, pgAdmin doesn't use that function, but only the
basic pg_relation_size(). Phew!

Good for you :-)

Also, the change to pg_cancel_backend breaks backwards compatibility
with 8.0, which is a whole lot worse than breaking it with

8.1-beta1.

Unfortunately, core doesn't see this as backward compatibility break,
instead it's regarded as adjustment of a new function.
Anything that's not in core isn't worth a single thought....

This function has been in core since 8.0. The other functions were new
(to core that is, imported from external module), but this one was in
the main backend already.

//Magnus

On Sat, 2005-17-09 at 14:47 +0200, Magnus Hagander wrote:

Also, the change to pg_cancel_backend breaks backwards compatibility
with 8.0, which is a whole lot worse than breaking it with 8.1-beta1.

Yeah, I thought about that (and Bruce and I already discussed it offlist
before I committed the changes). The function was newly added in 8.0 --
if we're *ever* going to fix it, fixing it before 8.1 ships is the best
time to do so. I would also guess that (a) not many people are using the
function (b) the changes in client code should be minimal (as you point
out). So IMHO making the API change and noting it in the release notes
was probably best.

Sure, can be fairly easily recoded with CASE, but... If nothing else
this needs to go in as a "backwards incompatible change" in the release
notes.

This is already done.

-Neil

On Sun, 18 Sep 2005, Magnus Hagander wrote:

Having spent days, no, weeks deciding on that name on list I do not
want to see it change this late, especially as we'll now need to go
and update pgAdmin again!

Fortunately, pgAdmin doesn't use that function, but only the
basic pg_relation_size(). Phew!

Good for you :-)

Also, the change to pg_cancel_backend breaks backwards compatibility
with 8.0, which is a whole lot worse than breaking it with

8.1-beta1.

Unfortunately, core doesn't see this as backward compatibility break,
instead it's regarded as adjustment of a new function.
Anything that's not in core isn't worth a single thought....

This function has been in core since 8.0. The other functions were new
(to core that is, imported from external module), but this one was in
the main backend already.

Is there a reason the old/new can't be aliaseed to each other, instead of
the old just being removed?

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664

Also, the change to pg_cancel_backend breaks backwards

compatibility

with 8.0, which is a whole lot worse than breaking it with

8.1-beta1.

Unfortunately, core doesn't see this as backward

compatibility break,

instead it's regarded as adjustment of a new function.
Anything that's not in core isn't worth a single thought....

This function has been in core since 8.0. The other

functions were new

(to core that is, imported from external module), but this

one was in

the main backend already.

Is there a reason the old/new can't be aliaseed to each
other, instead of the old just being removed?

It shuold be possible to do for the changed names - not sure if we want
to.
Can't do it for pg_cancel_backend(), because only the return type has
changed and you acn't overload two functions that differ only in return
type.

//Magnus

Also, the change to pg_cancel_backend breaks backwards

compatibility

with 8.0, which is a whole lot worse than breaking it with

8.1-beta1.

Yeah, I thought about that (and Bruce and I already discussed
it offlist before I committed the changes). The function was
newly added in 8.0 -- if we're *ever* going to fix it, fixing
it before 8.1 ships is the best time to do so. I would also
guess that (a) not many people are using the function (b) the
changes in client code should be minimal (as you point out).
So IMHO making the API change and noting it in the release
notes was probably best.

Hmm. Yeah, I agree provided the "if we're ever going to". I'm just not
as convinced we have to do it - it's not that broken in the first place.

Sure, can be fairly easily recoded with CASE, but... If

nothing else

this needs to go in as a "backwards incompatible change" in the
release notes.

This is already done.

Great.

//Magnus

"Marc G. Fournier" <scrappy@postgresql.org> writes:

Is there a reason the old/new can't be aliaseed to each other, instead of
the old just being removed?

Any change like that would require another initdb. If we were going to
force another initdb, my vote would be to revert these functions to
where they were in beta1. It was a mistake to change them in such a
hurry.

regards, tom lane

Is there a reason the old/new can't be aliaseed to each

other, instead

of the old just being removed?

Any change like that would require another initdb. If we
were going to force another initdb, my vote would be to
revert these functions to where they were in beta1. It was a
mistake to change them in such a hurry.

If we want to do that, we should probably try to roll a beta3 with that
as soon as possible, so not too many people have upgraded to beta2...

//Magnus

On Mon, 19 Sep 2005, Tom Lane wrote:

"Marc G. Fournier" <scrappy@postgresql.org> writes:

Is there a reason the old/new can't be aliaseed to each other, instead of
the old just being removed?

Any change like that would require another initdb. If we were going to
force another initdb, my vote would be to revert these functions to
where they were in beta1. It was a mistake to change them in such a
hurry.

+1 on reverting them back then ... and on a quick beta3 (ie. by end of
week?)

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664

-----Original Message-----
From: "Marc G. Fournier"<scrappy@postgresql.org>
Sent: 19/09/05 16:56:23
To: "Tom Lane"<tgl@sss.pgh.pa.us>
Cc: "Marc G. Fournier"<scrappy@postgresql.org>, "Magnus Hagander"<mha@sollentuna.net>, "Andreas Pflug"<pgadmin@pse-consulting.de>, "Dave Page"<dpage@vale-housing.co.uk>, "pgsql-hackers@postgresql.org"<pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] Beta2 Wrap Up ...

+1 on reverting them back then ... and on
a quick beta3 (ie. by end of week?)

+1 from me as well.

/D

-----Unmodified Original Message-----
On Mon, 19 Sep 2005, Tom Lane wrote:

"Marc G. Fournier" <scrappy@postgresql.org> writes:

Is there a reason the old/new can't be aliaseed to each other, instead of
the old just being removed?

Any change like that would require another initdb. If we were going to
force another initdb, my vote would be to revert these functions to
where they were in beta1. It was a mistake to change them in such a
hurry.

+1 on reverting them back then ... and on a quick beta3 (ie. by end of
week?)

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664

On Mon, 2005-19-09 at 10:57 -0400, Tom Lane wrote:

Any change like that would require another initdb. If we were going to
force another initdb, my vote would be to revert these functions to
where they were in beta1.

What purpose would that serve? About the only thing purpose I can see is
to avoid the API compatibility break for pg_cancel_backend() -- do
people actually consider that a major issue?

At any rate, I don't see any reason to revert the other changes (i.e.
those other than pg_cancel_backend()).

-Neil