--- 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.
It could not be negative - subtruction is doing only around repalloc call, in
all other places it only grows.
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.
It could be an int64 without any regressions or problems. I choose Size type
because variable allocatedMemory actually stores a size of piece of memory and
it's the same type as returned by GetMemoryChunkSpace()
AFAIK, size_t type (type Size is a just typedef alias) is a part of C99 and it
should be unsigned and large enough to store size of any chunk of avaliable RAM.
And it at least twice larger than allowed all memory-related GUC variables.
I don't see a reason to use int64 except, may be, general practice in pgsql. But
then why we keep Size type - let us just use int64.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
