Re: RESET SESSION v3

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Marko Kreen <markokr(at)gmail(dot)com>
Cc: Neil Conway <neilc(at)samurai(dot)com>, PGSQL-Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: RESET SESSION v3
Date: 2007-04-10 01:05:06
Message-ID: 200704100105.l3A156H16182@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches


Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Marko Kreen wrote:
> Changes in v3:
>
> * DEALLOCATE PREPARE ALL
> * RESET PLANS wont check for ACL_CREATE_TEMP anymore.
> * Add prepare.h and portal.h to guc.c.
>
>
> On 4/7/07, Neil Conway <neilc(at)samurai(dot)com> wrote:
> > > * RESET SESSION does not ABORT anymore, instead fails if in transaction.
>
> > I think it's quite bizarre to have RESET SESSION fail if used in a
> > transaction, but to allow an equivalent sequence of commands to be
> > executed by hand inside a transaction.
>
> I think implicit ABORT would annoy various tools that
> partially parse user sql and expect to know what transaction
> state currently is. For them a new tranaction control statement
> would be nuisance.
>
> > guc.c is missing some #includes.
>
> For some reason gcc4.0 did not show any warnings by default.
>
> > > * DEALLOCATE PREPARE ALL gives bison conflicts. Is that even needed?
> >
> > Seems best to have it, for the sake of consistency. The shift/reduce
> > conflict is easy to workaround, provided you're content to duplicate the
> > body of the DEALLOCATE ALL rule -- e.g. see the attached incremental
> > diff.
>
> Thanks, included.
>
> > > * Are the CommandComplete changes needed?
> >
> > Seems warranted to me. BTW, why is CLOSE's command tag "CLOSE CURSOR",
> > not "CLOSE"? That seems needlessly verbose, and inconsistent with other
> > commands (e.g. DEALLOCATE).
>
> Because the regular tag is "CLOSE CURSOR". I did not
> want to break any expectations. But yes, the inconsistency
> is weird.
>
> > > * ResetPlanCache() is implemented as PlanCacheCallback((Datum)0,
> > > InvalidOid); That seems to leave plans for utility commands untouched.
> > > Is it problem?
> >
> > Yes, I'd think you'd also want to cleanup plans for utility commands.
>
> Tom thought otherwise, so I kept the old way.
>
>
> --
> marko

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

In response to

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2007-04-10 01:05:50 Re: Minor recovery changes
Previous Message David Fetter 2007-04-10 00:18:46 Re: non-recursive WITH clause support