Re: Proposed patch to remove USERLIMIT

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I would be glad to see USERLIMIT gone, even though I wrote it. However,
> > I feel we are removing user's ability of non-super users to turn logging
> > on and off easily without really having thought through a mechanism to
> > give them that.
>
> I think that is a very weak argument. Remember that the discussion

Well, I disagree or I wouldn't have made the argument, would I?

> started because setting these variables via ALTER USER fails to work.

It fails to work if logging was already on and someone wants to turn it
off via ALTER USER, and that matches the expected behavior.

> Why is that of less concern than the fact that unprivileged users won't
> be able to increase the log level by themselves? I think it's pretty

Because it is working as planned, meaning normal users can't turn off
logging.

> debatable whether the current behavior is a feature at all, rather than
> a security hole.
>
> > I just don't see people creating secuirty definer
> > functions easily to do this
>
> create or replace function enable_logging(bool) returns void as $$
> begin
> if $1 then
> set log_statement = 'all';
> else
> set log_statement = 'ddl';
> end if;
> return;
> end$$ language plpgsql security definer;
>
> revoke execute on function enable_logging(bool) from public;
> grant execute on function enable_logging(bool) to joe_user;
>
> (My, that was hard ...)

Well, you have just made the system less secure by creating that
function. Why didn't you create a function that has the existing
behavior of only allowing users to increase the log level? Because it
is harder to do, and certainly not something trivial you would write
during a debugging session.

> You seem to be supposing that anyone who needs this will be needing a
> bug-for-bug equivalent to the existing USERLIMIT behavior. I don't

What bug?

> think so. I think that in the field, people are going to have at most
> a couple of logging configurations they want to allow users to select,
> and they will use code not significantly more complex than the above.

Can you show me the function?

> Arguably, this approach is better than directly calling the SET commands
> anyway, because it insulates the application code from the changing
> details of the specific GUC variables. Need I remind you that any app
> that directly issues "SET log_statement" is going to be broken by 8.0?
> In 7.4 this variable took "on" and "off", now it does not. The argument
> of preserving backwards-compatible behavior looks a bit funny beside
> that reality.

I realize that but I think we need to give people the existing
functionality out of the box, or at least not make such a change so
close to final without time to discuss. You should ask on general where
we usually do such polls of feature changes? People are already
complaining we don't make logging easy enough (pg_stat_activity) and now
we are going to make it harder still? Doesn't make sense to me, and I
don't see your change as a bug fix.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073