Re: [PATCHES] DEALLOCATE ALL
Alvaro Herrera wrote: Marko Kreen escribió: On 3/30/07, Alvaro Herrera [EMAIL PROTECTED] wrote: In any case it's not likely that there are going to be thousands of prepared statements, so is this really an issue? I think the issue is here that its very common thing to do, so open-coding it everywhere is waste, there should be some utility function for that. void hash_foreach(HTAB, void (*cb_func)(void *)); Extra points if you can implement a map() function for hashes ;-) (I think it's called mutator in our sources for other kind of stuff) I think it would be void *hash_map(HTAB, void *(*map_func) (void *)) Not sure what the return value would be though :-( (Maybe this is extra complication enough that it's not worth the hassle) hash_map and hash_foreach seem like overkill to me, looping with hash_seq_search and doing stuff is simple enough that it doesn't really require any additional shorthands. Besides, to use them you'd always have to have a separate function and often a struct to pass down to the function, so it's not really any shorter or simpler. What would be useful is a hash_seq_remove-function that removes the previous item returned by hash_seq_search without the overhead of recalculating the hash value. While reviewing ITAGAKI's dead space map patch I noticed that he also added a hash_truncate function that removes all entries in the hash table, but unlike hash_destroy leaves the hash table intact. His implementation also calls hash_search(REMOVE) in a loop, which isn't the most efficient way to do it, but clearly there's need for more ways to empty a hash table. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] DEALLOCATE ALL
Neil Conway escribió: As to the implementation, calling hash_remove() in a loop seems a pretty unfortunate way to clear a hash table -- adding a hash_reset() function to the dynahash API would be cleaner and faster. I wonder why hash_drop cannot be used? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] DEALLOCATE ALL
On 3/30/07, Alvaro Herrera [EMAIL PROTECTED] wrote: Neil Conway escribió: As to the implementation, calling hash_remove() in a loop seems a pretty unfortunate way to clear a hash table -- adding a hash_reset() function to the dynahash API would be cleaner and faster. I wonder why hash_drop cannot be used? hash_destroy()? Each element need separate destruction. -- marko ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] DEALLOCATE ALL
Marko Kreen escribió: On 3/30/07, Alvaro Herrera [EMAIL PROTECTED] wrote: Neil Conway escribió: As to the implementation, calling hash_remove() in a loop seems a pretty unfortunate way to clear a hash table -- adding a hash_reset() function to the dynahash API would be cleaner and faster. I wonder why hash_drop cannot be used? hash_destroy()? Each element need separate destruction. Hmm, so maybe something like hash_destroy_deep, like the List equivalent? If it's a simple pfree() for every element this would be simple enough. If this is the case, an even simpler idea would be to allocate the elements in the same MemoryContext as the hash itself (or in children thereof); then calling hash_destroy() would delete (reset?) the context and thus all elements are freed at once as well. If by destruction you mean something different than pfree, then maybe hash_remove in a loop is the best solution, the other idea being passing a function pointer to hash_destroy_deep to call on each element, which is probably too messy an API. In any case it's not likely that there are going to be thousands of prepared statements, so is this really an issue? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] DEALLOCATE ALL
On 3/30/07, Alvaro Herrera [EMAIL PROTECTED] wrote: If by destruction you mean something different than pfree, then maybe hash_remove in a loop is the best solution, the other idea being passing a function pointer to hash_destroy_deep to call on each element, which is probably too messy an API. Yes, callback function is needed, either in HASHCTL or as argument to deep_free(). In any case it's not likely that there are going to be thousands of prepared statements, so is this really an issue? I think the issue is here that its very common thing to do, so open-coding it everywhere is waste, there should be some utility function for that. void hash_foreach(HTAB, void (*cb_func)(void *)); -- marko ---(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] DEALLOCATE ALL
Marko Kreen escribió: On 3/30/07, Alvaro Herrera [EMAIL PROTECTED] wrote: In any case it's not likely that there are going to be thousands of prepared statements, so is this really an issue? I think the issue is here that its very common thing to do, so open-coding it everywhere is waste, there should be some utility function for that. void hash_foreach(HTAB, void (*cb_func)(void *)); Extra points if you can implement a map() function for hashes ;-) (I think it's called mutator in our sources for other kind of stuff) I think it would be void *hash_map(HTAB, void *(*map_func) (void *)) Not sure what the return value would be though :-( (Maybe this is extra complication enough that it's not worth the hassle) -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] DEALLOCATE ALL
Marko Kreen wrote: Then a pooler argument: there is one pooler where RandomJoe executes queries and another for specific app where the subset of SQL it uses is known. I want to RESET only specific things in app case. So it would be good if the RESET-s for specific areas would be available. Also the objections to the Hans' patch give impression that different pooling solutions want different RESET EVERYTHING, so again, it would be good if RESET-s for different areas are available and the all-encomassing RESET EVERYTHING just ties all the specific RESETs together. Totally agree. Please make the adjustments to DEALLOCATE ALL, and roll that into the 2004 patch for RESET SESSION and post and updated version. Thanks. -- 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 5: don't forget to increase your free space map settings
Re: [PATCHES] DEALLOCATE ALL
Marko Kreen wrote: When pooling connections where prepared statements are in use, it is hard to give new client totally clean connection as there may be allocated statements that give errors when new client starts preparing statements again. I agree with the other comments that RESET SESSION is the right API for this, although we can also provide DEALLOCATE ALL, I suppose. As to the implementation, calling hash_remove() in a loop seems a pretty unfortunate way to clear a hash table -- adding a hash_reset() function to the dynahash API would be cleaner and faster. -Neil ---(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] DEALLOCATE ALL
When pooling connections where prepared statements are in use, it is hard to give new client totally clean connection as there may be allocated statements that give errors when new client starts preparing statements again. Currently PgPool solves the situation by parsing all queries and keeping list of prepares statements. This may not be a big problem for it as it parses the queries anyway, but for simple pooler like PgBouncer (see pgfoundry) that does not look inside libpq packets it is huge problem. Attached is a patch that allows keyword ALL to be used with DEALLOCATE and then frees all prepared queryes. I think it is useful for most pooling situations, not only PgBouncer. Also its similar in the spirit to RESET ALL. I did it slightly hacky way - if DeallocateStmt-name is NULL is signifies DEALLOCATE ALL command. All the code that looks into DeallocateStmt seems to survive the situation so it should be problem. If still a new node is needed or additional field in the node I can rework the patch. -- marko dealloc_all.diff Description: Binary data ---(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] DEALLOCATE ALL
Marko Kreen wrote: When pooling connections where prepared statements are in use, it is hard to give new client totally clean connection as there may be allocated statements that give errors when new client starts preparing statements again. Huh, didn't we have a RESET SESSION command to do just that? What about cursors, for example? I did it slightly hacky way - if DeallocateStmt-name is NULL is signifies DEALLOCATE ALL command. All the code that looks into DeallocateStmt seems to survive the situation so it should be problem. If still a new node is needed or additional field in the node I can rework the patch. Wouldn't it be easier to just add a bool to DeallocateStmt? -- Alvaro Herrerahttp://www.PlanetPostgreSQL.org Si un desconocido se acerca y te regala un CD de Ubuntu ... Eso es ... Eau de Tux ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] DEALLOCATE ALL
Alvaro Herrera [EMAIL PROTECTED] writes: Marko Kreen wrote: When pooling connections where prepared statements are in use, it is hard to give new client totally clean connection as there may be allocated statements that give errors when new client starts preparing statements again. Huh, didn't we have a RESET SESSION command to do just that? What about cursors, for example? We don't actually *have* one, but I believe it was agreed that that is the right API to provide. If a pooler has to remember to clear prepared statements, GUCs, cursors, and who knows what else, it'll be perpetually broken because there'll be something it omits. There might be a use-case for DEALLOCATE ALL, but needs of poolers aren't it. I'd be inclined to vote against this unless someone can point to a better use-case. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] DEALLOCATE ALL
On 3/27/07, Tom Lane [EMAIL PROTECTED] wrote: Alvaro Herrera [EMAIL PROTECTED] writes: Marko Kreen wrote: When pooling connections where prepared statements are in use, it is hard to give new client totally clean connection as there may be allocated statements that give errors when new client starts preparing statements again. Huh, didn't we have a RESET SESSION command to do just that? What about cursors, for example? We don't actually *have* one, but I believe it was agreed that that is the right API to provide. If a pooler has to remember to clear prepared statements, GUCs, cursors, and who knows what else, it'll be perpetually broken because there'll be something it omits. Well. Please apply following patch then: http://archives.postgresql.org/pgsql-patches/2004-12/msg00228.php Even if it is incomplete, the missing parts can be added later. I see no reason to keep it from users. There might be a use-case for DEALLOCATE ALL, but needs of poolers aren't it. I'd be inclined to vote against this unless someone can point to a better use-case. Ok, a non-pooler argument: prepared statements are supposed to be garbage-collected by the user. Thats it. There should be friendly way to get a clean state without the need for user to specifically keep track of whats allocated, or to do messy exception-handling around PREPARE/DEALLOCATE. (PREPARE OR REPLACE and DEALLOCATE IF EXISTS would also lessen the pain.) Then a pooler argument: there is one pooler where RandomJoe executes queries and another for specific app where the subset of SQL it uses is known. I want to RESET only specific things in app case. So it would be good if the RESET-s for specific areas would be available. Also the objections to the Hans' patch give impression that different pooling solutions want different RESET EVERYTHING, so again, it would be good if RESET-s for different areas are available and the all-encomassing RESET EVERYTHING just ties all the specific RESETs together. -- marko ---(end of broadcast)--- TIP 6: explain analyze is your friend
