abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Started by Nathan Bossart3 months ago70 messages
#1Nathan Bossart
nathandbossart@gmail.com

My recent commit 688dc62, which was back-patched to v18, has made the
abi-compliance-check on buildfarm member baza unhappy:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=baza&dt=2025-10-17%2013%3A11%3A11

Specifically, I replaced two functions related to lookups/privilege checks
for the new stats stuff in v18 with RangeVarGetRelidExtended(). FWIW I did
check codesearch.debian.net and GitHub for any third-party usage of these
functions before committing, and I found none. Also, these functions are
only present in exactly one release (18.0).

My thinking was that this ABI breakage was probably fine, as I don't think
we really intended for these functions to be used elsewhere. However,
since we have a buildfarm failure, I thought it best to broadcast my
thought process. While I judged back-patching worth the risk, I could live
with reverting the change on v18 if anyone is concerned.

--
nathan

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#1)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

My thinking was that this ABI breakage was probably fine, as I don't think
we really intended for these functions to be used elsewhere. However,
since we have a buildfarm failure, I thought it best to broadcast my
thought process. While I judged back-patching worth the risk, I could live
with reverting the change on v18 if anyone is concerned.

I don't have a problem with the change you made. I do have a problem
with baza having spun up this check before we settled on a way to
manage it. AFAIK we don't have any process by which we can decide
that a reported ABI change is acceptable and then clear the failure.
There was some discussion of how to control it [1]/messages/by-id/438875.1752433368@sss.pgh.pa.us, but nothing's been
done yet.

FWIW, I favor the approach of having an in-tree, per-branch file
containing the commit hash of a commit that is the current ABI
reference for that branch. If the file doesn't exist (which it
wouldn't in master, and probably not in recently-forked branches),
skip ABI checking. I think this is superior to the discussed
alternative of depending on git tags, because files are easy to
change or remove, while tags are not. In particular, I think it'd
likely be impossible to make the ABI reference point go backwards
if we use tags. Maybe that's not a case we'd ever need, but I'm
unconvinced of that.

regards, tom lane

[1]: /messages/by-id/438875.1752433368@sss.pgh.pa.us

