Re: [PATCHES] 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)
+ {
+ charnamespaceName[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
diff -u src/backend/parser/gram.y src/backend/parser/gram.y
--- src/backend/parser/gram.y 3 Apr 2007 07:09:31 -
+++ src/backend/parser/gram.y 7 Apr 2007 20:14:48 -
@@ -5596,6 +5596,12 @@
n-name = NULL;
$$ = (Node *) n;
}
+| DEALLOCATE PREPARE ALL
+ {
+ DeallocateStmt *n = makeNode(DeallocateStmt);
+ n-name = NULL;
+ $$ = (Node *) n;
+ }
;
/*
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] RESET SESSION v2
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] 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 reset_session_v2.diff Description: Binary data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] 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 [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
