Re: Add support for logging the current role

* Itagaki Takahiro (itagaki(dot)takahiro(at)gmail(dot)com) wrote:
> I agree that it's logically good design, but we could not accept it
> as long as it breaks tools in the real world...

If it does, I think it's pretty clear that those tools are themselves
broken..

> Will it break "SHOW ALL" and "SELECT * FROM pg_settings" output?
> I'm worried that they are not designed to display such a long value.

It certainly won't break those commands in PostgreSQL. If tools made
assumptions about how long a string could be that don't match PG's
understand, those tools are broken. This also isn't the only string and
such tools could be broken by, eg, setting log_prefix to a very long
value (entirely possible to do..).

> >> I think it depends default log filename, that contains %S (seconds)
> >> suffix. We can remove %S from log_filename; if we use a log per-day,
> >> those log might contain different columns even after restart. If we
> >> cannot avoid zigged csv fields completely, SIGHUP seems reasonable for it.
> >
> > This problem is bigger than SIGHUP, but at least with a restart
> > required, the default will work properly.  The default configuration
> > wouldn't work w/ a change to the log line and a SIGHUP done.
>
> "Only works with the default settings" is just wrong design.

It's also not anywhere close to what I said. If you have a suggestion
about how to fix it that's reasonable, please suggest it. Suggesting
that "because we could, given a complicated enough configuration,
create jagged files anyway, we should encourage it to happen by allowing
the change on SIGHUP" isn't a solution to the problem and will just
create more problems. It will certainly work just fine with more than
just the default settings, but, yes, there are some settings which would
cause PG to create jagged CSV files.

If we keep it as requiring restart and then automatically move or
truncate log files which are in the way on that restart, or decide to
not start at all, we could make it so PG doesn't create a jagged CSV
file. I don't particularly care for any of those options. At least
one of those would be a solution, however none of those include
allowing it on SIGHUP, which is what you're advocating.

> If we cannot provide a perfect solution, we should allow users to
> control everything as they like. I still think PGC_SIGHUP is the
> best mode for the parameter, with a note of caution in the docs.

Doing it on a SIGHUP would be *guaranteed* to create jagged CSV files
which then couldn't be loaded into PG. I don't agree with this and the
impression I got from Tom and Andrew was that they agreed to not do it
on SIGHUP. It's unfortuante that we seem to be at an impasse here, but
of everyone voicing opinions on it so far, it would appear that you're
in the minority.

Thanks,

Stephen