btoidsortsupport issue
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253363psql -h localhost -U postgresBuilt from patchset v1 (message #1), August 09, 2026 at 08:31 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253363_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253363_1 && git checkout t253363_1Patchset v1 (message #1) is on t253363_1
I had Opus 5 look for bugs in recent Postgres commits. It authored the
test case added by the attached patch. The test fails for me on master
after commit 51cd5d6f:
# ORDER BY f1;
# f1
# ------------
# 5
# 10
# 15
# 987
# 1234
# 1235
# 99999999
# - 4294966256
# 4294967000
# + 4294966256
# (9 rows)
btoidsortsupport() uses ssup_datum_unsigned_cmp() at that commit,
which compares all 64 bits of each Datum. The test case shows that the
same oid value can reach the comparator with the extraneous bits
filled two different ways. Tuple deformation sign-extends every 4-byte
pass-by-value attribute (see fetch_att() and friends), whereas
ObjectIdGetDatum() zero-extends instead. This inconsistency makes
their datums incompatible for ssup_datum_unsigned_cmp's purposes,
leading to wrong answers in certain narrow cases.
--
Peter Geoghegan
On Mon, 10 Aug 2026 at 08:19, Peter Geoghegan <pg@bowt.ie> wrote:
btoidsortsupport() uses ssup_datum_unsigned_cmp() at that commit,
which compares all 64 bits of each Datum. The test case shows that the
same oid value can reach the comparator with the extraneous bits
filled two different ways. Tuple deformation sign-extends every 4-byte
pass-by-value attribute (see fetch_att() and friends), whereas
ObjectIdGetDatum() zero-extends instead. This inconsistency makes
their datums incompatible for ssup_datum_unsigned_cmp's purposes,
leading to wrong answers in certain narrow cases.
This seems to be a duplicate of [1]/messages/by-id/CA+fm-RMyLC94NfrxCh273+dKs44U0ZJjRczznvzvgw=KtpPNVw@mail.gmail.com.
David
[1]: /messages/by-id/CA+fm-RMyLC94NfrxCh273+dKs44U0ZJjRczznvzvgw=KtpPNVw@mail.gmail.com
David Rowley <dgrowleyml@gmail.com> writes:
This seems to be a duplicate of [1].
I think Peter's point is that that commit (51cd5d6f0) is in
fact incorrect.
regards, tom lane
On Mon, 10 Aug 2026 at 09:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowleyml@gmail.com> writes:
This seems to be a duplicate of [1].
I think Peter's point is that that commit (51cd5d6f0) is in
fact incorrect.
What Peter pointed out has already been pointed out in the thread that
I linked to. I felt it was useful to point out the duplicate to avoid
anyone wasting time looking deeper or producing a duplicate fix. Maybe
I've missed something, but I did try John's fix, and it does make
Peter's test pass, so to me it seems like a duplicate report. What
did I miss?
David
On Sun, Aug 9, 2026 at 5:36 PM David Rowley <dgrowleyml@gmail.com> wrote:
What Peter pointed out has already been pointed out in the thread that
I linked to. I felt it was useful to point out the duplicate to avoid
anyone wasting time looking deeper or producing a duplicate fix.
I agree that this is a duplicate of that known issue. Pointing that
out to me was useful.
Maybe I've missed something, but I did try John's fix, and it does make
Peter's test pass, so to me it seems like a duplicate report. What
did I miss?
You linked to the original feature thread message, not the specific
message with the bug report, which was confusing.
--
Peter Geoghegan
David Rowley <dgrowleyml@gmail.com> writes:
On Mon, 10 Aug 2026 at 09:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think Peter's point is that that commit (51cd5d6f0) is in
fact incorrect.
What Peter pointed out has already been pointed out in the thread that
I linked to. I felt it was useful to point out the duplicate to avoid
anyone wasting time looking deeper or producing a duplicate fix. Maybe
I've missed something, but I did try John's fix, and it does make
Peter's test pass, so to me it seems like a duplicate report. What
did I miss?
Ah, nothing. You pointed to the original message with the faulty
patch, not to the downthread message that said it was wrong, so
I misunderstood what you were saying.
regards, tom lane
On Mon, 10 Aug 2026 at 09:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowleyml@gmail.com> writes:
What Peter pointed out has already been pointed out in the thread that
I linked to. I felt it was useful to point out the duplicate to avoid
anyone wasting time looking deeper or producing a duplicate fix. Maybe
I've missed something, but I did try John's fix, and it does make
Peter's test pass, so to me it seems like a duplicate report. What
did I miss?Ah, nothing. You pointed to the original message with the faulty
patch, not to the downthread message that said it was wrong, so
I misunderstood what you were saying.
Oh sorry. I should have paid more attention. I'd mistakenly thought
the whole thread was about the bug. I only got involved halfway
through and didn't read the initial email. Ooops.
David
On Mon, Aug 10, 2026 at 4:07 AM David Rowley <dgrowleyml@gmail.com> wrote:
On Mon, 10 Aug 2026 at 08:19, Peter Geoghegan <pg@bowt.ie> wrote:
btoidsortsupport() uses ssup_datum_unsigned_cmp() at that commit,
which compares all 64 bits of each Datum. The test case shows that the
same oid value can reach the comparator with the extraneous bits
filled two different ways. Tuple deformation sign-extends every 4-byte
pass-by-value attribute (see fetch_att() and friends), whereas
ObjectIdGetDatum() zero-extends instead. This inconsistency makes
their datums incompatible for ssup_datum_unsigned_cmp's purposes,
leading to wrong answers in certain narrow cases.This seems to be a duplicate of [1].
David
[1] /messages/by-id/CA+fm-RMyLC94NfrxCh273+dKs44U0ZJjRczznvzvgw=KtpPNVw@mail.gmail.com
For the archives, I've now committed the fix discussed in that thread.
I went ahead and incorporated Peter's regression test since my draft
unnecessarily used a separate temp table
--
John Naylor
Amazon Web Services