I noticed that commit 30bb26b5 ("Allow usage of huge
maintenance_work_mem for GIN build") made the following modification:

--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -903,7 +903,7 @@ typedef struct GinEntryAccumulator
 typedef struct
 {
    GinState   *ginstate;
-   long        allocatedMemory;
+   Size        allocatedMemory;
    GinEntryAccumulator *entryallocator;
    uint32      eas_used;
    RBTree     *tree;

Are you sure this is safe, Teodor? I don't have time to study the
patch in detail, but offhand I think that it might have been better to
make allocatedMemory of type int64, just like the tuplesort.c memory
accounting variables are post-MaxAllocHuge. It's not obvious to me
that this variable isn't allowed to occasionally become negative, just
like in tuplesort.c. It looks like that *might* be true -- ginbulk.c
may let allocatedMemory go negative for a period, which would now be
broken.

If you did make this exact error, you would not be the first. If it
isn't actually broken, perhaps you should still make this change,
simply on general principle. I'd like to hear other opinions on that,
though.

Thanks
-- 
Peter Geoghegan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to