Re: RESET SESSION v2

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