Re: Remove traces of long in dynahash.c

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

In response to

Responses

Browse pgsql-hackers by date

  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