Thread: pgScript patch
Hi pgadmin hackers, pgScript can now be integrated into pgAdmin3. I have made a patch on revision 7394 of pgAdmin. This patch is big therefore I do not post it in this email, it is instead available on the following server: http://pgscript.projects.postgresql.org/pgadmin This patch works on a fresh copy of revision 7394. I tested it on: * Linux Slackware / g++ 4.2.3 * Windows XP / VC++ Express 2005 It adds a pgScript button and a menu entry in the SQL Query Tool. When one clicks on this button, the content of the text box is sent to the pgScript engine instead of PostgreSQL. The pgScript syntax is detailed on this page: http://pgscript.projects.postgresql.org/SCRIPT.html That's it. I hope to have feedbacks. Best regards, Mickael
Hi, Mickael Deloison a écrit : > [...] > pgScript can now be integrated into pgAdmin3. I have made a patch on > revision 7394 of pgAdmin. This patch is big therefore I do not post it > in this email, it is instead available on the following server: > http://pgscript.projects.postgresql.org/pgadmin > > This patch works on a fresh copy of revision 7394. I tested it on: > * Linux Slackware / g++ 4.2.3 > * Windows XP / VC++ Express 2005 > > It adds a pgScript button and a menu entry in the SQL Query Tool. When > one clicks on this button, the content of the text box is sent to the > pgScript engine instead of PostgreSQL. The pgScript syntax is detailed > on this page: > http://pgscript.projects.postgresql.org/SCRIPT.html > > That's it. I hope to have feedbacks. > It applies cleanly. The build works. After pgscript run, you should set the status bar to indicate the end of the script. It shows "pgscript running" even after the end of the run. This script causes a pgAdmin3 crash. Don't know why. declare @index; set @index = 20; while @index > 0 begin print @index; insert into t1 values (@index); set @index = @index - 1; end You'll find attached a log written on the console after pgadmin crashes. We should ask Ennixo to create a better icon to launch a pgscript. Regards. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Attachment
Hi, > This script causes a pgAdmin3 crash. Don't know why. > > declare @index; > > set @index = 20; > > > while @index > 0 > begin > print @index; > insert into t1 values (@index); > set @index = @index - 1; > end > > You'll find attached a log written on the console after pgadmin crashes. > This is weird. 1) I cannot reproduce the problem at all if there is no Unique or Primary Key constraint on t1 first column (where to put @index). 2) No problem under Windows even with Primary/Unique constraint. A Warning window is popped up when a Primary Key is duplicated and the script goes on. 3) Under Linux/GTK there seems to be a problem when a query fails (when a Primary Key is duplicated in this example) and that pgScript wants to log a warning (which is supposed to show a Warning window). But, even in this case my error message are not exactly the same as yours (log file attached). So I will investigate more next week. Regards, Mickael
Attachment
Hi Mickael On Sun, Jul 27, 2008 at 3:34 PM, Mickael Deloison <mdeloison@gmail.com> wrote: > Hi pgadmin hackers, > > pgScript can now be integrated into pgAdmin3. I have made a patch on > revision 7394 of pgAdmin. This patch is big therefore I do not post it > in this email, it is instead available on the following server: > http://pgscript.projects.postgresql.org/pgadmin Cool. It is indeed a huge patch, so I'll have to leave it to your mentor to undertake a more extensive code review, but here are a few points I noticed whilst testing on Mac: - Please include "Copyright (C) 2002 - 2008, The pgAdmin Development Team" in the copyright notices at the top of each source file. I'm happy for you to include your own copyright there also, but it makes things much easier from a legal POV if you list us as well. - The build failed initially with: ./pgscript/statements/pgsStmtList.cpp: In member function 'virtual void pgsStmtList::eval(pgsVarMap&) const': ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid with -fno-rtti ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid with -fno-rtti ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid with -fno-rtti ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid with -fno-rtti ./pgscript/statements/pgsStmtList.cpp: In member function 'virtual void pgsStmtList::eval(pgsVarMap&) const': ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid with -fno-rtti ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid with -fno-rtti ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid with -fno-rtti ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid with -fno-rtti lipo: can't figure out the architecture type of: /var/folders/uk/ukdzizfJHxe07gKAk8a+NE+++TI/-Tmp-//ccIbnqxz.out make[2]: *** [pgsStmtList.o] Error 1 make[1]: *** [all-recursive] Error 1 make: *** [all] Error 2 After removing -fno-rtti from acinclude.m4: - I see the following warnings: ./pgadmin/include/frm/frmQuery.h: In constructor 'frmQuery::frmQuery(frmMain*, const wxString&, pgConn*, const wxString&, const wxString&)': ../pgadmin/include/frm/frmQuery.h:75: warning: 'frmQuery::pgscript' will be initialized after ../pgadmin/include/frm/frmQuery.h:71: warning: 'wxTimer frmQuery::timer' ./frm/frmQuery.cpp:130: warning: when initialized here ../pgadmin/include/frm/frmQuery.h: In constructor 'frmQuery::frmQuery(frmMain*, const wxString&, pgConn*, const wxString&, const wxString&)': ../pgadmin/include/frm/frmQuery.h:75: warning: 'frmQuery::pgscript' will be initialized after ../pgadmin/include/frm/frmQuery.h:71: warning: 'wxTimer frmQuery::timer' ./frm/frmQuery.cpp:130: warning: when initialized here - The following script crashes (yes, I realise it's missing a cast) declare @i, @t; set @i = 0; while @i < 20 begin set @t = 'aa' + @i; create table @t (id serial primary key, data text); set @i = @i + 1; end - The corrected script gives no feedback that it's finished, other than re-enabling buttons. I would expect to see the appropriate notices from the server about each table that is created, and the status message on the status bar should change. - The following script (with missing increment of @i) gave appropriate errors when run the first time, but ran silently the second: declare @i, @t; set @i = 0; while @i < 20 begin set @t = 'aa' + cast(@i as string); create table @t (id serial primary key, data text); end - Cancelling that script the first time round is awkward (we should offer a cancel option on the error dialogue). Using the Stop button seems to crash. I'll leave it at that for now, and look forward to the next patch :-) -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
On Mon, Aug 4, 2008 at 11:03 AM, Dave Page <dpage@pgadmin.org> wrote: > I'll leave it at that for now, and look forward to the next patch :-) Forgot to mention - we'll also need a doc patch when this is applied. Probably some tweaks to the query to page, and a reformated version of your syntax reference. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Hello Dave, 2008/8/4 Dave Page <dpage@pgadmin.org>: > Cool. It is indeed a huge patch, so I'll have to leave it to your > mentor to undertake a more extensive code review, but here are a few > points I noticed whilst testing on Mac: > > - Please include "Copyright (C) 2002 - 2008, The pgAdmin Development > Team" in the copyright notices at the top of each source file. I'm > happy for you to include your own copyright there also, but it makes > things much easier from a legal POV if you list us as well. Will be done in pgScript-1.0-beta-2. > - The build failed initially with: > > ./pgscript/statements/pgsStmtList.cpp: In member function 'virtual > void pgsStmtList::eval(pgsVarMap&) const': > ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid > with -fno-rtti > ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid > with -fno-rtti > ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid > with -fno-rtti > ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid > with -fno-rtti > ./pgscript/statements/pgsStmtList.cpp: In member function 'virtual > void pgsStmtList::eval(pgsVarMap&) const': > ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid > with -fno-rtti > ./pgscript/statements/pgsStmtList.cpp:56: error: cannot use typeid > with -fno-rtti > ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid > with -fno-rtti > ./pgscript/statements/pgsStmtList.cpp:57: error: cannot use typeid > with -fno-rtti > lipo: can't figure out the architecture type of: > /var/folders/uk/ukdzizfJHxe07gKAk8a+NE+++TI/-Tmp-//ccIbnqxz.out > make[2]: *** [pgsStmtList.o] Error 1 > make[1]: *** [all-recursive] Error 1 > make: *** [all] Error 2 Indeed, I am using virtual methods. Is there any reasons why you compiled with -fno-rtti ? Do I have to use wxWidgets RTTI capabilities instead of C++ built-in capabilities? > After removing -fno-rtti from acinclude.m4: > > - I see the following warnings: > > ./pgadmin/include/frm/frmQuery.h: In constructor > 'frmQuery::frmQuery(frmMain*, const wxString&, pgConn*, const > wxString&, const wxString&)': > ../pgadmin/include/frm/frmQuery.h:75: warning: 'frmQuery::pgscript' > will be initialized after > ../pgadmin/include/frm/frmQuery.h:71: warning: 'wxTimer frmQuery::timer' > ./frm/frmQuery.cpp:130: warning: when initialized here > ../pgadmin/include/frm/frmQuery.h: In constructor > 'frmQuery::frmQuery(frmMain*, const wxString&, pgConn*, const > wxString&, const wxString&)': > ../pgadmin/include/frm/frmQuery.h:75: warning: 'frmQuery::pgscript' > will be initialized after > ../pgadmin/include/frm/frmQuery.h:71: warning: 'wxTimer frmQuery::timer' > ./frm/frmQuery.cpp:130: warning: when initialized here Will fix that in the next patch. > - The following script crashes (yes, I realise it's missing a cast) > > declare @i, @t; > > set @i = 0; > > while @i < 20 > begin > set @t = 'aa' + @i; > create table @t (id serial primary key, data text); > > set @i = @i + 1; > end I will look deeper at this problem tomorrow. > - The corrected script gives no feedback that it's finished, other > than re-enabling buttons. I would expect to see the appropriate > notices from the server about each table that is created, and the > status message on the status bar should change. Right, it is the same feedback as Guillaume and I am currently working on that. Once again it will be included in the next patch. > - The following script (with missing increment of @i) gave appropriate > errors when run the first time, but ran silently the second: > > declare @i, @t; > > set @i = 0; > > while @i < 20 > begin > set @t = 'aa' + cast(@i as string); > create table @t (id serial primary key, data text); > end > > - Cancelling that script the first time round is awkward (we should > offer a cancel option on the error dialogue). Using the Stop button > seems to crash. Weird, as the previous problem I need to look deeper at that in the next following days. About the documentation, I thought about adding information about the pgScript button in the Query page and a link to the (possibly reformated) syntax reference. About this syntax reference, how did you find it? Was it clear? Thank you for your feedback, it is really helpful! Best regards, Mickael
Hi On Wed, Aug 6, 2008 at 6:57 PM, Mickael Deloison <mdeloison@gmail.com> wrote: > Indeed, I am using virtual methods. Is there any reasons why you > compiled with -fno-rtti ? Do I have to use wxWidgets RTTI capabilities > instead of C++ built-in capabilities? I think it was probably paranoia about overhead from including rtti info. We rarely use the wxWidgets code - pretty much everywhere in the pgAdmin code where we cast things about, it's from one child of pgObject to another. We keep our own meta type id in each object variation so we can know when to cast safely. If there's no major difference in binary size etc. from enabling rtti, I don't object to doing so. > Weird, as the previous problem I need to look deeper at that in the > next following days. :-) > About the documentation, I thought about adding information about the > pgScript button in the Query page and a link to the (possibly > reformated) syntax reference. Agree on the button. I'd like to see the syntax reference as a separate page in the docs though, not a link to an external site. > About this syntax reference, how did you > find it? Was it clear? Clear, but could use a handful of good examples :-) Keep up the good work! -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
On Tue, Aug 12, 2008 at 12:51 PM, Mickael Deloison <mdeloison@gmail.com> wrote: Hi, > The big question. You told me that my first patch is a huge patch. Yes, but that was just a passing comment - I didn't mean we shouldn't use it, or that you needed to reduce the size. I hope you didn't spend too much time on it because of a misunderstanding. > Indeed! A big code review is required and perhaps a lot of more tests. > So maybe this way of doing (separate program) is good for now. What's > your opinion about that? Should I keep pgScript code integrated into > pgAdmin or should I keep it as a separate program? Well that really is an interesting question. I'm not sure I know the answer, so I'll attempt to sum up some pros and cons and see what the other hackers think: Pro integration: - One less dependency for packages to worry about - One build system to maintain - Functionality is more responsive - No problems with different DLL versions if builds come from different machines. - Cannot be broken by moving one exe etc. Pro seperation: - pgScript can be maintained (and upgraded) independently of pgAdmin - No big impact on the pgAdmin source tree (and consequent learning curve for other developers). Despite the difference in numbers, I think those pros and cons are roughly equal. Any other thoughts? (Magnus, Guillaume, Hiroshi etc). -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Dave Page wrote: > On Tue, Aug 12, 2008 at 12:51 PM, Mickael Deloison <mdeloison@gmail.com> wrote: >> Indeed! A big code review is required and perhaps a lot of more tests. >> So maybe this way of doing (separate program) is good for now. What's >> your opinion about that? Should I keep pgScript code integrated into >> pgAdmin or should I keep it as a separate program? > > Well that really is an interesting question. I'm not sure I know the > answer, so I'll attempt to sum up some pros and cons and see what the > other hackers think: > > Pro integration: > > - One less dependency for packages to worry about > - One build system to maintain > - Functionality is more responsive > - No problems with different DLL versions if builds come from > different machines. > - Cannot be broken by moving one exe etc. > > Pro seperation: > > - pgScript can be maintained (and upgraded) independently of pgAdmin > - No big impact on the pgAdmin source tree (and consequent learning > curve for other developers). > > Despite the difference in numbers, I think those pros and cons are > roughly equal. > > Any other thoughts? (Magnus, Guillaume, Hiroshi etc). I think the Pros overweigh, so I'd like to see it there. If the isolation is still pretty good, there is no need for a large number of "pgadmin developers" to get a learning curve to get over. And the part in the query tool will need to be done that way *anyway*. The only "major" con is the upgrade one, but I think that's fairly survivable. Since pgscript depends on all the wx stuff anyway, it's not feasible for it to ever be included in the backend distribution, which is what I would've liked even more (as a libpgscript thingy). I think bundling it with pgadmin makes most sense in this case, both in a binary and source perspective, the same way we do with pgAgent. //Magnus
On Tue, Aug 12, 2008 at 3:29 PM, Magnus Hagander <magnus@hagander.net> wrote: > I think the Pros overweigh, so I'd like to see it there. > > If the isolation is still pretty good, there is no need for a large > number of "pgadmin developers" to get a learning curve to get over. And > the part in the query tool will need to be done that way *anyway*. > > The only "major" con is the upgrade one, but I think that's fairly > survivable. > > Since pgscript depends on all the wx stuff anyway, it's not feasible for > it to ever be included in the backend distribution, which is what I > would've liked even more (as a libpgscript thingy). I think bundling it > with pgadmin makes most sense in this case, both in a binary and source > perspective, the same way we do with pgAgent. pgAgent is a bad example as it's scheduled for removal from the core this release anyway. If we're going to bundle pgScript with pgAdmin, then I'd rather just integrate and be done with it. If we're going to keep it separate, then that should be because it truly is separate (can have it's own release schedule etc). Of course, that wouldn't mean it can't share pgAdmin resources such as SVN and the website etc. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Dave Page wrote: > On Tue, Aug 12, 2008 at 3:29 PM, Magnus Hagander <magnus@hagander.net> wrote: > >> I think the Pros overweigh, so I'd like to see it there. >> >> If the isolation is still pretty good, there is no need for a large >> number of "pgadmin developers" to get a learning curve to get over. And >> the part in the query tool will need to be done that way *anyway*. >> >> The only "major" con is the upgrade one, but I think that's fairly >> survivable. >> >> Since pgscript depends on all the wx stuff anyway, it's not feasible for >> it to ever be included in the backend distribution, which is what I >> would've liked even more (as a libpgscript thingy). I think bundling it >> with pgadmin makes most sense in this case, both in a binary and source >> perspective, the same way we do with pgAgent. > > pgAgent is a bad example as it's scheduled for removal from the core > this release anyway. If we're going to bundle pgScript with pgAdmin, > then I'd rather just integrate and be done with it. If we're going to > keep it separate, then that should be because it truly is separate > (can have it's own release schedule etc). Of course, that wouldn't > mean it can't share pgAdmin resources such as SVN and the website etc. Ok, bad example. But I think you misunderstood the part about "isolation", if that's what you mean with the part about separate. I mean, the vast majority of pgscript will not overlap with th vast majority of pgadmin code. Thus, someone who's "just working on the object tree" for example, will not be affected at all by integrating pgscript. Whereas we should of course integrate them at all points where it's reasonable, because that is likely to give a much better user experience. //Magnus
On Tue, Aug 12, 2008 at 3:39 PM, Magnus Hagander <magnus@hagander.net> wrote: > Ok, bad example. But I think you misunderstood the part about > "isolation", if that's what you mean with the part about separate. > > I mean, the vast majority of pgscript will not overlap with th vast > majority of pgadmin code. Thus, someone who's "just working on the > object tree" for example, will not be affected at all by integrating > pgscript. > > Whereas we should of course integrate them at all points where it's > reasonable, because that is likely to give a much better user experience. What you advocate there sounds to me like it should be integrated in the sense that it's part of our codebase, but isolated in it's own project and built as a DLL to be used by pgAdmin. Which in some ways gives us the best of both worlds, as Mickael could continue to maintain the code outside of the pgAdmin cycle, either directly on branches of the pgAdmin code, or by working on a copy from which we update the pgAdmin tree periodically. I actually kinda like that idea... -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
2008/8/12 Dave Page <dpage@pgadmin.org>: > What you advocate there sounds to me like it should be integrated in > the sense that it's part of our codebase, but isolated in it's own > project and built as a DLL to be used by pgAdmin. Which in some ways > gives us the best of both worlds, as Mickael could continue to > maintain the code outside of the pgAdmin cycle, either directly on > branches of the pgAdmin code, or by working on a copy from which we > update the pgAdmin tree periodically. > > I actually kinda like that idea... At this time, when fully integrated into pgAdmin codebase, pgScript is in pgadmin/include/pgscript and pgadmin/pgscript directories: so this quite "isolated". But, for me, the big pro of having pgScript as a separate executable (pgadmin3.exe and pgscript.exe) is that the operating system takes care of cleaning pgScript mess when this last one exits: you cannot have any memory leak or instability in pgAdmin because of a potential bug in pgScript [ I hope there is none ;-), but at this time I have some problems with the threads ]. I think a DLL is just a way of isolating pgScript a bit more but maybe I'm wrong. Anyway, pgScript can be compiled as a static library (lipbpgs.a). I do not know how to make a DLL (never done that) but I guess it could be done quite easily since I can compile it as a static library. Mickael
Mickael Deloison wrote: > 2008/8/12 Dave Page <dpage@pgadmin.org>: >> What you advocate there sounds to me like it should be integrated in >> the sense that it's part of our codebase, but isolated in it's own >> project and built as a DLL to be used by pgAdmin. Which in some ways >> gives us the best of both worlds, as Mickael could continue to >> maintain the code outside of the pgAdmin cycle, either directly on >> branches of the pgAdmin code, or by working on a copy from which we >> update the pgAdmin tree periodically. >> >> I actually kinda like that idea... > > At this time, when fully integrated into pgAdmin codebase, pgScript is > in pgadmin/include/pgscript and pgadmin/pgscript directories: so this > quite "isolated". > > But, for me, the big pro of having pgScript as a separate executable > (pgadmin3.exe and pgscript.exe) is that the operating system takes > care of cleaning pgScript mess when this last one exits: you cannot > have any memory leak or instability in pgAdmin because of a potential > bug in pgScript [ I hope there is none ;-), but at this time I have > some problems with the threads ]. > > I think a DLL is just a way of isolating pgScript a bit more but maybe > I'm wrong. Anyway, pgScript can be compiled as a static library > (lipbpgs.a). I do not know how to make a DLL (never done that) but I > guess it could be done quite easily since I can compile it as a static > library. It usually is. But if it's not bundled, a very clear interface has to be defined, and it's a contract you cannot violate easily. That's what causes DLL Hell... Which is one reason I like the tight integration. //Magnus
Dave Page a écrit : > On Tue, Aug 12, 2008 at 3:39 PM, Magnus Hagander <magnus@hagander.net> wrote: > >> Ok, bad example. But I think you misunderstood the part about >> "isolation", if that's what you mean with the part about separate. >> >> I mean, the vast majority of pgscript will not overlap with th vast >> majority of pgadmin code. Thus, someone who's "just working on the >> object tree" for example, will not be affected at all by integrating >> pgscript. >> >> Whereas we should of course integrate them at all points where it's >> reasonable, because that is likely to give a much better user experience. > > What you advocate there sounds to me like it should be integrated in > the sense that it's part of our codebase, but isolated in it's own > project and built as a DLL to be used by pgAdmin. Which in some ways > gives us the best of both worlds, as Mickael could continue to > maintain the code outside of the pgAdmin cycle, either directly on > branches of the pgAdmin code, or by working on a copy from which we > update the pgAdmin tree periodically. > > I actually kinda like that idea... > I also prefer the DLL idea. Not sure I understand the interest of pgscript in pgAdmin, though... -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
2008/8/12 Guillaume Lelarge <guillaume@lelarge.info>: > I also prefer the DLL idea. Not sure I understand the interest of pgscript > in pgAdmin, though... Hi, Do you have instructions or advice on doing so? Mickael
Mickael Deloison a écrit : > 2008/8/12 Guillaume Lelarge <guillaume@lelarge.info>: >> I also prefer the DLL idea. Not sure I understand the interest of pgscript >> in pgAdmin, though... > > Hi, > Do you have instructions or advice on doing so? About the DLL stuff? unfortunately, no. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com