Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Tom Lane (tgl@sss.pgh.pa.us) wrote:
> >> I haven't been paying too close attention to this thread, but isn't
> >> that exactly what it does now and always has?  guc.c, at least, certainly
> >> is going to interpret duplicate entries that way.
> 
> > The issue isn't with reading them and interpreting them, it's what
> > happens when you run ALTER SYSTEM and it goes and modifies the file.
> > Presently, it basically operates on the first entry it finds when
> > performing a SET or a RESET.
> 
> Ah, got it.  So it seems like the correct behavior might be for
> ALTER SYSTEM to
> (a) run through the whole file and remove any conflicting lines;
> (b) append new setting at the end.

Sure- and every other tool that modifies that file should know that
*that* is how you do it, and therefore, if everyone is doing it right,
you don't ever end up with duplicates in the file.  If you do, someone's
doing it wrong, and we should issue a warning.

That's more-or-less the conclusion on the other thread, as I understood
it.

> If you had some fancy setup with comments associated with entries,
> you might not be pleased with that.  But I can't muster a lot of
> sympathy for tools putting comments in postgresql.auto.conf anyway;
> it's not intended to be a human-readable file.

If we were to *keep* the duplicates, then I could see value in including
information about prior configuration entries (I mean, that's what a lot
of external tools do with our postgresql.conf file- put it into git or
some other configuration management tool...).  If we aren't keeping the
dups, then I agree that there doesn't seem much point.

> If anybody does complain, my first reaction would be to make ALTER
> SYSTEM strip all comment lines too.

Uh, I believe it already does?

Thanks,

Stephen