Re: Remove traces of long in dynahash.c
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Chao Li <li.evan.chao@gmail.com>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-09-01T03:25:45Z
Lists: pgsql-hackers
Attachments
- v2-0001-Clean-up-my_log2-in-dynahash.c-adding-equivalents.patch (text/x-diff) patch v2-0001
On Wed, Aug 27, 2025 at 10:00:16AM +0200, Peter Eisentraut wrote:
> That seems highly confusing. What is the meaning of the "32" then?
>
> If you need 64-bit behavior, use the variant with "64" in the name.
static int
next_pow2_int(int64 num)
{
if (num > INT_MAX / 2)
num = INT_MAX / 2;
return 1 << my_log2(num);
}
The pain point for me is the assumption of this routine on HEAD and
older branches, leading to a more protective overflow pattern for the
number of partitions calculated. I don't see an elegant way to keep
the same calculations for the "next power" routines while making the
int32 flavor more compliant with the fact that it may have a int64
argument (long previously), because it would mean that we would
underestimate the number returned here each time "num" is higher than
(INT_MAX / 2). That's quite dangerous when applied to dynahash.c,
which is a layer that extensions like. That would lead to doubling
the number of "next power" routines in pg_bitutils.h, which is not
cool in the long-term because it would facilitate incorrect uses.
So, taking a step back, I don't know what would be a good fit for
these duplicates of the "next power" routines upper-bounded on input
when attached to pg_bitutils.h. However, I do see that we can get rid
of pg_log2() and dynahash.h with a consistent interface in
pg_bitutils.h, by reducing my proposal to the introduction of
pg_ceil_log2_32_bound() and pg_ceil_log2_64_bound().
At the end, next_pow2_int64() and next_pow2_int() are a lesser deal to
me, being static to dynahash.c. With that in mind, I am finishing
with the attached. Less ambitious, still it's a nice cleanup IMO.
What do you think?
--
Michael
Commits
-
Remove dynahash.h
- e6da68a6e1d6 19 (unreleased) landed
-
Replace callers of dynahash.h's my_log() by equivalent in pg_bitutils.h
- b1187266e077 19 (unreleased) landed
-
Change dynahash.c and hsearch.h to use int64 instead of long
- 13b935cd5217 19 (unreleased) landed