Re: setting per-database/role parameters checks them against wrong context

On Sat, Oct  6, 2012 at 02:20:53PM -0700, Selena Deckelmann wrote:
> On Mon, Oct 1, 2012 at 2:28 PM, Selena Deckelmann <selena@chesnok.com> wrote:
> > On Mon, Oct 1, 2012 at 1:37 PM, Selena Deckelmann <selena@chesnok.com> wrote:
> >> On Mon, Oct 1, 2012 at 1:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>> Selena Deckelmann <selena@chesnok.com> writes:
> >>>> The check_temp_buffers() problem seems like a regression and blocks us
> >>>> from upgrading to 9.2. The use case are functions that set
> >>>> temp_buffers and occasionally are called in a series from a parent
> >>>> session.  The work around is... a lot of work.
> >>>
> >>> Uh ... how is that a regression?  AFAIK it's been that way right along.
> >>
> >> We're running 9.0 - looks like it changed in 9.1, last revision to the
> >> relevant line was 6/2011. The group decided not to upgrade to 9.1 last
> >> year, but was going to just go directly to 9.2 in the next few weeks.
> >
> > And, I was basing the regression comment on the documentation for
> > temp_buffers: "The setting can be changed within individual sessions,
> > but only before the first use of temporary tables within the session;
> > subsequent attempts to change the value will have no effect on that
> > session." This statement has been there since at least 8.1.
> >
> > A warning, and then not failing seems more appropriate than an error,
> > given the documented behavior.
> 
> I tried out a few things, and then realized that perhaps just adding
> PGC_S_SESSION to the list of sources that are at elevel WARNING
> partially fixes this.
> 
> This doesn't fix the issue with log_statement_stats, but it makes the
> behavior of temp_buffers  the documented behavior (no longer errors
> out in a transaction), while still warning the user.

> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> index 6b202e0..0677059 100644
> --- a/src/backend/utils/misc/guc.c
> +++ b/src/backend/utils/misc/guc.c
> @@ -5150,7 +5150,7 @@ set_config_option(const char *name, const char *value,
>              elevel = IsUnderPostmaster ? DEBUG3 : LOG;
>          }
>          else if (source == PGC_S_DATABASE || source == PGC_S_USER ||
> -                 source == PGC_S_DATABASE_USER)
> +                 source == PGC_S_DATABASE_USER || source == PG_S_SESSION)
>              elevel = WARNING;
>          else
>              elevel = ERROR;

Is there any opinion on whether we need this patch?  It basically allows
SET from a session to issue a warning rather than an error.  This is
certainly a much larger change than just fixing temp_buffers.  The user
complaint was that functions were setting temp_buffers and getting
errors because temp table has already been used:
       The setting can be changed within individual sessions, but only       before the first use of temporary tables
withinthe session;       subsequent attempts to change the value will have no effect on       that session.
 

The full thread is here:
http://www.postgresql.org/message-id/1348802984.3584.22.camel@vanquo.pezone.net

Seems this changed in PG 9.1.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +