RESET SESSION v2

Started by Marko Kreenover 19 years ago4 messagespatches
Jump to latest
#1Marko Kreen
markokr@gmail.com

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

Attachments:

reset_session_v2.diffapplication/octet-stream; name=reset_session_v2.diffDownload+452-40
#2Bruce Momjian
bruce@momjian.us
In reply to: Marko Kreen (#1)
Re: RESET SESSION v2

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

#3Neil Conway
neilc@samurai.com
In reply to: Marko Kreen (#1)
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

Attachments:

bison_hack-1.patchtext/x-patch; charset=us-ascii; name=bison_hack-1.patchDownload+6-0
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#3)
Re: RESET SESSION v2

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