On Tue, Jan 21, 2014 at 7:47 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
>>> After going through commit 65d6e4c (introducing ALTER SYSTEM SET), I
>>> noticed a couple of typo mistakes as well as (I think) a weird way of
>>> using the temporary auto-configuration name postgresql.auto.conf.temp
>>> in two different places, resulting in the patch attached.
>>
>> .tmp suffix is used at couple other places in code as well.
>> snapmgr.c
>> XactExportFilePath(pathtmp, topXid, list_length(exportedSnapshots), ".tmp");
>> receivelog.c
>> snprintf(tmppath, MAXPGPATH, "%s.tmp", path);
>>
>> Although similar suffix is used at other places, but still I think it might be
>> better to define for current case as here prefix (postgresql.auto.conf) is also
>> fixed and chances of getting it changed are less. However even if we don't
>> do, it might be okay as we are using such suffixes at other places as well.
> In the case of snapmgr.c and receivelog.c, the operations are kept
> local, so it does not matter much if this way of naming is done as all
> the operations for a temporary file are kept within the same, isolated
> function. However, the case of postgresql.auto.conf.temp is different:
> this temporary file name is used as well for a check in basebackup.c,
> so I imagine that it would be better to centralize this information in
> a single place. Now this name is only used in two places, but who
> knows if some additional checks here are there won't be needed for
> this temp file...
> postgresql.auto.conf.temp is also located at the root of PGDATA, so it
> might be surprising for the user to find it even if there are few
> chances that it can happen.
I don't think there's any real reason to defined
PG_AUTOCONF_FILENAME_TEMP. pg_stat_statements just writes
PGSS_DUMP_FILE ".tmp" and that hasn't been a problem that I know of.
I do wonder why ALTER SYSTEM SET is spelling the suffix "temp" instead
of "tmp".
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
