Stephen Frost <sfr...@snowman.net> writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> Moreover, if you don't check it then the appname recorded
>> by log_connections would not match appearances for the same session
>> later in the log, which puts the entire use-case for this patch into
>> question. So no, this concern must not be dismissed.
> If the call to check_application_name() fails then I had been expecting
> the connection to fail. If we continue to let the connection go on then
> we've already got an issue as someone might pass in an application name
> that isn't being set to the GUC and isn't being therefore used in the
> existing log_line_prefix where it should be.
No, check_application_name doesn't reject funny names, it just silently
modifies them in-place.
>> However ... I've not looked at the patch, but I thought the idea was to
>> allow assignment of that GUC to occur before the log_connections log entry
>> is emitted, so that it'd show up in the entry's log_line_prefix. Wouldn't
>> check_application_name happen automatically at that point?
> We log that message quite early and it certainly didn't look trivial to
> set up the GUC to be already in place at that point, so the plan was to
> simply spit out what gets passed in (as we were doing for "user", if I'm
> remembering that code correctly...).
Hm. Well, the code isn't exactly complicated, you could duplicate it.
Or maybe better refactor to allow it to be called from $wherever.
Looks like check_cluster_name, for one, could also share use of
an ascii-lobotomizing subroutine.
But having said that, I don't exactly see why you couldn't force it
with an ultimately-redundant SetConfigOption call to put the value
in place before the ereport happens. The GUC machinery is surely
functional before we do authorization.
regards, tom lane
