On Fri, May 6, 2016 at 7:58 PM, Peter Geoghegan <p...@heroku.com> wrote: > 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.
I've added this to the open items list. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers