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. +
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 |