Re: Patch pg_is_in_backup()

From: Gabriele Bartolini <gabriele(dot)bartolini(at)2ndQuadrant(dot)it>
To: Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Magnus Hagander <magnus(at)hagander(dot)net>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Bernd Helmle <mailings(at)oopsware(dot)de>, Euler Taveira de Oliveira <euler(at)timbira(dot)com>, Marti Raudsepp <marti(at)juffo(dot)org>
Subject: Re: Patch pg_is_in_backup()
Date: 2012-05-02 17:53:21
Message-ID: 4FA17491.4040406@2ndQuadrant.it
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Gilles,

Sorry for the delay.

Il 03/04/12 14:21, Gilles Darold ha scritto:
> +1, this is also my point of view.

I have looked at the patch that contains both pg_is_in_backup() and
pg_backup_start_time().

From a functional point of view it looks fine to me. I was thinking
of adding the BackupInProgress() at the beginning of
pg_backup_start_time(), but the AllocateFile() function already make
sure the file exists.

I have performed some basic testing of both functions and tried to
inject invalid characters in the start time field of the backup_label
file and it is handled (with an exception) by the server. Cool.

I spotted though some formatting issues, in particular indentation
and multi-line comments. Some rows are longer than 80 chars.

Please resubmit with these cosmetic changes and it is fine with me.
Thank you.

Cheers,
Gabriele

--
Gabriele Bartolini - 2ndQuadrant Italia
PostgreSQL Training, Services and Support
gabriele(dot)bartolini(at)2ndQuadrant(dot)it | www.2ndQuadrant.it

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2012-05-02 18:04:59 Re: plpython crash (PG 92)
Previous Message Magnus Hagander 2012-05-02 17:42:47 Re: Have we out-grown Flex?