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/
From c2b6bd266c5f706c23317c25923ab9cbe7f8ef10 Mon Sep 17 00:00:00 2001 From: Karina Litskevich <[email protected]> Date: Mon, 13 Jul 2026 15:44:07 +0300 Subject: [PATCH v1] Use int64 for number of entries in pg_stat_statements In 13b935cd hash_get_num_entries was changed to return int64. Clean up one of its usages in pg_stat_statements that was missed. Don't bother to change PGSS_FILE_HEADER, since the file header format is only to be changed in not yet released major versions. Also fix the definition of pgver according to its usage while we are here. --- contrib/pg_stat_statements/pg_stat_statements.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 92315627916..6c6c09e9968 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -533,9 +533,9 @@ pgss_shmem_init(void *arg) FILE *file = NULL; FILE *qfile = NULL; uint32 header; - int32 num; - int32 pgver; - int32 i; + int64 num; + uint32 pgver; + int64 i; int buffer_size; char *buffer = NULL; @@ -612,7 +612,7 @@ pgss_shmem_init(void *arg) if (fread(&header, sizeof(uint32), 1, file) != 1 || fread(&pgver, sizeof(uint32), 1, file) != 1 || - fread(&num, sizeof(int32), 1, file) != 1) + fread(&num, sizeof(int64), 1, file) != 1) goto read_error; if (header != PGSS_FILE_HEADER || @@ -737,7 +737,7 @@ pgss_shmem_shutdown(int code, Datum arg) char *qbuffer = NULL; Size qbuffer_size = 0; HASH_SEQ_STATUS hash_seq; - int32 num_entries; + int64 num_entries; pgssEntry *entry; /* Don't try to dump during a crash. */ @@ -761,7 +761,7 @@ pgss_shmem_shutdown(int code, Datum arg) if (fwrite(&PGSS_PG_MAJOR_VERSION, sizeof(uint32), 1, file) != 1) goto error; num_entries = hash_get_num_entries(pgss_hash); - if (fwrite(&num_entries, sizeof(int32), 1, file) != 1) + if (fwrite(&num_entries, sizeof(int64), 1, file) != 1) goto error; qbuffer = qtext_load_file(&qbuffer_size); -- 2.51.0
From 749d05cbbcb6f3e67648e36a048acdb86cbdeb25 Mon Sep 17 00:00:00 2001 From: Karina Litskevich <[email protected]> Date: Mon, 13 Jul 2026 16:39:04 +0300 Subject: [PATCH] pg_stat_statements: explicitly convert number of entries to int32 hash_get_num_entries returns long before PG19 (int64 since 13b935cd). Technically assigning it to int32 is wrong. In practice number of entries should not exceed pgss_max. While pgss_max fits int32 we should be okay. Add the corresponding check for safety's sake. We do this instead of changing num_entries type to avoid changing the file header format. Also fix the definition of pgver according to its usage while we are here. --- contrib/pg_stat_statements/pg_stat_statements.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 92315627916..6f4d172d9ab 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -534,7 +534,7 @@ pgss_shmem_init(void *arg) FILE *qfile = NULL; uint32 header; int32 num; - int32 pgver; + uint32 pgver; int32 i; int buffer_size; char *buffer = NULL; @@ -760,7 +760,13 @@ pgss_shmem_shutdown(int code, Datum arg) goto error; if (fwrite(&PGSS_PG_MAJOR_VERSION, sizeof(uint32), 1, file) != 1) goto error; - num_entries = hash_get_num_entries(pgss_hash); + + /* + * Number of elements in pgss_hash should not exceed pgss_max. And we want + * it to fit into int32. + */ + StaticAssertStmt(sizeof(pgss_max) <= sizeof(int32), "pgss_max should fit into int32"); + num_entries = (int32) hash_get_num_entries(pgss_hash); if (fwrite(&num_entries, sizeof(int32), 1, file) != 1) goto error; -- 2.51.0
