On Sun, 17 Aug 2025 at 18:54, Michael Paquier <michael@paquier.xyz> wrote:
> -#ifdef HASH_STATISTICS
> -static long hash_accesses,
> - hash_collisions,
> - hash_expansions;
> -#endif
>
> These global counters are as old as d31084e9d111. Removing them
> should not be a problem.
Ancient!
> Side thing.. I'm wondering what prevents us from wiping out entirely
> the use of long in this file. long is 8 bytes everywhere, except on
> WIN32 where it's 4 bytes (as you say), which is a bad practice as we
> have been bitten by overflows because of this dependency in the patch.
> Not related to this patch, still seems worth cleaning up while looking
> at this code. There are a few more things like HASHCTL, of course..
No objections here. I think we're generally chipping away at that
problem anyway. At least, I did some of that recently, and I recall
Tom adjusted things to allow windows to have > 2GB work_mem, which was
a restriction imposed by 32-bit longs.
One last thing, in order to inform people of breakages sooner than a
post-commit report from the buildfarm, I wondered is if we should do:
-#ifdef HASH_DEBUG
+#if defined(HASH_DEBUG) || defined(USE_ASSERT_CHECKING)
The HASH_DEBUG does not add any extra fields, so the overhead only
amounts to the elog(DEBUG4) line. HASH_STATISTICS adds extra fields
and counter incrementing, so I don't propose the same treatment for
that.
David
