Re: Beta2 Wrap Up ...

> > > 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

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