Skip site navigation (1) Skip section navigation (2)

Re: RESET SESSION v2

From: Neil Conway <neilc(at)samurai(dot)com>
To: Marko Kreen <markokr(at)gmail(dot)com>
Cc: PGSQL-Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: RESET SESSION v2
Date: 2007-04-07 20:37:53
Message-ID: 1175978273.4023.15.camel@localhost.localdomain (view raw or flat)
Thread:
Lists: 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: bison_hack-1.patch
Description: text/x-patch (483 bytes)

In response to

Responses

pgsql-patches by date

Next:From: Simon RiggsDate: 2007-04-07 22:06:41
Subject: Re: Heap page diagnostic/test functions (v2)
Previous:From: Tom LaneDate: 2007-04-07 18:11:48
Subject: Re: LIMIT/SORT optimization

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group