Thread: Re: PGAdmin 3 Patch // Memory Leaks Fixed // Ignore last patch
> -----Original Message----- > From: efesar [mailto:efesar@nmia.com] > Sent: 06 March 2003 05:51 > To: Dave Page; Pgadmin-Hackers > Subject: PGAdmin 3 Patch // Memory Leaks Fixed // Ignore last patch > > > > Dave, > > Nevermind that last patch if you haven't patched it yet. > > I got seriously bored of looking at memory leaks in VCC so I > went through and patched all the memory leaks that I could > find. Also, there are a couple of updates on the Query Builder. > > Here is a synopsis of changes: Hi Keith, Nice work, thanks. You must have been bored :-). As you can probably tell I'm fairly new to C++ and still get a little confused with references & pointers from time to time. Anyway, I've applied this patch to my copy of the tree, and once I found the wx2.4 change you commented (downloading it now), it built OK. Unfortunately, I still can't get the QueryBuilder to do anything. When I try to add a table there are none listed - I've tried a few databases, all of which have a number of tables. I haven't had a chance to figure out what's wrong. Also, if I click the Data tab I get a parse error, even if I manually type in a valid query (which then vanishes). > because using the one from pgServer was choosing random connections/databases. The one from pgServer should *always* be the master connection (ie. The database in the login dialogue). Is this not the case? On another note, how did you generate the patch? I had a heck of a job applying it as it found files in /src but not any of the others. It's probably me though - I often end up battling with patch.... Regards, Dave.
> Nice work, thanks. You must have been bored :-). As you can probably
> tell I'm fairly new to C++ and still get a little confused with
> references & pointers from time to time.
Dave,
I've been programming C++ for years and I consistently have memory leaks --
especially in MFC. The really hard part about MFC is that it only takes
ownership of attached items under certain conditions (and of course since
it's Microsoft code, you never really know when that is). The really *great*
thing about wxWindows is that if you attach something to a window, dialog,
frame or control, it's deallocation is *taken care of* all the time.
> Anyway, I've applied this patch to my copy of the tree, and once I found
> the wx2.4 change you commented (downloading it now), it built OK.
> Unfortunately, I still can't get the QueryBuilder to do anything. When I
> try to add a table there are none listed - I've tried a few databases,
> all of which have a number of tables. I haven't had a chance to figure
> out what's wrong. Also, if I click the Data tab I get a parse error,
> even if I manually type in a valid query (which then vanishes).
I'm really confused as to why it's not showing any tables/views ... the
function is below. As for the SQL tab, it's one way (for now ... which
reminds me, do we have a "deconstruct sql" function lying around?) ... the
data tab calls the BuildSQL function before it gets data ... it can't get
any data unless there are some columns, and columns require tables, and as
you know ... the tables are for some strange reason not working on your
configuration.
I'm running PostgreSQL 7.3 on Cygwin so ... Maybe I misjudged the query.
Take a look at it below.
////////////////////////////////////////////////////////////////////////////
////
void dlgAddTableView::InitLists()
{
// We need system settings
extern sysSettings *settings;
// We need to know if we're going to show system objects
wxString sysobjstr;
if (!settings->GetShowSystemObjects())
sysobjstr = " AND relowner > 1 ";
// Clear the lists
m_tablelist->Clear();
m_viewlist->Clear();
if (m_database->Connect() == PGCONN_OK) {
// tables
pgSet *tables = m_database->ExecuteSet(
wxT("SELECT oid,relname FROM pg_class "
"WHERE relkind='r' " + sysobjstr +
"ORDER BY lower(relname)"));
while (!tables->Eof()) {
m_tablelist->Append(tables->GetVal(wxT("relname")));
tables->MoveNext();
}
delete tables;
// views
pgSet *views = m_database->ExecuteSet(
wxT("SELECT relname FROM pg_class "
"WHERE relkind='v' " + sysobjstr +
"ORDER BY lower(relname)"));
while (!views->Eof()) {
m_viewlist->Append(views->GetVal(wxT("relname")));
views->MoveNext();
}
delete views;
}
}
////////////////////////////////////////////////////////////////////////////
////
> The one from pgServer should *always* be the master connection (ie. The
> database in the login dialogue). Is this not the case?
Yes, that makes sense now. Since pgServer chooses the master connection, I
added the ExecuteSet command to the pgDatabase so that the user can query
the pgConn member of pgDatabase, which already has the correct database
open. That way I won't have to issue a "change database" command to
pgServer. To be honest, I don't even know how to make pgServer switch
databases.
> On another note, how did you generate the patch? I had a heck of a job
> applying it as it found files in /src but not any of the others. It's
> probably me though - I often end up battling with patch....
I'm using this program called TortoiseCVS. I can't get WinCVS to work to
save my life. Tortoise works pretty well, but I have no idea how patches
work for the most part. I just click "Make Patch" ... Since there were 6
files that were not in CVS, I zipped them in with the .patch file ... but
didn't put them into any particular subdirectories. If you have suggestions
on how to make better patches, I'll try em.
I found this document in the wxwindows domain:
http://www.wxwindows.org/technote/patches.htm
I will draw your attention to item #3: cvs diff cannot make a patch which
adds or removes a file, so you must use the standalone diff utility to do
that ...
Huh? It does my brain no justice to be up this late trying to interpret what
that means.
-Keith
Hi Keith, > -----Original Message----- > From: efesar [mailto:efesar@nmia.com] > Sent: 06 March 2003 10:35 > To: Dave Page; Pgadmin-Hackers > Subject: Re: [pgadmin-hackers] PGAdmin 3 Patch // Memory > Leaks Fixed // Ignore last patch > > > > > Nice work, thanks. You must have been bored :-). As you can > probably > > tell I'm fairly new to C++ and still get a little confused with > > references & pointers from time to time. > > Dave, > > I've been programming C++ for years and I consistently have > memory leaks -- especially in MFC. The really hard part about > MFC is that it only takes ownership of attached items under > certain conditions (and of course since it's Microsoft code, > you never really know when that is). The really *great* thing > about wxWindows is that if you attach something to a window, > dialog, frame or control, it's deallocation is *taken care > of* all the time. It's good to know there's hope for me yet! > I'm really confused as to why it's not showing any > tables/views ... the function is below. It's because I had System Objects turned off, and as your definition of a system object is relowner > 1, it ignored all my objects which are owned by user postgres, id = 1!!! What you need to do is discount tables and views if they are in the pg_catalog, pg_temp_* or pg_toast namespaces, and columns is attnum < 1. > As for the SQL tab, > it's one way (for now ... which reminds me, do we have a > "deconstruct sql" function lying around?) Umm, no. The closest we have is in the query tool in pgAdmin II and it's big, complex and occasionally falls over. Perhaps we can steal some of the PostgreSQL parser code for pgAdmin III? I know nothing about how that works though. Anyway, now it does some things here, it looks excellent - Nice work!! > I'm using this program called TortoiseCVS. I can't get WinCVS > to work to save my life. Funny that, I could never get TortoiseCVS to work! > Tortoise works pretty well, but I > have no idea how patches work for the most part. I just click > "Make Patch" ... Since there were 6 files that were not in > CVS, I zipped them in with the .patch file ... but didn't put > them into any particular subdirectories. If you have > suggestions on how to make better patches, I'll try em. I always do it from the command line (I have Cygwin and most of the tools installed in my laptop). % cvs -z5 diff <dir/filename> Iirc. Of course, I only tend to create patches for other projects on odd occasions. > I found this document in the wxwindows domain: > > http://www.wxwindows.org/technote/patches.htm > > I will draw your attention to item #3: cvs diff cannot make a patch which adds or > removes a file, so you must use the standalone diff utility to do that ... > > Huh? It does my brain no justice to be up this late trying to interpret what > that means. :-). I just send new files manually as you have. Thanks again for your work. Regards, Dave.
-*- Dave Page <dpage@vale-housing.co.uk> [ 2003-03-07 09:49 ]: > > Tortoise works pretty well, but I > > have no idea how patches work for the most part. I just click > > "Make Patch" ... Since there were 6 files that were not in > > CVS, I zipped them in with the .patch file ... but didn't put > > them into any particular subdirectories. If you have > > suggestions on how to make better patches, I'll try em. > > I always do it from the command line (I have Cygwin and most of the > tools installed in my laptop). When I was searching for a CVS client for Win, TortoiseCVS was easy to set up and get working -- but people complained aboutexplorer being slow-as-hell, so I think most of them uninstalled it. However, we all use SmartCVS (www.smartcvs.com)nowadays. -- Regards, Tolli tolli@tol.li
> -----Original Message----- > From: Þórhallur Hálfdánarson [mailto:tolli@tol.li] > Sent: 07 March 2003 10:50 > To: Dave Page > Cc: efesar; Pgadmin-Hackers > Subject: Re: [pgadmin-hackers] PGAdmin 3 Patch // Memory > Leaks Fixed // Ignore last patch > > > -*- Dave Page <dpage@vale-housing.co.uk> [ 2003-03-07 09:49 ]: > > > Tortoise works pretty well, but I > > > have no idea how patches work for the most part. I just click > > > "Make Patch" ... Since there were 6 files that were not in > > > CVS, I zipped them in with the .patch file ... but didn't put > > > them into any particular subdirectories. If you have > > > suggestions on how to make better patches, I'll try em. > > > > I always do it from the command line (I have Cygwin and most of the > > tools installed in my laptop). > > When I was searching for a CVS client for Win, TortoiseCVS > was easy to set up and get working -- but people complained > about explorer being slow-as-hell, so I think most of them > uninstalled it. However, we all use SmartCVS > (www.smartcvs.com) nowadays. Hi Tolli, Actually it's only patches I do from the command line - I use WinCVS for most of my other work. But thanks for the link,I'll give it a look. Regards, Dave.
> > I'm really confused as to why it's not showing any > > tables/views ... the function is below. > > It's because I had System Objects turned off, and as your definition of > a system object is relowner > 1, it ignored all my objects which are > owned by user postgres, id = 1!!! > > What you need to do is discount tables and views if they are in the > pg_catalog, pg_temp_* or pg_toast namespaces, and columns is attnum < 1. Dave, Okay, that change is made. Now I do a join with pg_namespace.oid = pg_class.relnamespace and it looks like it's working. And for columns, I changed attnum >= 0 to attnum > 0 and it works as indicated. It will be functional in the next patch. > > As for the SQL tab, > > it's one way (for now ... which reminds me, do we have a > > "deconstruct sql" function lying around?) > > Umm, no. The closest we have is in the query tool in pgAdmin II and it's > big, complex and occasionally falls over. Perhaps we can steal some of > the PostgreSQL parser code for pgAdmin III? I know nothing about how > that works though. Yeah, that might be possible, but it will take some work. I did some cursory investigation, and a lot of function names and defines in pgsql conflict with the MS and wxWindows libraries. For right now I'm just building a small regex doodad to split everything up. It's working pretty well. Unfortunately the regex library that comes with WX is POSIX instead of PCRE. Oh well, I know both syntaxes, I'm just much more familiar with PCRE. For now I will make the SQL box read-only. I know there are other solutions, but are too costly to implement right now. > Anyway, now it does some things here, it looks excellent - Nice work!! Thanks. The next patch will add tons of functionality. At this point it can create 85% of the queries you'd ever build. > > I'm using this program called TortoiseCVS. I can't get WinCVS > > to work to save my life. > > Funny that, I could never get TortoiseCVS to work! Okay, I added certain files to .cvsignore ... hopefully the patches work better now. -Keith
It's rumoured that efesar once said: > > For now I will make the SQL box read-only. I know there are other > solutions, but are too costly to implement right now. Hi Keith, Some development on that. We're discussing some changes to the wire protocol for v7.4 on the pgsql-hackers list, and I've requested the addition of the attrelid and attnum columns to the RowDescription method whereever possible. This means we will know exactly where each attribute in a resultset has come from, so we can easily find out any info about it. If the message leaves those values blank, then we know there is noi underlying base attribute, so we cannot update (eg and aggregate, function call or undeterminable column from a natural join). I think it's going in, I've answered all the questions that Tom Lane has posed, and there are a few other strong supporters of the proposal. >> Anyway, now it does some things here, it looks excellent - Nice work!! > > Thanks. The next patch will add tons of functionality. At this point it > can create 85% of the queries you'd ever build. Excellent, looking forward to it. Regards, Dave.