Thread: Re: pgsql: Move scanint8() to numutils.c
On 2/14/22 16:18, Peter Eisentraut wrote: > Move scanint8() to numutils.c > > Move scanint8() to numutils.c and rename to pg_strtoint64(). We > already have a "16" and "32" version of that, and the code inside the > functions was aligned, so this move makes all three versions > consistent. The API is also changed to no longer provide the errorOK > case. Users that need the error checking can use strtoi64(). > > Reviewed-by: John Naylor <john.naylor@enterprisedb.com> > Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com (moving to hackers) I guess shame on me for not noticing the thread, but I don't see any discussion about the potential for breakage to external projects. scanint8() is exported, and this change breaks at least two extensions I maintain. A quick scan (no pun intended ;-)) of github shows other potential breakage, including at least older (still open source) versions of pglogical. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
On Tue, Feb 15, 2022 at 10:39 AM Joe Conway <mail@joeconway.com> wrote: > (moving to hackers) > > I guess shame on me for not noticing the thread, but I don't see any > discussion about the potential for breakage to external projects. > > scanint8() is exported, and this change breaks at least two extensions I > maintain. > > A quick scan (no pun intended ;-)) of github shows other potential > breakage, including at least older (still open source) versions of > pglogical. I don't have a particularly strong view on whether the underlying change was a good idea here, but the breakage you're talking about seems pretty easy to fix, unless I'm missing something? I think it would be a bad idea to make such a change in a minor release without concern for extension breakage, but in a new major release it doesn't seem like a problem to me. -- Robert Haas EDB: http://www.enterprisedb.com
On 2/15/22 13:47, Robert Haas wrote: > On Tue, Feb 15, 2022 at 10:39 AM Joe Conway <mail@joeconway.com> wrote: >> (moving to hackers) >> >> I guess shame on me for not noticing the thread, but I don't see any >> discussion about the potential for breakage to external projects. >> >> scanint8() is exported, and this change breaks at least two extensions I >> maintain. >> >> A quick scan (no pun intended ;-)) of github shows other potential >> breakage, including at least older (still open source) versions of >> pglogical. > > I don't have a particularly strong view on whether the underlying > change was a good idea here, but the breakage you're talking about > seems pretty easy to fix, unless I'm missing something? I think it > would be a bad idea to make such a change in a minor release without > concern for extension breakage, but in a new major release it doesn't > seem like a problem to me. I'm not saying it is hard to fix, but it seems a bit cavalier to not even discuss the potential for breakage. If nothing else we should flag this as something for the release notes. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Feb 15, 2022 at 10:39 AM Joe Conway <mail@joeconway.com> wrote:
>> scanint8() is exported, and this change breaks at least two extensions I
>> maintain.
> I don't have a particularly strong view on whether the underlying
> change was a good idea here, but the breakage you're talking about
> seems pretty easy to fix, unless I'm missing something? I think it
> would be a bad idea to make such a change in a minor release without
> concern for extension breakage, but in a new major release it doesn't
> seem like a problem to me.
Yeah, this seems well within our expectations for major-release
changes.
regards, tom lane
On 15.02.22 20:00, Joe Conway wrote: > I'm not saying it is hard to fix, but it seems a bit cavalier to not > even discuss the potential for breakage. If nothing else we should flag > this as something for the release notes. I don't think we have ever systematically release-noted backend API changes. I don't know whether that would be useful, but a complete treatment would be a significant effort (speaking from experience of porting the mentioned pglogical between major releases).
On Wed, Feb 16, 2022 at 6:09 AM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote: > I don't think we have ever systematically release-noted backend API > changes. I don't know whether that would be useful, but a complete > treatment would be a significant effort (speaking from experience of > porting the mentioned pglogical between major releases). Personally, I don't think I would ever have used such a thing if it had existed, because looking through the git history seems more efficient to me. The release notes can be wrong or can fail to contain enough information to fix whatever issue I've encountered, but the offending commit always tells the real story. It sounds like Joe may feel differently which is fair enough; I can only speak to my own experience. -- Robert Haas EDB: http://www.enterprisedb.com
Hi, On Wed, Feb 16, 2022 at 08:24:31AM -0500, Robert Haas wrote: > On Wed, Feb 16, 2022 at 6:09 AM Peter Eisentraut > <peter.eisentraut@enterprisedb.com> wrote: > > I don't think we have ever systematically release-noted backend API > > changes. I don't know whether that would be useful, but a complete > > treatment would be a significant effort (speaking from experience of > > porting the mentioned pglogical between major releases). > > Personally, I don't think I would ever have used such a thing if it > had existed, because looking through the git history seems more > efficient to me. The release notes can be wrong or can fail to contain > enough information to fix whatever issue I've encountered, but the > offending commit always tells the real story. It sounds like Joe may > feel differently which is fair enough; I can only speak to my own > experience. Agreed. I have been maintaining extensions for quite some time and the commit log (and possibly the referenced discussions) always contains everything needed to fix whatever code is broken with all the important details. I also try to rebuild my extensions regularly against the current HEAD, so at the time the release notes are written they wouldn't be of any use anyway. The only reason I see to have something in the release notes would be to warn about a problematic API change, which doesn't result in hard compilation failure or something mostly immediate like that.