[HACKERS] PostgresNode::append_conf considered dangerous

Поиск
Список
Период
Сортировка
От Tom Lane
Тема [HACKERS] PostgresNode::append_conf considered dangerous
Дата
Msg-id 19751.1492892376@sss.pgh.pa.us
обсуждение исходный текст
Ответы Re: [HACKERS] PostgresNode::append_conf considered dangerous
Список pgsql-hackers
Okay, so append_conf is perfectly straightforward about what it does:
A newline is NOT automatically appended to the string.

However, I count at least eight places that are being lazy, like this:
 # Change a setting and restart $node->append_conf('postgresql.conf', 'hot_standby = on'); $node->restart();

The worst part is that that works, as long as you don't do it more
than once in a script.  When you do it twice, though, you have a
syntactically invalid config file.  Thus for example this bit in
src/test/modules/commit_ts/t/003_standby_2.pl isn't doing anywhere
near what it thinks it's doing:

$master->append_conf('postgresql.conf', 'track_commit_timestamp = on');
$master->restart;
$master->append_conf('postgresql.conf', 'track_commit_timestamp = off');
$master->restart;

(I found that after deciding that PostgresNode::restart should complain
if pg_ctl failed.  The node is in fact not running after this, but the
existing test script fails to notice that.)

Now we could run around and make all these places do it the "right way",
like this other call:

$node_master->append_conf('postgresql.conf', qq(
track_commit_timestamp = on
));

But (1) that's pretty darn ugly code, and (2) this fix does not prevent
future mistakes of the same ilk, which are obviously easy to make.

I think instead we should change append_conf to append a newline, and
simplify the callers that could thereby be simplified.  I don't see
any callers that would be broken by that; and we do not have any config
files in which extra blank lines are problematic.
        regards, tom lane



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Noah Misch
Дата:
Сообщение: Re: [HACKERS] pg_dump emits ALTER TABLE ONLY partitioned_table
Следующее
От: Tom Lane
Дата:
Сообщение: Re: [HACKERS] A note about debugging TAP failures