Re: RESET SESSION v2 - Mailing list pgsql-patches

From Neil Conway
Subject Re: RESET SESSION v2
Date
Msg-id 1175978273.4023.15.camel@localhost.localdomain
Whole thread Raw
In response to RESET SESSION v2  ("Marko Kreen" <markokr@gmail.com>)
Responses Re: RESET SESSION v2  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
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


Attachment

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: LIMIT/SORT optimization
Next
From: "Simon Riggs"
Date:
Subject: Re: Heap page diagnostic/test functions (v2)