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
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
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
[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
