Обсуждение: RESET SESSION v2

New commands:

 CLOSE ALL                      -- close all cursors
 DEALLOCATE ALL                 -- close all prepared stmts
 RESET PLANS                    -- drop all plans
 RESET TEMP | TEMPORARY         -- drop all temp tables

 RESET SESSION                  -- drop/close/free everything

So in the end RESET SESSION is eqivalent to following commands:

 SET SESSION AUTHORIZATION DEFAULT;
 RESET ALL;
 DEALLOCATE ALL;
 CLOSE ALL;
 UNLISTEN *;
 RESET PLANS;
 RESET TEMP;

Changes in v2:

* RESET TEMPS -> RESET TEMP | TEMPORARY
* RESET SESSION does not ABORT anymore, instead fails if in transaction.
* DEALLOCATE ALL, CLOSE ALL and RESET SESSION change CommandComplete string
to "DEALLOCATE ALL", "CLOSE CURSOR ALL" and "RESET SESSION" respectively.
* Regression tests.
* Some docs.
* The ParamStatuses for changed options are already sent by ResetAllOptions(),
so this already works.

Questions:

* DEALLOCATE PREPARE ALL gives bison conflicts.  Is that even needed?

* Are the CommandComplete changes needed?  As there is possible to
hide DEALLOCATE ALL inside function?  OTOH, I like the idea
of more descriptive CommandComplete string.  I'd like it to
include even actual item name for ordinary DECLARE/CLOSE,
PREPARE/DEALLOCATE and SET/RESET in the future.

* ResetPlanCache() is implemented as PlanCacheCallback((Datum)0, InvalidOid);
That seems to leave plans for utility commands untouched.  Is it problem?
Should it walk plan list itself?


--
marko

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:
> New commands:
>
>  CLOSE ALL                      -- close all cursors
>  DEALLOCATE ALL                 -- close all prepared stmts
>  RESET PLANS                    -- drop all plans
>  RESET TEMP | TEMPORARY         -- drop all temp tables
>
>  RESET SESSION                  -- drop/close/free everything
>
> So in the end RESET SESSION is eqivalent to following commands:
>
>  SET SESSION AUTHORIZATION DEFAULT;
>  RESET ALL;
>  DEALLOCATE ALL;
>  CLOSE ALL;
>  UNLISTEN *;
>  RESET PLANS;
>  RESET TEMP;
>
> Changes in v2:
>
> * RESET TEMPS -> RESET TEMP | TEMPORARY
> * RESET SESSION does not ABORT anymore, instead fails if in transaction.
> * DEALLOCATE ALL, CLOSE ALL and RESET SESSION change CommandComplete string
> to "DEALLOCATE ALL", "CLOSE CURSOR ALL" and "RESET SESSION" respectively.
> * Regression tests.
> * Some docs.
> * The ParamStatuses for changed options are already sent by ResetAllOptions(),
> so this already works.
>
> Questions:
>
> * DEALLOCATE PREPARE ALL gives bison conflicts.  Is that even needed?
>
> * Are the CommandComplete changes needed?  As there is possible to
> hide DEALLOCATE ALL inside function?  OTOH, I like the idea
> of more descriptive CommandComplete string.  I'd like it to
> include even actual item name for ordinary DECLARE/CLOSE,
> PREPARE/DEALLOCATE and SET/RESET in the future.
>
> * ResetPlanCache() is implemented as PlanCacheCallback((Datum)0, InvalidOid);
> That seems to leave plans for utility commands untouched.  Is it problem?
> Should it walk plan list itself?
>
>
> --
> marko

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

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

On Tue, 2007-04-03 at 10:15 +0300, Marko Kreen wrote:
> New commands:
>
>  CLOSE ALL                      -- close all cursors
>  DEALLOCATE ALL                 -- close all prepared stmts
>  RESET PLANS                    -- drop all plans
>  RESET TEMP | TEMPORARY         -- drop all temp tables
>
>  RESET SESSION                  -- drop/close/free everything

+ void
+ ResetTempTableNamespace(void)
+ {
+     char        namespaceName[NAMEDATALEN];
+     Oid            namespaceId;
+
+     /* If not allowed to create, no point proceeding */
+     if (pg_database_aclcheck(MyDatabaseId, GetUserId(),
+                 ACL_CREATE_TEMP) != ACLCHECK_OK)
+         return;

ISTM this is buggy: if the user's TEMPORARY privilege is revoked between
the time that they create a temporary table and when they execute RESET
SESSION, the temporary table won't be cleaned up.

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

guc.c is missing some #includes.

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

> * 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).

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

-Neil

Neil Conway <neilc@samurai.com> writes:
>> * 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.

Utility commands haven't got plans.

            regards, tom lane