Thread: Query tool execute to file prompting
Hi! This patch attempts to address the issue that at least I've heard a couple of time, of that the "execute to file" feature doesn't prompt for the filename until after it's finished. This is quite annoying when running multi-hour queries. Other than a review please, I couldn't find where we actually destroy the QueryExecInfo object allocated in frmQuery::execQuery(). It gets picked up in OnQueryComplete(), but I don't see a delete anywhere. Is it automatically deleted by the wx stuff (in which case I have a leak of qi->toFileExportForm that I have to take care of), or do we have a leak there today (in which case I have that leak to take care of :D). -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Attachment
On Tue, Mar 30, 2010 at 4:34 PM, Magnus Hagander <magnus@hagander.net> wrote: > Hi! > > This patch attempts to address the issue that at least I've heard a > couple of time, of that the "execute to file" feature doesn't prompt > for the filename until after it's finished. This is quite annoying > when running multi-hour queries. Looks sane. > Other than a review please, I couldn't find where we actually destroy > the QueryExecInfo object allocated in frmQuery::execQuery(). It gets > picked up in OnQueryComplete(), but I don't see a delete anywhere. Is > it automatically deleted by the wx stuff (in which case I have a leak > of qi->toFileExportForm that I have to take care of), or do we have a > leak there today (in which case I have that leak to take care of :D). It sure looks like a leak to me. Seems like we should delete it after the call to completeQuery. Not sure if that will leave a leak if the query is aborted though. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
On Wed, Mar 31, 2010 at 12:01 PM, Dave Page <dpage@pgadmin.org> wrote: > On Tue, Mar 30, 2010 at 4:34 PM, Magnus Hagander <magnus@hagander.net> wrote: >> Hi! >> >> This patch attempts to address the issue that at least I've heard a >> couple of time, of that the "execute to file" feature doesn't prompt >> for the filename until after it's finished. This is quite annoying >> when running multi-hour queries. > > Looks sane. > >> Other than a review please, I couldn't find where we actually destroy >> the QueryExecInfo object allocated in frmQuery::execQuery(). It gets >> picked up in OnQueryComplete(), but I don't see a delete anywhere. Is >> it automatically deleted by the wx stuff (in which case I have a leak >> of qi->toFileExportForm that I have to take care of), or do we have a >> leak there today (in which case I have that leak to take care of :D). > > It sure looks like a leak to me. Seems like we should delete it after > the call to completeQuery. Not sure if that will leave a leak if the > query is aborted though. Doesn't that just say that we have to delete it in OnCancel() as well? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
On Tue, Apr 6, 2010 at 6:55 PM, Magnus Hagander <magnus@hagander.net> wrote: > On Wed, Mar 31, 2010 at 12:01 PM, Dave Page <dpage@pgadmin.org> wrote: >> On Tue, Mar 30, 2010 at 4:34 PM, Magnus Hagander <magnus@hagander.net> wrote: >>> Hi! >>> >>> This patch attempts to address the issue that at least I've heard a >>> couple of time, of that the "execute to file" feature doesn't prompt >>> for the filename until after it's finished. This is quite annoying >>> when running multi-hour queries. >> >> Looks sane. >> >>> Other than a review please, I couldn't find where we actually destroy >>> the QueryExecInfo object allocated in frmQuery::execQuery(). It gets >>> picked up in OnQueryComplete(), but I don't see a delete anywhere. Is >>> it automatically deleted by the wx stuff (in which case I have a leak >>> of qi->toFileExportForm that I have to take care of), or do we have a >>> leak there today (in which case I have that leak to take care of :D). >> >> It sure looks like a leak to me. Seems like we should delete it after >> the call to completeQuery. Not sure if that will leave a leak if the >> query is aborted though. > > Doesn't that just say that we have to delete it in OnCancel() as well? Like attached. This passes without crashing when: 1) Execute query 2) Execute query and cancel 3) Execute query to file Seems reasonable? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Attachment
On Tue, Apr 6, 2010 at 6:04 PM, Magnus Hagander <magnus@hagander.net> wrote: > This passes without crashing when: > 1) Execute query > 2) Execute query and cancel > 3) Execute query to file > > Seems reasonable? Yes. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
On Tue, Apr 6, 2010 at 8:13 PM, Dave Page <dpage@pgadmin.org> wrote: > On Tue, Apr 6, 2010 at 6:04 PM, Magnus Hagander <magnus@hagander.net> wrote: >> This passes without crashing when: >> 1) Execute query >> 2) Execute query and cancel >> 3) Execute query to file >> >> Seems reasonable? > > Yes. BUg fix applied to trunk and 1.10, change of prompting applied to trunk only. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/