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
