Re: Re: patch review : Add ability to constrain backend temporary file space

Mark Kirkwood <mark(dot)kirkwood(at)catalyst(dot)net(dot)nz> writes:
> This version moves the check *before* we write the new buffer, so
> should take care of issues about really large write buffers, plugins
> etc.

This logic seems pretty obviously wrong:

+ if (temp_file_limit >= 0 && VfdCache[file].fdstate & FD_TEMPORARY)
+ {
+ if (VfdCache[file].seekPos + amount >= VfdCache[file].fileSize)
+ {
+ increaseSize = true;
+ if ((temporary_files_size + (double)amount) / 1024.0 > (double)temp_file_limit)
+ ereport(ERROR,
+ (errcode(ERRCODE_QUERY_CANCELED),
+ errmsg("aborting due to exceeding temp file limit")));
+ }
+ }

It's supposing that the write length ("amount") is exactly the amount by
which the file length will grow, which would only be true if seekPos is
exactly equal to fileSize before the write. I think that would often be
the case in our usage patterns, but surely code at this low level should
not be assuming that.

BTW, the definition of the GUC seems to constrain the total temp file
size to be no more than 2GB on 32-bit machines ... do we really want
it to act like that?

regards, tom lane