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

On 02/06/11 18:34, Jaime Casanova wrote:
> On Wed, Jun 1, 2011 at 6:35 PM, Mark Kirkwood
> <mark(dot)kirkwood(at)catalyst(dot)net(dot)nz> wrote:
>>
>> I've created a new patch (attached)
> Hi Mark,
>
> A few comments:
>
> - why only superusers can set this? if this is a per-backend setting,
> i don't see the problem in allowing normal users to restrict their own
> queries
>

Yeah, that's a good point, I was thinking that as a resource control
parameter it might be good to prevent casual users increasing their
limit. However the most likely use case would be ad-hoc query tools that
don't have the ability to do SET anyway. Hmm - what do you think?

> - why the calculations are done as double?
> + if (temporary_files_size / 1024.0> (double)work_disk)
>
>
>

I originally coded this with the idea that the guc would (or could) be a
double - to allow for seriously big limits in data warehousing
situations etc. But subsequent discussion led to that being discarded.
However work_disk can go to INT_MAX * 1024 bytes so I need to make sure
that whatever datatype I use can handle that - double seemed sufficient.

> - the patch adds this to serial_schedule but no test has been added...
>
> diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
> index bb654f9..325cb3d 100644
> --- a/src/test/regress/serial_schedule
> +++ b/src/test/regress/serial_schedule
> @@ -127,3 +127,4 @@ test: largeobject
> test: with
> test: xml
> test: stats
> +test: resource
>

Bugger - I think I forgot to 'git add' em before doing the diff.

I can sense a v5 coming on.