In reply to: Tom Lane (#2)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 1:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't have a problem with the change you made. I do have a problem
with baza having spun up this check before we settled on a way to
manage it.

+1.

AFAIK we don't have any process by which we can decide
that a reported ABI change is acceptable and then clear the failure.
There was some discussion of how to control it [1], but nothing's been
done yet.

The fact that this is now causing problems was entirely predictable
(and was in fact predicted). Having a way to suppress individual
warnings that are deemed to be invalid is 100% essential if we're
detecting ABI changes on a buildfarm animal like this.

--
Peter Geoghegan

#4Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#2)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 01:15:20PM -0400, Tom Lane wrote:

FWIW, I favor the approach of having an in-tree, per-branch file
containing the commit hash of a commit that is the current ABI
reference for that branch. If the file doesn't exist (which it
wouldn't in master, and probably not in recently-forked branches),
skip ABI checking. I think this is superior to the discussed
alternative of depending on git tags, because files are easy to
change or remove, while tags are not. In particular, I think it'd
likely be impossible to make the ABI reference point go backwards
if we use tags. Maybe that's not a case we'd ever need, but I'm
unconvinced of that.

I'm new to the topic, but IMHO the per-branch file approach is by far the
best approach. Not only is it much more flexible, but we could even use it
as a centralized list of ABI breaks for a given branch with justification
for each. I can't think of any strong advantages of keeping this stuff in
git metadata. git itself uses a file for blame.ignoreRevsFile...

--
nathan

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#4)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

On Fri, Oct 17, 2025 at 01:15:20PM -0400, Tom Lane wrote:

FWIW, I favor the approach of having an in-tree, per-branch file
containing the commit hash of a commit that is the current ABI
reference for that branch.

I'm new to the topic, but IMHO the per-branch file approach is by far the
best approach. Not only is it much more flexible, but we could even use it
as a centralized list of ABI breaks for a given branch with justification
for each. I can't think of any strong advantages of keeping this stuff in
git metadata. git itself uses a file for blame.ignoreRevsFile...

Good idea. We'd have to allow comments in the file, but that's
probably a good thing anyway.

regards, tom lane

#6Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#5)
1 attachment(s)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 02:45:12PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

On Fri, Oct 17, 2025 at 01:15:20PM -0400, Tom Lane wrote:

FWIW, I favor the approach of having an in-tree, per-branch file
containing the commit hash of a commit that is the current ABI
reference for that branch.

I'm new to the topic, but IMHO the per-branch file approach is by far the
best approach. Not only is it much more flexible, but we could even use it
as a centralized list of ABI breaks for a given branch with justification
for each. I can't think of any strong advantages of keeping this stuff in
git metadata. git itself uses a file for blame.ignoreRevsFile...

Good idea. We'd have to allow comments in the file, but that's
probably a good thing anyway.

I've attached a first try. You'll notice that I have borrowed heavily from
.git-blame-ignore-revs. Some other things that might be worthwhile:

* Add commentary about when this file is needed (i.e., after the .0).
* Add instructions for creating file on new stable branch to
RELEASE_CHANGES.
* Adjust format for readability. It is a bit comment-heavy at the moment.

Anything else? I suppose this idea is entirely dependent on the
maintainers of the abi-compliance-check code to adapt to it, so we'll need
buy-in from them, too.

--
nathan

Attachments:

v1-0001-Add-.abi-compliance-history-file.patchtext/plain; charset=us-asciiDownload
From 4564f78bb861e2862cc037a95b517c38b38d06b6 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 17 Oct 2025 14:03:50 -0500
Subject: [PATCH v1 1/1] Add .abi-compliance-history file.

---
 .abi-compliance-history | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 .abi-compliance-history

diff --git a/.abi-compliance-history b/.abi-compliance-history
new file mode 100644
index 00000000000..0148d0fcda4
--- /dev/null
+++ b/.abi-compliance-history
@@ -0,0 +1,19 @@
+# This file lists commits on the REL_18_STABLE branch that break ABI
+# compatibility in ways that have been deemed acceptable (e.g., removing an
+# extern function with no third-party uses).  The primary intent of this file
+# is to placate the ABI compliance checks on the buildfarm, but it also serves
+# as a central location to document the justification for each.
+#
+# Add new entries by adding the output of the following to the top of the file:
+#
+# $ git log --pretty=format:"%H # %cd%n# %s" $ABIBREAKGITHASH -1 --date=iso
+#
+# Be sure to include additional context in a comment below the entry.
+
+c8af5019bee5c57502db830f8005a01cba60fee0 # 2025-10-15 12:47:33 -0500
+# Fix lookups in pg_{clear,restore}_{attribute,relation}_stats().
+#
+# This commit replaced two functions related to lookups/privilege checks for
+# the new stats stuff in v18 with RangeVarGetRelidExtended().  These functions
+# were not intended for use elsewhere, exist in exactly one release (18.0), and
+# do not have any known third-party callers.
-- 
2.39.5 (Apple Git-154)

In reply to: Nathan Bossart (#6)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 3:11 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

Anything else? I suppose this idea is entirely dependent on the
maintainers of the abi-compliance-check code to adapt to it, so we'll need
buy-in from them, too.

That would require parsing the file and understanding that any
compliance failures associated with a given commit should be
suppressed. But that seems decidedly nontrivial to me. I can easily
think of (admittedly somewhat contrived) scenarios where it's
basically impossible to make this work due to transitive dependencies
across commits.

I suspect that any practical approach to solving this problem will
have to involve ignore files that look somewhat like a Valgrind
suppression file. It'll have to be based on symbol names, plus
possibly a specific ABI breakage type.

--
Peter Geoghegan

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#6)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

I've attached a first try. You'll notice that I have borrowed heavily from
.git-blame-ignore-revs. Some other things that might be worthwhile:

There would need to be an initial entry at the time the file is
created, which would presumably point to some commit shortly before
the .0 version stamp is applied (or maybe we'd choose to do it around
rc1). The mockup should include that.

I'd be slightly inclined to have just one non-comment line, which
is the active reference hash value, and all the rest be comments.
The way you have it here requires the reading code to be smart
about end-of-line comments, which is code complexity we don't need
and doesn't seem amazingly legible either. OTOH, the precedent of
.git-blame-ignore-revs may be worth following regardless of our
personal druthers.

regards, tom lane

#9Nathan Bossart
nathandbossart@gmail.com
In reply to: Peter Geoghegan (#7)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 03:20:55PM -0400, Peter Geoghegan wrote:

On Fri, Oct 17, 2025 at 3:11 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

Anything else? I suppose this idea is entirely dependent on the
maintainers of the abi-compliance-check code to adapt to it, so we'll need
buy-in from them, too.

That would require parsing the file and understanding that any
compliance failures associated with a given commit should be
suppressed. But that seems decidedly nontrivial to me. I can easily
think of (admittedly somewhat contrived) scenarios where it's
basically impossible to make this work due to transitive dependencies
across commits.

I was imagining this working more like what Tom suggested. IOW we'd use
the latest commit listed in the file (perhaps always the first one) as the
baseline. Of course, this doesn't work too well if we have a bunch of ABI
breaks between buildfarm checks. But my guess is that we could deal with
that pretty easily (e.g., make sure the buildfarm member in question runs
for every commit on the stable branch).

--
nathan

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#7)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Peter Geoghegan <pg@bowt.ie> writes:

That would require parsing the file and understanding that any
compliance failures associated with a given commit should be
suppressed. But that seems decidedly nontrivial to me.

No, I do not think there is any expectation of that. The idea of
this file IMO is to record a "blessed" commit which the buildfarm
should compare branch tip to. Nathan is proposing that the file
should also have the function of recording all previous blessed
commits for historical purposes. That's not essential but I can
see some value in it. All it requires from the buildfarm code
is that it take the first non-comment entry in the file.

regards, tom lane

In reply to: Nathan Bossart (#9)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 3:28 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

I was imagining this working more like what Tom suggested. IOW we'd use
the latest commit listed in the file (perhaps always the first one) as the
baseline.

You said "I suppose this idea is entirely dependent on the maintainers
of the abi-compliance-check code to adapt to it", which I understood
to mean that you thought that the upstream tool would somehow be made
to accept these kinds of ignore files. Obviously I misunderstood.

Of course, this doesn't work too well if we have a bunch of ABI
breaks between buildfarm checks. But my guess is that we could deal with
that pretty easily (e.g., make sure the buildfarm member in question runs
for every commit on the stable branch).

In practice I think that it would be up to the person writing the next
suppression to verify that there were no unrelated changes in the
interim between their new blessed/suppression commit and the prior
one. That doesn't seem super onerous to me, given that even false
positives don't seem to be all that common with
abi-compliance-checker.

--
Peter Geoghegan

#12Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#8)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 03:27:10PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

I've attached a first try. You'll notice that I have borrowed heavily from
.git-blame-ignore-revs. Some other things that might be worthwhile:

There would need to be an initial entry at the time the file is
created, which would presumably point to some commit shortly before
the .0 version stamp is applied (or maybe we'd choose to do it around
rc1). The mockup should include that.

Sure, makes sense.

I'd be slightly inclined to have just one non-comment line, which
is the active reference hash value, and all the rest be comments.
The way you have it here requires the reading code to be smart
about end-of-line comments, which is code complexity we don't need
and doesn't seem amazingly legible either. OTOH, the precedent of
.git-blame-ignore-revs may be worth following regardless of our
personal druthers.

That crossed my mind, too. I'm personally not too concerned about small
deviations from .git-blame-ignore-revs, especially if it improves
machine/human readability.

--
nathan

#13Nathan Bossart
nathandbossart@gmail.com
In reply to: Peter Geoghegan (#11)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 03:33:28PM -0400, Peter Geoghegan wrote:

On Fri, Oct 17, 2025 at 3:28 PM Nathan Bossart <nathandbossart@gmail.com> wrote:

I was imagining this working more like what Tom suggested. IOW we'd use
the latest commit listed in the file (perhaps always the first one) as the
baseline.

You said "I suppose this idea is entirely dependent on the maintainers
of the abi-compliance-check code to adapt to it", which I understood
to mean that you thought that the upstream tool would somehow be made
to accept these kinds of ignore files. Obviously I misunderstood.

Sorry, I wasn't clear there.

Of course, this doesn't work too well if we have a bunch of ABI
breaks between buildfarm checks. But my guess is that we could deal with
that pretty easily (e.g., make sure the buildfarm member in question runs
for every commit on the stable branch).

In practice I think that it would be up to the person writing the next
suppression to verify that there were no unrelated changes in the
interim between their new blessed/suppression commit and the prior
one. That doesn't seem super onerous to me, given that even false
positives don't seem to be all that common with
abi-compliance-checker.

Agreed. Even if someone forgets to do that validation, the chances of
missing something seem low.

--
nathan

#14David E. Wheeler
david@justatheory.com
In reply to: Nathan Bossart (#6)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Hackers,

Adding Mankirat, who developed the ABI checker for his GSoC project.

Good idea. We'd have to allow comments in the file, but that's
probably a good thing anyway.

I've attached a first try. You'll notice that I have borrowed heavily from
.git-blame-ignore-revs. Some other things that might be worthwhile:

* Add commentary about when this file is needed (i.e., after the .0).
* Add instructions for creating file on new stable branch to
RELEASE_CHANGES.
* Adjust format for readability. It is a bit comment-heavy at the moment.

Anything else? I suppose this idea is entirely dependent on the
maintainers of the abi-compliance-check code to adapt to it, so we'll need
buy-in from them, too.

Is the idea that the ABI checker just has to scan the first non-comment line that starts with a commit identifier (SHA or tag)? Example from your patch:

```
# This file lists commits on the REL_18_STABLE branch that break ABI
# compatibility in ways that have been deemed acceptable (e.g., removing an
# extern function with no third-party uses). The primary intent of this file
# is to placate the ABI compliance checks on the buildfarm, but it also serves
# as a central location to document the justification for each.
#
# Add new entries by adding the output of the following to the top of the file:
#
# $ git log --pretty=format:"%H # %cd%n# %s" $ABIBREAKGITHASH -1 --date=iso
#
# Be sure to include additional context in a comment below the entry.

c8af5019bee5c57502db830f8005a01cba60fee0 # 2025-10-15 12:47:33 -0500
# Fix lookups in pg_{clear,restore}_{attribute,relation}_stats().
#
# This commit replaced two functions related to lookups/privilege checks for
# the new stats stuff in v18 with RangeVarGetRelidExtended(). These functions
# were not intended for use elsewhere, exist in exactly one release (18.0), and
# do not have any known third-party callers.
--
2.39.5 (Apple Git-154)
```

Seems totally do-able, though I don’t know what that `(Apple Git-154)` bit is doing at the end. I presume it would list the history of changes in reverse chronological order, yes?

If there is a tag _AFTER_ the listed SHA, should we prefer that tag as the baseline?

Best,

David

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#13)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

On Fri, Oct 17, 2025 at 03:33:28PM -0400, Peter Geoghegan wrote:

In practice I think that it would be up to the person writing the next
suppression to verify that there were no unrelated changes in the
interim between their new blessed/suppression commit and the prior
one. That doesn't seem super onerous to me, given that even false
positives don't seem to be all that common with
abi-compliance-checker.

Agreed. Even if someone forgets to do that validation, the chances of
missing something seem low.

I don't see a race condition here. What would happen is we make
some commit, realizing either at the time or later that it involves
an ABI break. We verify via some later buildfarm run that the
break is as-expected (ie the commit doesn't introduce any unwanted
changes, nor is there anything hanging around from some older commit).
Then we push an update to the .abi_reference file that points at
that commit, and the buildfarm starts comparing ABI of branch tip
to that commit instead of whatever was the reference commit before.
No later activity breaks the conclusion that we were okay with the ABI
that that commit creates, nor can any earlier commit cause problems
so long as we did our due diligence in checking the ABI-break reports.

In theory, if two ABI-breaking commits go in so close together that
there was no ABI-checking buildfarm run in between, we might have
difficulty untangling their effects. That doesn't seem very likely
in practice, and even if it happens, so what? Either we're good with
the ABI-break report when we see it, or we're not.

regards, tom lane

#16Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#15)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 03:53:09PM -0400, Tom Lane wrote:

I don't see a race condition here. What would happen is we make
some commit, realizing either at the time or later that it involves
an ABI break. We verify via some later buildfarm run that the
break is as-expected (ie the commit doesn't introduce any unwanted
changes, nor is there anything hanging around from some older commit).
Then we push an update to the .abi_reference file that points at
that commit, and the buildfarm starts comparing ABI of branch tip
to that commit instead of whatever was the reference commit before.
No later activity breaks the conclusion that we were okay with the ABI
that that commit creates, nor can any earlier commit cause problems
so long as we did our due diligence in checking the ABI-break reports.

In theory, if two ABI-breaking commits go in so close together that
there was no ABI-checking buildfarm run in between, we might have
difficulty untangling their effects. That doesn't seem very likely
in practice, and even if it happens, so what? Either we're good with
the ABI-break report when we see it, or we're not.

Ah, I was thinking of a more proactive approach (e.g., I commit something
that I know introduces ABI breakage, and then I immediately update the ABI
reference file in the next commit). I like the idea of simply reacting to
the reports and using that as an opportunity to verify it's what we expect.

--
nathan

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#16)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

On Fri, Oct 17, 2025 at 03:53:09PM -0400, Tom Lane wrote:

I don't see a race condition here. What would happen is we make
some commit, realizing either at the time or later that it involves
an ABI break. We verify via some later buildfarm run that the
break is as-expected (ie the commit doesn't introduce any unwanted
changes, nor is there anything hanging around from some older commit).
Then we push an update to the .abi_reference file that points at
that commit,

Ah, I was thinking of a more proactive approach (e.g., I commit something
that I know introduces ABI breakage, and then I immediately update the ABI
reference file in the next commit). I like the idea of simply reacting to
the reports and using that as an opportunity to verify it's what we expect.

Right. This does mean that those BF members might stay red for a
little bit while we verify that we're seeing expected results, but
I think that's acceptable. Trying to prevent the BF from ever
seeing a bad state seems to me to carry too much risk of masking
problems we didn't expect.

regards, tom lane

#18Nathan Bossart
nathandbossart@gmail.com
In reply to: David E. Wheeler (#14)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 03:52:18PM -0400, David E. Wheeler wrote:

Adding Mankirat, who developed the ABI checker for his GSoC project.

Thanks for chiming in.

Is the idea that the ABI checker just has to scan the first non-comment
line that starts with a commit identifier (SHA or tag)?

Yes.

Seems totally do-able, though I don’t know what that `(Apple Git-154)`
bit is doing at the end.

I think that's just telling you what version of git I used to create the
patch file.

I presume it would list the history of changes in reverse chronological
order, yes?

Yes, although we could change it to whatever we want.

If there is a tag _AFTER_ the listed SHA, should we prefer that tag as
the baseline?

I don't see any need to consider tags at all. We'd initialize this file
when creating the new STABLE branch with a baseline commit near a release
candidate or the .0, and then we'd just add future baselines as needed.
The ABI checks would always use the latest baseline, even if it points to
something before the latest release tag. (At least, this is how I'm
thinking about it.)

--
nathan

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#18)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

On Fri, Oct 17, 2025 at 03:52:18PM -0400, David E. Wheeler wrote:

If there is a tag _AFTER_ the listed SHA, should we prefer that tag as
the baseline?

I don't see any need to consider tags at all. We'd initialize this file
when creating the new STABLE branch with a baseline commit near a release
candidate or the .0, and then we'd just add future baselines as needed.
The ABI checks would always use the latest baseline, even if it points to
something before the latest release tag. (At least, this is how I'm
thinking about it.)

I agree. I don't think the buildfarm should consider git tags at all
in this behavior. One reason is that our release process dictates
applying tags at very specific times that aren't necessarily relevant
to deciding that an ABI break is or is not okay. I think we want
moving the baseline to be a considered reaction to an observed ABI
report, and not an action that is automatic according to some other
process.

regards, tom lane

#20David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#19)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 17, 2025, at 16:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I agree. I don't think the buildfarm should consider git tags at all
in this behavior. One reason is that our release process dictates
applying tags at very specific times that aren't necessarily relevant
to deciding that an ABI break is or is not okay. I think we want
moving the baseline to be a considered reaction to an observed ABI
report, and not an action that is automatic according to some other
process.

Okay, so the rule is:

* If there is no .abi-compliance-history file, baseline from the latest tag
* Otherwise, baseline from the first SHA to appear in the file

Easy peasy.

D

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#20)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

"David E. Wheeler" <david@justatheory.com> writes:

Okay, so the rule is:

* If there is no .abi-compliance-history file, baseline from the latest tag
* Otherwise, baseline from the first SHA to appear in the file

NO. The rule is: if there's no such file, do not apply ABI checking.
We are not interested in ABI complaints against master.

regards, tom lane

#22David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#21)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 17, 2025, at 17:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Okay, so the rule is:

* If there is no .abi-compliance-history file, baseline from the latest tag
* Otherwise, baseline from the first SHA to appear in the file

NO. The rule is: if there's no such file, do not apply ABI checking.
We are not interested in ABI complaints against master.

It only runs against maintenance branches.

D

#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#22)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

"David E. Wheeler" <david@justatheory.com> writes:

On Oct 17, 2025, at 17:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:

NO. The rule is: if there's no such file, do not apply ABI checking.
We are not interested in ABI complaints against master.

It only runs against maintenance branches.

That seems overcomplicated: how does the buildfarm know
what's a maintenance branch? I think the rule should be
just "run ABI checks if the control file exists, else not".

As an example of why that's better, what if we did decide
we wanted ABI checks on master?

regards, tom lane

#24Mankirat Singh
mankiratsingh1315@gmail.com
In reply to: David E. Wheeler (#14)
1 attachment(s)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Sat, 18 Oct 2025 at 01:22, David E. Wheeler <david@justatheory.com>
wrote:

Adding Mankirat, who developed the ABI checker for his GSoC project.

Thanks!

Really interesting discussion.

On Sat, 18 Oct 2025 at 00:51, Peter Geoghegan <pg@bowt.ie> wrote:

On Fri, Oct 17, 2025 at 3:11 PM Nathan Bossart <nathandbossart@gmail.com>

wrote:

Anything else? I suppose this idea is entirely dependent on the
maintainers of the abi-compliance-check code to adapt to it, so we'll

need

buy-in from them, too.

That would require parsing the file and understanding that any
compliance failures associated with a given commit should be
suppressed. But that seems decidedly nontrivial to me. I can easily
think of (admittedly somewhat contrived) scenarios where it's
basically impossible to make this work due to transitive dependencies
across commits.

I suspect that any practical approach to solving this problem will
have to involve ignore files that look somewhat like a Valgrind
suppression file. It'll have to be based on symbol names, plus
possibly a specific ABI breakage type.

The per-branch file containing the commit hash reference for that branch is
a good solution and can be easily implemented since perl is really good at
working with files.

But if we want a much clearer way based on symbol names, we could use a
standard per branch suppression file, which the abidiff tool directly
supports for this purpose[1]https://sourceware.org/libabigail/manual/suppression-specifications.html#code-examples.
For example, each branch could have an `abi.suppr` file with content like
this:

# commit hash: 123456abc
[suppress_function]
name=stats_lock_check_privileges
# comments for this function

# commit hash: oldercommit345
[suppress_type]
type_kind = struct
name = some_struct_name
changed_data_members = updated_member_name

If needed, the contents of this file could be truncated on a new minor
release maybe?

Added a patch for the same for handling current abi compliance failures on
baza.

Regards,
Mankirat

[1]: https://sourceware.org/libabigail/manual/suppression-specifications.html#code-examples
https://sourceware.org/libabigail/manual/suppression-specifications.html#code-examples

Attachments:

0001-PATCH-Add-ABI-suppressions-file-v1.patchtext/x-patch; charset=US-ASCII; name=0001-PATCH-Add-ABI-suppressions-file-v1.patchDownload
From 1adff8bc2de70e4a9d44e6ba84cf9669cc14442b Mon Sep 17 00:00:00 2001
From: Mankirat Singh <mankiratsingh1315@gmail.com>
Date: Sat, 18 Oct 2025 18:24:24 +0530
Subject: [PATCH] [PATCH] Add ABI suppressions file v1

---
 abi.suppr | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 abi.suppr

diff --git a/abi.suppr b/abi.suppr
new file mode 100644
index 00000000000..52bf0ec4fbb
--- /dev/null
+++ b/abi.suppr
@@ -0,0 +1,24 @@
+# This file lists abidiff suppressions on the REL_18_STABLE branch that break ABI
+# compatibility in ways that have been deemed acceptable (e.g., removing an
+# extern function with no third-party uses).  The primary intent of this file
+# is to placate the ABI compliance checks on the buildfarm, but it also serves
+# as a central location to document the justification for each.
+#
+# Add new suppressions as required in reaction to the abi-compliance-check reports
+# on buildfarm.
+
+
+
+# commit hash :c8af5019bee5c57502db830f8005a01cba60fee0
+#
+[suppress_function]
+    name=stats_lock_check_privileges
+#
+[suppress_function]
+    name=stats_lookup_relid
+#
+# Fix lookups in pg_{clear,restore}_{attribute,relation}_stats().
+# This commit replaced two functions related to lookups/privilege checks for
+# the new stats stuff in v18 with RangeVarGetRelidExtended().  These functions
+# were not intended for use elsewhere, exist in exactly one release (18.0), and
+# do not have any known third-party callers.
\ No newline at end of file
-- 
2.43.0

#25Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mankirat Singh (#24)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Mankirat Singh <mankiratsingh1315@gmail.com> writes:

But if we want a much clearer way based on symbol names, we could use a
standard per branch suppression file, which the abidiff tool directly
supports for this purpose[1].

I am strongly against relying on suppression files. I think that
is a very imprecise technology, and it is certainly harder to use
than the "choose a blessed reference point" approach.

regards, tom lane

#26David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#23)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 17, 2025, at 19:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:

That seems overcomplicated: how does the buildfarm know
what's a maintenance branch? I think the rule should be
just "run ABI checks if the control file exists, else not".

As an example of why that's better, what if we did decide
we wanted ABI checks on master?

It’s part of the design of the build farm. The setup() function[0]https://github.com/PGBuildFarm/client-code/pull/38/files#diff-207ca93813cc123f656dbb12b7723d305e9ade5e03d7b1cdb406180e4eaab9a2R194 checks various things to see if it should be run, e.g.,

```perl
if ($^O ne 'linux')
{
emit("Only Linux is supported for ABICompCheck Module, skipping.");
return;
}

# Only proceed if this is a stable branch with git SCM, not using msvc
if ($conf->{scm} ne 'git')
{
emit("Only git SCM is supported for ABICompCheck Module, skipping.");
return;
}
if ($branch !~ /_STABLE$/)
{
emit("Skipping ABI check; '$branch' is not a stable branch.");
return;
}
```

So as long as the branch naming remains consistent it should work.

D

[0]: https://github.com/PGBuildFarm/client-code/pull/38/files#diff-207ca93813cc123f656dbb12b7723d305e9ade5e03d7b1cdb406180e4eaab9a2R194

#27David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#25)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 18, 2025, at 11:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I am strongly against relying on suppression files. I think that
is a very imprecise technology, and it is certainly harder to use
than the "choose a blessed reference point" approach.

Seconded. I’m also not keen on using something specific to libabigail if, long term, we want to enable similar checks on other platforms using other tools that may not support it suppression file format.

Let’s do the baseline SHA file and see how it goes.

Best,

David

#28Mankirat Singh
mankiratsingh1315@gmail.com
In reply to: David E. Wheeler (#27)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Sat, 18 Oct 2025 at 21:17, David E. Wheeler <david@justatheory.com>
wrote:

On Oct 18, 2025, at 11:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I am strongly against relying on suppression files. I think that
is a very imprecise technology, and it is certainly harder to use
than the "choose a blessed reference point" approach.

Seconded. I’m also not keen on using something specific to libabigail if,

long term, we want to enable similar checks on other platforms using other
tools that may not support it suppression file format.

Let’s do the baseline SHA file and see how it goes.

Oh right, I didn't think about the other platforms.

I've implemented the support for .abi-compliance-history file. If the file
is present in a STABLE branch, it will be used by default; otherwise, the
latest tag will be used. You can view the changes here[1]https://github.com/PGBuildFarm/client-code/pull/38/commits/f88873625ba0466e9609225947d40092748ff555.

Regards,
Mankirat

[1]: https://github.com/PGBuildFarm/client-code/pull/38/commits/f88873625ba0466e9609225947d40092748ff555
https://github.com/PGBuildFarm/client-code/pull/38/commits/f88873625ba0466e9609225947d40092748ff555

#29David E. Wheeler
david@justatheory.com
In reply to: Mankirat Singh (#28)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 18, 2025, at 22:48, Mankirat Singh <mankiratsingh1315@gmail.com> wrote:

I've implemented the support for .abi-compliance-history file. If the file is present in a STABLE branch, it will be used by default; otherwise, the latest tag will be used. You can view the changes here[1].

I’ve now deployed this change (well, a subsequent change that does it better) to baza, so once the .abi-compliance-history has been committed to the rev 18 branch (assuming the format is compatible), it should start passing again.

Best,

David

#30Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#29)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

"David E. Wheeler" <david@justatheory.com> writes:

I’ve now deployed this change (well, a subsequent change that does it better) to baza, so once the .abi-compliance-history has been committed to the rev 18 branch (assuming the format is compatible), it should start passing again.

OK, I pushed a placeholder following Nathan's formatting proposal.
The ABI checker should still complain, because I made it point at
REL_18_0^, which is what I expect we'd do in practice. After we
see it respond to that, we can move the reference point to where
it needs to be.

regards, tom lane

#31David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#30)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 20, 2025, at 16:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:

OK, I pushed a placeholder following Nathan's formatting proposal.
The ABI checker should still complain, because I made it point at
REL_18_0^, which is what I expect we'd do in practice. After we
see it respond to that, we can move the reference point to where
it needs to be.

Nice timing, as Mankirat also recently updated the code to determine the files to compare based on feedback from Peter E; see today’s failure[1]https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-10-20%2013%3A11%3A05&amp;stg=abi-compliance-check for an example. Looks like this:

```
Branch: REL_18_STABLE
Git HEAD: 399a9e04e5491f8a76ffb482f4a86b9acb6f91fb
Changes since: REL_18_0

Binaries compared:
bin/postgres
lib/libecpg.so
lib/libecpg_compat.so
lib/libpgtypes.so
lib/libpq.so

log files for step abi-compliance-check:

Leaf changes summary: 2 artifacts changed
Changed leaf types summary: 0 leaf type changed
Removed/Changed/Added functions summary: 2 Removed, 0 Changed, 0 Added function (2 filtered out)
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

2 Removed functions:

[D] 'function void stats_lock_check_privileges(Oid)' {stats_lock_check_privileges}
[D] 'function Oid stats_lookup_relid(const char*, const char*)' {stats_lookup_relid}
```

Should look the same tomorrow but instead say “Changes since REL_18_0^”.

Best,

David

[1]: https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-10-20%2013%3A11%3A05&amp;stg=abi-compliance-check

#32Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#30)
2 attachment(s)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Mon, Oct 20, 2025 at 10:39:19AM -0400, Tom Lane wrote:

OK, I pushed a placeholder following Nathan's formatting proposal.
The ABI checker should still complain, because I made it point at
REL_18_0^, which is what I expect we'd do in practice. After we
see it respond to that, we can move the reference point to where
it needs to be.

Thanks. Here's a new patch set. The v18 patch is just an update to the
.abi-compliance-history file that Tom committed. The master patch adds
instructions for generating the file to RELEASE_NOTES. I imagine we'll
want to add .abi-compliance-history files for the back-branches, too
(except for perhaps v13, which is about to go out of support in a couple
weeks).

--
nathan

Attachments:

v2-0001-Add-notes-for-creating-.abi-compliance-history.patch.mastertext/plain; charset=us-asciiDownload
From 64f9f480c759fabdaecdb7f7e0d1634d3fd6a50d Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 20 Oct 2025 11:43:14 -0500
Subject: [PATCH v2 1/1] Add notes for creating .abi-compliance-history.

---
 src/tools/RELEASE_CHANGES       |  5 +++++
 src/tools/make_abi_history_file | 31 +++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100755 src/tools/make_abi_history_file

diff --git a/src/tools/RELEASE_CHANGES b/src/tools/RELEASE_CHANGES
index c0d75c213be..938dcbb7ab6 100644
--- a/src/tools/RELEASE_CHANGES
+++ b/src/tools/RELEASE_CHANGES
@@ -61,6 +61,11 @@ in both master and the branch.
 * Ports
 	o update ports list in doc/src/sgml/installation.sgml
 
+* Create .abi-compliance-history file with initial entry shortly before the .0
+  stamp by running "src/tools/make_abi_history_file" (from the top of the
+  source tree) and then following the instructions in the file to add the
+  entry.
+
 
 Pre-Beta Tasks
 ==============
diff --git a/src/tools/make_abi_history_file b/src/tools/make_abi_history_file
new file mode 100755
index 00000000000..1c6c1743695
--- /dev/null
+++ b/src/tools/make_abi_history_file
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+# Copyright (c) 2025, PostgreSQL Global Development Group
+#
+# src/tools/make_abi_history_file
+#
+# NB: Must be run from top of source tree!
+
+cat << EOF >> .abi-compliance-history
+# Reference point for ABI compliance checks
+#
+# This file lists commits on the current branch that break ABI compatibility in
+# ways that have been deemed acceptable (e.g., removing an extern function with
+# no third-party uses).  The primary intent of this file is to placate the ABI
+# compliance checks on the buildfarm, but it also serves as a central location
+# to document the justification for each.
+#
+# In general, entries should be added reactively after an abi-compliance-check
+# buildfarm failure.  It is important to verify the details of the breakage
+# match expectations, as the first entry listed will become the updated ABI
+# baseline point.
+#
+# Add new entries by adding the output of the following to the top of the file:
+#
+# $ git log --pretty=format:"%H%n#%n# %s%n# %cd%n#%n# <ADD JUSTIFICATION HERE>" $ABIBREAKGITHASH -1 --date=iso
+#
+# Be sure to replace "<ADD JUSTIFICATION HERE>" with details of your change and
+# why it is deemed acceptable.
+
+
+EOF
-- 
2.39.5 (Apple Git-154)

v2-0001-Update-.abi-compliance-history-file.patch.v18text/plain; charset=us-asciiDownload
From cbebfe6cf49f71e293136bf4ef482f894f0d8adc Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 20 Oct 2025 11:21:04 -0500
Subject: [PATCH v2 1/1] Update .abi-compliance-history file.

---
 .abi-compliance-history | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/.abi-compliance-history b/.abi-compliance-history
index 3d0d26aff90..b7265aa78fe 100644
--- a/.abi-compliance-history
+++ b/.abi-compliance-history
@@ -1,7 +1,36 @@
 # Reference point for ABI compliance checks
+#
+# This file lists commits on the current branch that break ABI compatibility in
+# ways that have been deemed acceptable (e.g., removing an extern function with
+# no third-party uses).  The primary intent of this file is to placate the ABI
+# compliance checks on the buildfarm, but it also serves as a central location
+# to document the justification for each.
+#
+# In general, entries should be added reactively after an abi-compliance-check
+# buildfarm failure.  It is important to verify the details of the breakage
+# match expectations, as the first entry listed will become the updated ABI
+# baseline point.
+#
 # Add new entries by adding the output of the following to the top of the file:
-# $ git log --pretty=format:"%H # %cd%n# %s" $ABIBREAKGITHASH -1 --date=iso
 #
-# Initial entry would normally point to a commit slightly before the .0 stamp.
-9bbcec6030a2744d83311370ec92213fbd76e514 # 2025-09-22 14:18:56 +0200
+# $ git log --pretty=format:"%H%n#%n# %s%n# %cd%n#%n# <ADD JUSTIFICATION HERE>" $ABIBREAKGITHASH -1 --date=iso
+#
+# Be sure to replace "<ADD JUSTIFICATION HERE>" with details of your change and
+# why it is deemed acceptable.
+
+c8af5019bee5c57502db830f8005a01cba60fee0
+#
+# Fix lookups in pg_{clear,restore}_{attribute,relation}_stats().
+# 2025-10-15 12:47:33 -0500
+#
+# This commit replaced two functions related to lookups/privilege checks for
+# the new stats stuff in v18 with RangeVarGetRelidExtended().  These functions
+# were not intended for use elsewhere, exist in exactly one release (18.0), and
+# do not have any known third-party callers.
+
+9bbcec6030a2744d83311370ec92213fbd76e514
+#
 # Translation updates
+# 2025-09-22 14:18:56 +0200
+#
+# This is the original ABI baseline point for REL_18_STABLE.
-- 
2.39.5 (Apple Git-154)

#33Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#32)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

Thanks. Here's a new patch set. The v18 patch is just an update to the
.abi-compliance-history file that Tom committed. The master patch adds
instructions for generating the file to RELEASE_NOTES.

I'd tend to s/placate/control/, otherwise the proposed wording in the
file looks good. I doubt we really need a script to generate the
file in the first place -- why wouldn't copying another branch's
boilerplate be good enough? If you're set on having a script,
at least make it pre-fill the initial entry. (Using branch HEAD
ought to be good enough for that.)

I imagine we'll
want to add .abi-compliance-history files for the back-branches, too
(except for perhaps v13, which is about to go out of support in a couple
weeks).

Agreed, but let's get v18 in shape first. I imagine the back branches
will require some effort to fill in the correct reference commits.
I was expecting we'd commit initial values pointing at the .0 releases
and then seeing what the ABI checker moans about in each branch ...

regards, tom lane

#34Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#33)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Mon, Oct 20, 2025 at 01:07:04PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

Thanks. Here's a new patch set. The v18 patch is just an update to the
.abi-compliance-history file that Tom committed. The master patch adds
instructions for generating the file to RELEASE_NOTES.

I'd tend to s/placate/control/, otherwise the proposed wording in the
file looks good. I doubt we really need a script to generate the
file in the first place -- why wouldn't copying another branch's
boilerplate be good enough? If you're set on having a script,
at least make it pre-fill the initial entry. (Using branch HEAD
ought to be good enough for that.)

I'm fine with leaving out the script if you are. It was only aimed at
making the release checklist a little less cumbersome, but even without the
script it's a whopping minute or two of effort that only needs to happen
once per year. I've probably already spent far more time automating it
than makes sense [0]https://xkcd.com/1205/.

I imagine we'll
want to add .abi-compliance-history files for the back-branches, too
(except for perhaps v13, which is about to go out of support in a couple
weeks).

Agreed, but let's get v18 in shape first. I imagine the back branches
will require some effort to fill in the correct reference commits.
I was expecting we'd commit initial values pointing at the .0 releases
and then seeing what the ABI checker moans about in each branch ...

Agreed.

[0]: https://xkcd.com/1205/

--
nathan

#35Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#34)
2 attachment(s)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Mon, Oct 20, 2025 at 12:14:09PM -0500, Nathan Bossart wrote:

On Mon, Oct 20, 2025 at 01:07:04PM -0400, Tom Lane wrote:

I'd tend to s/placate/control/, otherwise the proposed wording in the
file looks good. I doubt we really need a script to generate the
file in the first place -- why wouldn't copying another branch's
boilerplate be good enough? If you're set on having a script,
at least make it pre-fill the initial entry. (Using branch HEAD
ought to be good enough for that.)

I'm fine with leaving out the script if you are. It was only aimed at
making the release checklist a little less cumbersome, but even without the
script it's a whopping minute or two of effort that only needs to happen
once per year. I've probably already spent far more time automating it
than makes sense [0].

Here is an updated patch set.

--
nathan

Attachments:

v3-0001-Add-notes-for-creating-.abi-compliance-history.patch.mastertext/plain; charset=us-asciiDownload
From 02e535e224b6990fd12359fb4c01c762adacb682 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 20 Oct 2025 11:43:14 -0500
Subject: [PATCH v3 1/1] Add notes for creating .abi-compliance-history.

---
 src/tools/RELEASE_CHANGES | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/tools/RELEASE_CHANGES b/src/tools/RELEASE_CHANGES
index c0d75c213be..a7c20855d54 100644
--- a/src/tools/RELEASE_CHANGES
+++ b/src/tools/RELEASE_CHANGES
@@ -61,6 +61,11 @@ in both master and the branch.
 * Ports
 	o update ports list in doc/src/sgml/installation.sgml
 
+* Create .abi-compliance-history file with initial entry shortly before the .0
+  stamp.  The easiest way to do this is to copy it from the previous
+  REL_*_STABLE branch, remove all entries, and follow the instructions in the
+  file to add the initial reference point for the major version.
+
 
 Pre-Beta Tasks
 ==============
-- 
2.39.5 (Apple Git-154)

v3-0001-Update-.abi-compliance-history-file.patch.v18text/plain; charset=us-asciiDownload
From 93f53c805f4f43dfa79b0d48039999d3b13f5b55 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 20 Oct 2025 11:21:04 -0500
Subject: [PATCH v3 1/1] Update .abi-compliance-history file.

---
 .abi-compliance-history | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/.abi-compliance-history b/.abi-compliance-history
index 3d0d26aff90..68f8b3bcfc1 100644
--- a/.abi-compliance-history
+++ b/.abi-compliance-history
@@ -1,7 +1,36 @@
 # Reference point for ABI compliance checks
+#
+# This file lists commits on the current branch that break ABI compatibility in
+# ways that have been deemed acceptable (e.g., removing an extern function with
+# no third-party uses).  The primary intent of this file is to control the ABI
+# compliance checks on the buildfarm, but it also serves as a central location
+# to document the justification for each.
+#
+# In general, entries should be added reactively after an abi-compliance-check
+# buildfarm failure.  It is important to verify the details of the breakage
+# match expectations, as the first entry listed will become the updated ABI
+# baseline point.
+#
 # Add new entries by adding the output of the following to the top of the file:
-# $ git log --pretty=format:"%H # %cd%n# %s" $ABIBREAKGITHASH -1 --date=iso
 #
-# Initial entry would normally point to a commit slightly before the .0 stamp.
-9bbcec6030a2744d83311370ec92213fbd76e514 # 2025-09-22 14:18:56 +0200
+# $ git log --pretty=format:"%H%n#%n# %s%n# %cd%n#%n# <ADD JUSTIFICATION HERE>" $ABIBREAKGITHASH -1 --date=iso
+#
+# Be sure to replace "<ADD JUSTIFICATION HERE>" with details of your change and
+# why it is deemed acceptable.
+
+c8af5019bee5c57502db830f8005a01cba60fee0
+#
+# Fix lookups in pg_{clear,restore}_{attribute,relation}_stats().
+# 2025-10-15 12:47:33 -0500
+#
+# This commit replaced two functions related to lookups/privilege checks for
+# the new stats stuff in v18 with RangeVarGetRelidExtended().  These functions
+# were not intended for use elsewhere, exist in exactly one release (18.0), and
+# do not have any known third-party callers.
+
+9bbcec6030a2744d83311370ec92213fbd76e514
+#
 # Translation updates
+# 2025-09-22 14:18:56 +0200
+#
+# This is the original ABI baseline point for REL_18_STABLE.
-- 
2.39.5 (Apple Git-154)

#36Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#35)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

Here is an updated patch set.

The v3 patches work for me.

BTW, not critical right now, but thought I'd mention it: ISTM
this mechanism obviates the need for the single-purpose NodeTag ABI
checks installed by commit eea9fa9b2. We still need the checks in
gen_node_support.pl to ensure that the makefiles and meson files
build things the same way, but we shouldn't need the parts that were
intended to prevent accidental ABI changes in the back branches.
Since that stuff requires nonzero manual maintenance, I plan to get
rid of it once we're satisfied that the ABI checker is working well.

regards, tom lane

#37Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#36)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Mon, Oct 20, 2025 at 3:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

Here is an updated patch set.

The v3 patches work for me.

BTW, not critical right now, but thought I'd mention it: ISTM
this mechanism obviates the need for the single-purpose NodeTag ABI
checks installed by commit eea9fa9b2. We still need the checks in
gen_node_support.pl to ensure that the makefiles and meson files
build things the same way, but we shouldn't need the parts that were
intended to prevent accidental ABI changes in the back branches.
Since that stuff requires nonzero manual maintenance, I plan to get
rid of it once we're satisfied that the ABI checker is working well.

Hmm, but: the code in gen_node_support.pl would tell you about trouble
before you committed, whereas the ABI checks would only tell you about
trouble after you commit. It seems to me that we are in desperate need
of reducing, rather than increasing, the number of mistakes you can
make and find out about only after commit.

--
Robert Haas
EDB: http://www.enterprisedb.com

#38Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#37)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Oct 20, 2025 at 3:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

BTW, not critical right now, but thought I'd mention it: ISTM
this mechanism obviates the need for the single-purpose NodeTag ABI
checks installed by commit eea9fa9b2.

Hmm, but: the code in gen_node_support.pl would tell you about trouble
before you committed, whereas the ABI checks would only tell you about
trouble after you commit. It seems to me that we are in desperate need
of reducing, rather than increasing, the number of mistakes you can
make and find out about only after commit.

There are enough other ways to break ABI that I don't think catching
just this one before commit will move the needle for anyone. Instead,
the mistake I'm hoping to eliminate here is forgetting to arm the
NodeTag ABI check, or doing it wrong.

I do take your point that being able to find things before commit
is helpful. But I think the answer to that is to get this
general-purpose ABI check mechanism sufficiently well product-ized
that committers can run it locally if they choose. Ideally we'd
have multiple BF animals running it, so there's definitely motivation
to get it at least to the point where it doesn't require hand-feeding
by BF owners. (If memory serves, we've had ABI breaks that affected
only 32 bit or only 64 bit machines, and of course there's the
possibility of ones that only manifest with particular feature
selections. So I'm not content with just one animal running it.)

regards, tom lane

#39Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#36)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Mon, Oct 20, 2025 at 03:33:53PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

Here is an updated patch set.

The v3 patches work for me.

Thanks. If baza's next run looks good, I'll proceed with committing.

--
nathan

#40David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#33)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 20, 2025, at 19:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Agreed, but let's get v18 in shape first. I imagine the back branches
will require some effort to fill in the correct reference commits.
I was expecting we'd commit initial values pointing at the .0 releases
and then seeing what the ABI checker moans about in each branch …

FWIW they all fail. I pinned them all to the .1 tags for a couple days last month. Results:

curiously, all the back branches failed when comparing to their .1 tags.

* [REL_18_RC1 => c70b6db](https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-09-05%2013%3A10%3A34&amp;stg=abi-compliance-check)
* [REL_17_1 => 6195afb](https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-09-05%2012%3A54%3A38&amp;stg=abi-compliance-check)
* [REL_16_1 => 21a61b8](https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-09-05%2012%3A37%3A45&amp;stg=abi-compliance-check)
* [REL_15_1 => f871fba](https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-09-05%2012%3A21%3A55&amp;stg=abi-compliance-check)
* [REL_14_1 => ea65c88](https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-09-05%2012%3A10%3A11&amp;stg=abi-compliance-check)
* [REL_13_1 => dbef9cb](https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=baza&amp;dt=2025-09-05%2012%3A00%3A02&amp;stg=abi-compliance-check)

D

#41David E. Wheeler
david@justatheory.com
In reply to: Nathan Bossart (#39)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 20, 2025, at 22:52, Nathan Bossart <nathandbossart@gmail.com> wrote:

Thanks. If baza's next run looks good, I'll proceed with committing.

I suggest mentioning that new entries should be at the top, that the list should be in reverse chronological order.

Otherwise this looks great, love seeing it!

D

#42David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#38)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 20, 2025, at 22:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I do take your point that being able to find things before commit
is helpful. But I think the answer to that is to get this
general-purpose ABI check mechanism sufficiently well product-ized
that committers can run it locally if they choose. Ideally we'd
have multiple BF animals running it, so there's definitely motivation
to get it at least to the point where it doesn't require hand-feeding
by BF owners. (If memory serves, we've had ABI breaks that affected
only 32 bit or only 64 bit machines, and of course there's the
possibility of ones that only manifest with particular feature
selections. So I'm not content with just one animal running it.)

FWIW, running it on a Linux animal currently requires just a couple steps

1. Download the module:

```sh
curl -LO https://raw.githubusercontent.com/MankiratSingh1315/pg-bf-client-code/refs/heads/abi-comp-check/PGBuild/Modules/ABICompCheck.pm
mv ABICompCheck.pm build-farm-path/PGBuild/Modules/
```

2. Add it to `modules` in `build-farm.conf`, e.g.,

modules => [qw(TestUpgrade ABICompCheck)],

3. Install the abigail suite; I believe the Debian packages are `abigail-tools` and `libabigail0`

I think that’s it. I use `run_branches.pl --run-all` to test all the current maintenance branches. It does not run against master.

Best,

David

#43Nathan Bossart
nathandbossart@gmail.com
In reply to: David E. Wheeler (#41)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Mon, Oct 20, 2025 at 11:22:29PM +0200, David E. Wheeler wrote:

I suggest mentioning that new entries should be at the top, that the list
should be in reverse chronological order.

I felt that this was already covered in the existing commentary.

Otherwise this looks great, love seeing it!

Thanks for looking. Now that we have a new run from baza that appears to
be using the updated baseline, I've committed the remaining patches.

For the rest of the back-branches, I'm considering starting with a baseline
of the latest minor version stamps. While it would be nice to have a
comprehensive history of the ABI compatibility for each major version,
we've lived this long without it, and I think it's unlikely that we'd act
on any breakages that predate the latest release set. Thoughts?

--
nathan

#44Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#43)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

For the rest of the back-branches, I'm considering starting with a baseline
of the latest minor version stamps. While it would be nice to have a
comprehensive history of the ABI compatibility for each major version,
we've lived this long without it, and I think it's unlikely that we'd act
on any breakages that predate the latest release set. Thoughts?

Agreed that building a full list of ABI-changing commits in those
branches is probably not worth the trouble at this point. (My OCD
side kind of wants to do it anyway ... but it's hard to argue that
we'd get real value out of it, or that we'd change anything now
unless we get complaints.)

I searched the commit log, and found that the most recent intentional,
or at least documented-in-the-log, ABI break was here:

Author: Masahiko Sawada <msawada@postgresql.org>
Branch: master Release: REL_18_BR [d87d07b7a] 2025-06-16 17:36:01 -0700
Branch: REL_17_STABLE Release: REL_17_6 [45c357e0e] 2025-06-16 17:35:58 -0700
Branch: REL_16_STABLE Release: REL_16_10 [b2ae07720] 2025-06-16 17:35:55 -0700
Branch: REL_15_STABLE Release: REL_15_14 [fc0fb77c5] 2025-06-16 17:35:53 -0700
Branch: REL_14_STABLE Release: REL_14_19 [983b36362] 2025-06-16 17:35:50 -0700
Branch: REL_13_STABLE Release: REL_13_22 [1230be12f] 2025-06-16 17:35:48 -0700

...
Note that this commit adds two new fields to ReorderBufferTXN to store
the distributed transactions. This change breaks ABI compatibility in
back branches, affecting third-party extensions that depend on the
size of the ReorderBufferTXN struct, though this scenario seems
unlikely.
...

I propose that we should make .abi-compliance-history point at these
commits rather than REL_17_6 et al. If there is anything later than
this that affected ABI, it'd be worth finding out I think, even though
we might choose not to do anything about it. (In effect, we'd be
starting the ABI compliance histories there rather than all the way
back at the .0 releases.)

regards, tom lane

#45Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#44)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Tue, Oct 21, 2025 at 04:13:37PM -0400, Tom Lane wrote:

I propose that we should make .abi-compliance-history point at these
commits rather than REL_17_6 et al. If there is anything later than
this that affected ABI, it'd be worth finding out I think, even though
we might choose not to do anything about it. (In effect, we'd be
starting the ABI compliance histories there rather than all the way
back at the .0 releases.)

WFM. I'll make it so.

I see that baza is currently using the latest tags for <v18. David, will
it start using the .abi-compliance-history file on the rest of the
back-branches once it is added?

--
nathan

#46David E. Wheeler
david@justatheory.com
In reply to: Nathan Bossart (#45)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 21, 2025, at 22:21, Nathan Bossart <nathandbossart@gmail.com> wrote:

I see that baza is currently using the latest tags for <v18. David, will
it start using the .abi-compliance-history file on the rest of the
back-branches once it is added?

It should, yes. I added the latest changes from Mankirat to add support for it.

D

#47Nathan Bossart
nathandbossart@gmail.com
In reply to: David E. Wheeler (#46)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Tue, Oct 21, 2025 at 10:44:19PM +0200, David E. Wheeler wrote:

On Oct 21, 2025, at 22:21, Nathan Bossart <nathandbossart@gmail.com> wrote:

I see that baza is currently using the latest tags for <v18. David, will
it start using the .abi-compliance-history file on the rest of the
back-branches once it is added?

It should, yes. I added the latest changes from Mankirat to add support
for it.

Great. All back-branches now have an .abi-compliance-history file.

--
nathan

#48Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#47)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

Great. All back-branches now have an .abi-compliance-history file.

Looks like we're good on v13-v16, but v17 is complaining

1 Removed function:

[D] 'function bool check_max_slot_wal_keep_size(int*, void**, GucSource)' {check_max_slot_wal_keep_size}

so I guess we need to add 24f6c1bd4 to that history.

regards, tom lane

#49Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#48)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Wed, Oct 22, 2025 at 10:40:50AM -0400, Tom Lane wrote:

so I guess we need to add 24f6c1bd4 to that history.

Yup. Will do.

--
nathan

#50Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#49)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Wed, Oct 22, 2025 at 11:33:20AM -0500, Nathan Bossart wrote:

On Wed, Oct 22, 2025 at 10:40:50AM -0400, Tom Lane wrote:

so I guess we need to add 24f6c1bd4 to that history.

Yup. Will do.

Ope, looks like you took care of it already, thanks.

--
nathan

#51David E. Wheeler
david@justatheory.com
In reply to: Nathan Bossart (#50)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 22, 2025, at 18:36, Nathan Bossart <nathandbossart@gmail.com> wrote:

so I guess we need to add 24f6c1bd4 to that history.

Yup. Will do.

Ope, looks like you took care of it already, thanks.

Looks like everything’s back to green. Thanks everyone, this sort of setup is exactly what I had hoped to see from Mankirat’s project, and I appreciate hackers’ support for it!

Best,

David

#52Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#23)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 17, 2025 at 07:07:36PM -0400, Tom Lane wrote:

"David E. Wheeler" <david@justatheory.com> writes:

On Oct 17, 2025, at 17:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:

NO. The rule is: if there's no such file, do not apply ABI checking.
We are not interested in ABI complaints against master.

It only runs against maintenance branches.

That seems overcomplicated: how does the buildfarm know
what's a maintenance branch? I think the rule should be
just "run ABI checks if the control file exists, else not".

As an example of why that's better, what if we did decide
we wanted ABI checks on master?

I assume we would want ABI breakage checks on master between Beta 1 and
the time we branch for the new major release in July.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Do not let urgent matters crowd out time for investment in the future.

#53Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#52)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Oct 17, 2025 at 07:07:36PM -0400, Tom Lane wrote:

That seems overcomplicated: how does the buildfarm know
what's a maintenance branch? I think the rule should be
just "run ABI checks if the control file exists, else not".

I assume we would want ABI breakage checks on master between Beta 1 and
the time we branch for the new major release in July.

In the past we've never really thought that ABI was more than mildly
solidified until around rc1. On the whole I'd rather wait until after
the branch before starting to check ABI, simply because I don't care
for the idea of adding .abi-compliance-history in the master branch
only to remove it again later. Having said that, it would be good
if we *could* choose to do that, so I still do not like having any
policy decisions about which branches to check hard-wired into the
buildfarm client.

regards, tom lane

#54David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#53)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 29, 2025, at 17:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:

In the past we've never really thought that ABI was more than mildly
solidified until around rc1. On the whole I'd rather wait until after
the branch before starting to check ABI, simply because I don't care
for the idea of adding .abi-compliance-history in the master branch
only to remove it again later. Having said that, it would be good
if we *could* choose to do that, so I still do not like having any
policy decisions about which branches to check hard-wired into the
buildfarm client.

Well that’s pretty easily addressed by adding a configuration for it. Maybe a regex to match branches, defaulting to its current value[0]https://github.com/MankiratSingh1315/pg-bf-client-code/blob/abi-comp-check/PGBuild/Modules/ABICompCheck.pm#L248, `/_STABLE$/`.

D

[0]: https://github.com/MankiratSingh1315/pg-bf-client-code/blob/abi-comp-check/PGBuild/Modules/ABICompCheck.pm#L248

#55Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#54)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

"David E. Wheeler" <david@justatheory.com> writes:

On Oct 29, 2025, at 17:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:

... I still do not like having any
policy decisions about which branches to check hard-wired into the
buildfarm client.

Well that’s pretty easily addressed by adding a configuration for it. Maybe a regex to match branches, defaulting to its current value[0], `/_STABLE$/`.

I'm asking to remove complexity, not add more. The buildfarm client
should not be checking either branch names or git tags to control
this, when we have a perfectly good convention about the existence
of .abi-compliance-history to control it.

regards, tom lane

#56David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#55)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 29, 2025, at 19:52, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm asking to remove complexity, not add more. The buildfarm client
should not be checking either branch names or git tags to control
this, when we have a perfectly good convention about the existence
of .abi-compliance-history to control it.

We can remove the branch check, of course, but then you would have to maintain .abi-compliance-history in master, no?

D

#57Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#56)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

"David E. Wheeler" <david@justatheory.com> writes:

On Oct 29, 2025, at 19:52, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm asking to remove complexity, not add more. The buildfarm client
should not be checking either branch names or git tags to control
this, when we have a perfectly good convention about the existence
of .abi-compliance-history to control it.

We can remove the branch check, of course, but then you would have to maintain .abi-compliance-history in master, no?

No; my proposal was "don't run the ABI check unless the branch has
a .abi-compliance-history file". master would normally not contain
such a file, thus no check. As I was just discussing with Bruce,
we could put one there for awhile if we wanted to run ABI checks in
advance of forking a stable branch.

The reason I'm so allergic to having any of these decisions made on
the buildfarm-client side is years of herding cats^W^W trying to get
buildfarm owners to update their script versions and/or config files.
It's close to hopeless. Thus, your proposal a message or three back
to add another BF client config setting to control this sounds like
the worst of all possible worlds. If we needed a change in the
setting, getting the farm to converge to that would take months if not
years. The idea that we could transiently enable checks between beta1
and branch fork on the basis of that approach is downright risible.
On the other hand, if the decisions are driven purely by what is in
our git tree, a change is the work of moments.

regards, tom lane

#58David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#57)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 29, 2025, at 22:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:

We can remove the branch check, of course, but then you would have to maintain .abi-compliance-history in master, no?

No; my proposal was "don't run the ABI check unless the branch has
a .abi-compliance-history file". master would normally not contain
such a file, thus no check. As I was just discussing with Bruce,
we could put one there for awhile if we wanted to run ABI checks in
advance of forking a stable branch.

Oh, I see, that’s certainly do-able.

The reason I'm so allergic to having any of these decisions made on
the buildfarm-client side is years of herding cats^W^W trying to get
buildfarm owners to update their script versions and/or config files.
It's close to hopeless. Thus, your proposal a message or three back
to add another BF client config setting to control this sounds like
the worst of all possible worlds. If we needed a change in the
setting, getting the farm to converge to that would take months if not
years. The idea that we could transiently enable checks between beta1
and branch fork on the basis of that approach is downright risible.
On the other hand, if the decisions are driven purely by what is in
our git tree, a change is the work of moments.

That makes excellent sense, I appreciate the context.

Might I suggest, however, that it also run for branches matching /_STABLE$/ even if there is no .abi-hstory-file? The whole point of this exercise was to increase the confidence in ABI stability in stable branches. Running it in such branches would help remind maintainers to add the file.

OTOH, if someone ran it for really old branches it could bs super annoying, so maybe not…

D

#59Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#58)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

"David E. Wheeler" <david@justatheory.com> writes:

Might I suggest, however, that it also run for branches matching /_STABLE$/ even if there is no .abi-hstory-file? The whole point of this exercise was to increase the confidence in ABI stability in stable branches. Running it in such branches would help remind maintainers to add the file.

Trouble is, you then need an arbitrary client-made choice about which
commit to run the ABI check against. If that code does something we
realize we don't want, we're back up against the problem of moving the
buildfarm configuration to fix it. I'd rather the decision be opt-in.

(Also, the only rules I heard proposed for such client-driven choices
involved git tags. I already explained why I don't want that: git
tags are hard to modify and subject to too many other constraints.)

regards, tom lane

#60David E. Wheeler
david@justatheory.com
In reply to: Tom Lane (#59)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 30, 2025, at 09:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Trouble is, you then need an arbitrary client-made choice about which
commit to run the ABI check against.

It’s currently coded to use the most recent tag or, if there is none in the branch, the branch root.

If that code does something we
realize we don't want, we're back up against the problem of moving the
buildfarm configuration to fix it. I'd rather the decision be opt-in.

Fair. Just means that if no one adds a history file to a branch that branch will never be tested and there’s no automated way to realize it.

(Also, the only rules I heard proposed for such client-driven choices
involved git tags. I already explained why I don't want that: git
tags are hard to modify and subject to too many other constraints.)

Yeah, it just went with the most recent tag to keep it simple, no other metadata.

D

#61Mankirat Singh
mankiratsingh1315@gmail.com
In reply to: David E. Wheeler (#60)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Thu, 30 Oct 2025 at 19:40, David E. Wheeler <david@justatheory.com> wrote:

On Oct 30, 2025, at 09:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Trouble is, you then need an arbitrary client-made choice about which
commit to run the ABI check against.

It’s currently coded to use the most recent tag or, if there is none in the branch, the branch root.

Yes, like before the addition of .abi-compliance-history in the
REL_18_STABLE branch, REL_18_0 was being used as the baseline.

If that code does something we
realize we don't want, we're back up against the problem of moving the
buildfarm configuration to fix it. I'd rather the decision be opt-in.

No changes to individual animal configurations will be required. Once
a STABLE branch gets the .abi-compliance-history file, the baseline
will update automatically from the lastest tag to the mentioned commit
SHA for all clients. :D

Fair. Just means that if no one adds a history file to a branch that branch will never be tested and there’s no automated way to realize it.

Although I don’t oppose the idea of “don’t run the ABI check unless
the branch has a .abi-compliance-history file”, it would just need
some minor code removals and adjustments.
We can put a note in the compliance check result for new STABLE
branches - "no .abi-compliance-history file found", but keep the
client status green?

#62Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#57)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Wed, Oct 29, 2025 at 10:49:24PM -0400, Tom Lane wrote:

No; my proposal was "don't run the ABI check unless the branch has
a .abi-compliance-history file". master would normally not contain
such a file, thus no check. As I was just discussing with Bruce,
we could put one there for awhile if we wanted to run ABI checks in
advance of forking a stable branch.

The reason I'm so allergic to having any of these decisions made on
the buildfarm-client side is years of herding cats^W^W trying to get
buildfarm owners to update their script versions and/or config files.
It's close to hopeless. Thus, your proposal a message or three back
to add another BF client config setting to control this sounds like
the worst of all possible worlds. If we needed a change in the
setting, getting the farm to converge to that would take months if not
years. The idea that we could transiently enable checks between beta1
and branch fork on the basis of that approach is downright risible.
On the other hand, if the decisions are driven purely by what is in
our git tree, a change is the work of moments.

I agree with Tom. The lack of an .abi-compliance-history file should be
taken to mean that we're not interested in maintaining ABI compatibility
across commits for that branch, and the buildfarm check should be skipped.

--
nathan

#63Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#53)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Wed, Oct 29, 2025 at 05:37:36PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Oct 17, 2025 at 07:07:36PM -0400, Tom Lane wrote:

That seems overcomplicated: how does the buildfarm know
what's a maintenance branch? I think the rule should be
just "run ABI checks if the control file exists, else not".

I assume we would want ABI breakage checks on master between Beta 1 and
the time we branch for the new major release in July.

In the past we've never really thought that ABI was more than mildly
solidified until around rc1. On the whole I'd rather wait until after
the branch before starting to check ABI, simply because I don't care
for the idea of adding .abi-compliance-history in the master branch
only to remove it again later. Having said that, it would be good
if we *could* choose to do that, so I still do not like having any
policy decisions about which branches to check hard-wired into the
buildfarm client.

I guess I would like to be _notified_, in some way, of ABI breaks during
that period.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Do not let urgent matters crowd out time for investment in the future.

#64David E. Wheeler
david@justatheory.com
In reply to: Mankirat Singh (#61)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 30, 2025, at 11:22, Mankirat Singh <mankiratsingh1315@gmail.com> wrote:

We can put a note in the compliance check result for new STABLE
branches - "no .abi-compliance-history file found", but keep the
client status green?

Yes, I think that’s the thing to do.

D

#65Robert Haas
robertmhaas@gmail.com
In reply to: David E. Wheeler (#58)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Thu, Oct 30, 2025 at 8:42 AM David E. Wheeler <david@justatheory.com> wrote:

Might I suggest, however, that it also run for branches matching /_STABLE$/ even if there is no .abi-hstory-file? The whole point of this exercise was to increase the confidence in ABI stability in stable branches. Running it in such branches would help remind maintainers to add the file.

I think it is better to do as Tom suggests -- i.e. have a buildfarm
client that only cares about the presence of the file, and not at all
about the name of the branch. That gives us the maximum amount of
future flexibility for no real cost. Said differently, if somebody
nukes the ABI history file from a back-branch, I think we should
assume that represents a deliberate decision on the part of the
project to stop caring about ABI compatibility in that particular
back-branch, not an error that the build-farm needs to flag. Of
course, if somebody wants to argue that such a decision was wrongly
made, they're free to do so on pgsql-hackers, but we'd like the
buildfarm to be green rather than red while that's being litigated.

--
Robert Haas
EDB: http://www.enterprisedb.com

#66Peter Eisentraut
peter@eisentraut.org
In reply to: Tom Lane (#44)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On 21.10.25 22:13, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

For the rest of the back-branches, I'm considering starting with a baseline
of the latest minor version stamps. While it would be nice to have a
comprehensive history of the ABI compatibility for each major version,
we've lived this long without it, and I think it's unlikely that we'd act
on any breakages that predate the latest release set. Thoughts?

Agreed that building a full list of ABI-changing commits in those
branches is probably not worth the trouble at this point. (My OCD
side kind of wants to do it anyway ... but it's hard to argue that
we'd get real value out of it, or that we'd change anything now
unless we get complaints.)

What is the reason that this file is supposed to contain the history of
relevant changes, rather than just the last one?

If you want the history, you could look at the git log of the file
itself, no?

#67David E. Wheeler
david@justatheory.com
In reply to: Robert Haas (#65)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 30, 2025, at 16:10, Robert Haas <robertmhaas@gmail.com> wrote:

Said differently, if somebody
nukes the ABI history file from a back-branch, I think we should
assume that represents a deliberate decision on the part of the
project to stop caring about ABI compatibility in that particular
back-branch, not an error that the build-farm needs to flag.

I was thinking less of old branches than brand new ones.

I’m on board with requiring an ABI history file, and that it’s part of the branch for release process to create one should minimize the chances that someone forgets long enough for an ABI change to go uncaught and released.

Best,

David

#68Nathan Bossart
nathandbossart@gmail.com
In reply to: Peter Eisentraut (#66)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Fri, Oct 31, 2025 at 12:02:10PM +0100, Peter Eisentraut wrote:

What is the reason that this file is supposed to contain the history of
relevant changes, rather than just the last one?

If you want the history, you could look at the git log of the file itself,
no?

I think either way would ultimately be fine. I might argue that keeping
the full history in a file with detailed explanations is more accessible
than requiring folks to run

git log .abi-compliance-history

(Plus, if someone doesn't bother to put details in the commit message, you
then have to sleuth further to figure out what changed.)

--
nathan

#69Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nathan Bossart (#68)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

Nathan Bossart <nathandbossart@gmail.com> writes:

On Fri, Oct 31, 2025 at 12:02:10PM +0100, Peter Eisentraut wrote:

What is the reason that this file is supposed to contain the history of
relevant changes, rather than just the last one?
If you want the history, you could look at the git log of the file itself,
no?

I think either way would ultimately be fine. I might argue that keeping
the full history in a file with detailed explanations is more accessible
than requiring folks to run
git log .abi-compliance-history
(Plus, if someone doesn't bother to put details in the commit message, you
then have to sleuth further to figure out what changed.)

Yeah, that last. I would expect the annotation text in
.abi-compliance-history to be specific about the nature of the ABI
break, whereas the log message for the commit that changed things
might not bother with such details.

As we move along with this effort, we might ultimately decide that
this scheme is overdoing the amount of detail recorded. But to
start with, I'd rather err on the side of recording more info
not less.

regards, tom lane

#70David E. Wheeler
david@justatheory.com
In reply to: David E. Wheeler (#64)
Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()

On Oct 30, 2025, at 13:03, David E. Wheeler <david@justatheory.com> wrote:

On Oct 30, 2025, at 11:22, Mankirat Singh <mankiratsingh1315@gmail.com> wrote:

We can put a note in the compliance check result for new STABLE
branches - "no .abi-compliance-history file found", but keep the
client status green?

Yes, I think that’s the thing to do.

FYI, Mankirat made this change[0] and I’ve deployed it to baza. It will no only run for branches that contain the history file unless the animal owner configures specific baselines in the configuration file (mainly intended for testing).

Best,

David