Implicit conversion from int64 to int32 when calling hash_get_num_entries
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:t253066psql -h localhost -U postgresBuilt from patchset v7 (message #7), August 14, 2026 at 08:24 AM.
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 t253066_7 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 t253066_7 && git checkout t253066_7Patchset v7 (message #7) is on t253066_7
Hi hackers!
In 13b935cd hash_get_num_entries was changed to return int64 instead of
long. I noticed that there is one more usage of it in pg_stat_statements
that should be cleaned up. There hash_get_num_entries's return value is
assigned to int32, so it was technically a defect even before 13b935cd.
That's also probably a reason why it was missed. There are some other
hash_get_num_entries usages in backend where hash_get_num_entries's
return value is assigned to int32, but I was going to look into them
later.
In practice number of entries in pgss_hash should not exceed pgss_max,
and we should be okay while pgss_max fits int32.
Simply changing num_entries to int64 leads to the change of the file
header format. I suggest changing num_entries to int64 in PG19 and
later, because it wasn't released yet, and pgss files generated by
another major version are considered to be incompatible anyway. See
the v1 patch attached.
Doing the same in already released versions would require increasing
PGSS_FILE_HEADER version. I don't think it's worth it, so for older
versions I suggest just adding a comment, a check, and an explicit
conversion to int32. See the second patch attached.
Another minor problem I noticed was the definition of pgver. It's
defined as int32, but used as uint32. So I fixed it in both patches.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
Attachments:
t253066_1v1-0001-Use-int64-for-number-of-entries-in-pg_stat_statem.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Use-int64-for-number-of-entries-in-pg_stat_statem.patchDownload+6-7
0001-pg_stat_statements-explicitly-convert-number-of-entr.patchtext/x-patch; charset=US-ASCII; name=0001-pg_stat_statements-explicitly-convert-number-of-entr.patchDownload+8-3
On 13 Jul 2026, at 17:00, Karina Litskevich <litskevichkarina@gmail.com> wrote:
Simply changing num_entries to int64 leads to the change of the file
header format. I suggest changing num_entries to int64 in PG19 and
later, because it wasn't released yet, and pgss files generated by
another major version are considered to be incompatible anyway. See
the v1 patch attached.
Agreed, I think we should make sure to do this once beta2 has shipped for the
next beta release.
Doing the same in already released versions would require increasing
PGSS_FILE_HEADER version. I don't think it's worth it, so for older
versions I suggest just adding a comment, a check, and an explicit
conversion to int32. See the second patch attached.
Given the likelihood of an issue in backbranches, we could also only apply for
v19 and leave the backbranches.
Another minor problem I noticed was the definition of pgver. It's
defined as int32, but used as uint32. So I fixed it in both patches.
Makes sense.
--
Daniel Gustafsson
Thank you for the review, Daniel.
On Mon, Jul 13, 2026 at 11:36 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Given the likelihood of an issue in backbranches, we could also only apply for
v19 and leave the backbranches.
I'm okay with that too.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
I've created a CF entry. I'm sorry it took so long. Life happened.
https://commitfest.postgresql.org/patch/7082/
On Mon, Jul 13, 2026 at 11:36 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Simply changing num_entries to int64 leads to the change of the file
header format. I suggest changing num_entries to int64 in PG19 and
later, because it wasn't released yet, and pgss files generated by
another major version are considered to be incompatible anyway. See
the v1 patch attached.Agreed, I think we should make sure to do this once beta2 has shipped for the
next beta release.
Daniel, please feel free to become a reviewer and move it to RfC
if you like the patch.
I'm not sure how CFBot works. It's failing to apply the patch, and
I believe it's because I sent two alternative patches in one mail.
So I'm resending one patch this time, hoping it will work.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
On 31 Jul 2026, at 16:14, Karina Litskevich <litskevichkarina@gmail.com> wrote:
I've created a CF entry. I'm sorry it took so long. Life happened.
https://commitfest.postgresql.org/patch/7082/On Mon, Jul 13, 2026 at 11:36 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Simply changing num_entries to int64 leads to the change of the file
header format. I suggest changing num_entries to int64 in PG19 and
later, because it wasn't released yet, and pgss files generated by
another major version are considered to be incompatible anyway. See
the v1 patch attached.Agreed, I think we should make sure to do this once beta2 has shipped for the
next beta release.Daniel, please feel free to become a reviewer and move it to RfC
if you like the patch.
I'll have another look tomorrow, thanks for the reminder.
I'm not sure how CFBot works. It's failing to apply the patch, and
I believe it's because I sent two alternative patches in one mail.
So I'm resending one patch this time, hoping it will work.
The CFBot will try to apply all the patches in the mail as a patchset, so your
suspicion was correct. It's green in the CFBot now.
--
Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
On 31 Jul 2026, at 16:14, Karina Litskevich <litskevichkarina@gmail.com> wrote:
I'm not sure how CFBot works. It's failing to apply the patch, and
I believe it's because I sent two alternative patches in one mail.
So I'm resending one patch this time, hoping it will work.
The CFBot will try to apply all the patches in the mail as a patchset, so your
suspicion was correct. It's green in the CFBot now.
For future reference, you can prevent the bot from applying
"alternative" patches by naming your attachments properly.
A common choice is to append ".nocfbot" to the name of any
attachment you want the bot to ignore. More info here:
https://wiki.postgresql.org/wiki/Cfbot#Which_attachments_are_considered_to_be_patches?
regards, tom lane
I had a look at the patch and came up with the attached v2. I've separated the
pgver change into its own patch as it's unrelated from the hash_get_num_entries
change. For 0001 I also think we should cast usages of pgss_max when compated
to hash_get_num_entries to make this complete, to clarify the code.
--
Daniel Gustafsson
Attachments:
t253066_7v2-0002-Use-correct-data-type-for-version-identifier.patchapplication/octet-stream; name=v2-0002-Use-correct-data-type-for-version-identifier.patch; x-unix-mode=0644Download+1-2
v2-0001-Use-int64-for-number-of-entries-in-pg_stat_statem.patchapplication/octet-stream; name=v2-0001-Use-int64-for-number-of-entries-in-pg_stat_statem.patch; x-unix-mode=0644Download+7-9
On Fri, Jul 31, 2026 at 11:07 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
For future reference, you can prevent the bot from applying
"alternative" patches by naming your attachments properly.
A common choice is to append ".nocfbot" to the name of any
attachment you want the bot to ignore. More info here:https://wiki.postgresql.org/wiki/Cfbot#Which_attachments_are_considered_to_be_patches?
Thank you! This is very helpful.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
On Tue, Aug 4, 2026 at 12:07 PM Daniel Gustafsson <daniel@yesql.se> wrote:
I had a look at the patch and came up with the attached v2. I've separated the
pgver change into its own patch as it's unrelated from the hash_get_num_entries
change. For 0001 I also think we should cast usages of pgss_max when compated
to hash_get_num_entries to make this complete, to clarify the code.
I don't mind the changes.
Thank you for your attention to the patch!
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/