From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Remove traces of long in dynahash.c |
Date: | 2025-09-01 03:25:45 |
Message-ID: | aLUSOQcbt3Ela1Lg@paquier.xyz |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Clean-up-my_log2-in-dynahash.c-adding-equivalents.patch | text/x-diff | 7.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | shveta malik | 2025-09-01 03:30:37 | Re: Improve pg_sync_replication_slots() to wait for primary to advance |
Previous Message | Michael Paquier | 2025-09-01 02:29:22 | Re: Resetting recovery target parameters in pg_createsubscriber |