Re: Use compiler intrinsics for bit ops in hash

On Thu, Feb 27, 2020 at 1:56 PM David Fetter <david(at)fetter(dot)org> wrote:
>
> [v6 patch set]

Here I'm only looking at 0001. It needs rebasing, but it's trivial to
see what it does. I noticed in some places, you've replaced "long"
with uint64, but many are int64. I started making a list, but it got
too long, and I had to stop and ask: Is there a reason to change from
signed to unsigned for any of the ones that aren't directly related to
hashing code? Is there some larger pattern I'm missing?

-static long gistBuffersGetFreeBlock(GISTBuildBuffers *gfbb);
-static void gistBuffersReleaseBlock(GISTBuildBuffers *gfbb, long blocknum);
+static uint64 gistBuffersGetFreeBlock(GISTBuildBuffers *gfbb);
+static void gistBuffersReleaseBlock(GISTBuildBuffers *gfbb, uint64 blocknum);

I believe these should actually use BlockNumber, if these refer to
relation blocks as opposed to temp file blocks (I haven't read the
code).

-exec_execute_message(const char *portal_name, long max_rows)
+exec_execute_message(const char *portal_name, uint64 max_rows)

The only call site of this function uses an int32, which gets its
value from pq_getmsgint, which returns uint32.

--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services