On Thu, Feb 27, 2020 at 1:56 PM David Fetter <da...@fetter.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
