Thread: Query results in grid, new patch
Attached is a newer version of my query results in a grid patch. This version adds a custom table class to store the data. My quick testing on WinXP shows this patch to display query results in about one half the time my previous patch did. That patch, at least on my system, in turn displayed results in about one half the time the old code did. So at least on WinXP, this patch will result in results displaying in approximately 25% of the time they previously took. The table class I created contains the bare minimum functionality I needed to make it work. To help improve the speed, I designed the class to use as few memory allocations as possible. As with the previous patch, there are still focus issues that make the clipboard support flakey. The issue seems to be the grid control doesn't seem to hold onto the focus the way the old list object did, making it difficult to decide which object should handle a Copy request. I don't know very much about wxWidgets, so I'm stumped on the issue. Someone else will have to help out. Ed
Attachment
Edward Di Geronimo Jr. wrote: > > As with the previous patch, there are still focus issues that make the > clipboard support flakey. The issue seems to be the grid control doesn't > seem to hold onto the focus the way the old list object did, making it > difficult to decide which object should handle a Copy request. I don't > know very much about wxWidgets, so I'm stumped on the issue. Someone else > will have to help out. This is an ancient issue.... As I said, we had marking problems in wxGrid for years, still not fully solved. Regards, Andreas
> -----Original Message----- > From: pgadmin-hackers-owner@postgresql.org > [mailto:pgadmin-hackers-owner@postgresql.org] On Behalf Of > Edward Di Geronimo Jr. > Sent: 27 February 2006 08:00 > To: pgadmin-hackers@postgresql.org > Subject: [pgadmin-hackers] Query results in grid, new patch Hi Ed, > Attached is a newer version of my query results in a grid patch. This > version adds a custom table class to store the data. I got a couple of warnings building this that should be cleaned up: c:\documents and settings\dpage\my documents\svn\pgadmin3\src\ctl\ctlsqlresult.cpp(514) : warning C4018: '>=' : signed/unsigned mismatch c:\documents and settings\dpage\my documents\svn\pgadmin3\src\ctl\ctlsqlresult.cpp(519) : warning C4018: '>' : signed/unsigned mismatch > My quick > testing on > WinXP shows this patch to display query results in about one > half the time > my previous patch did. That patch, at least on my system, in turn > displayed results in about one half the time the old code > did. So at least > on WinXP, this patch will result in results displaying in > approximately > 25% of the time they previously took. I just tried it here with a LIMIT 100000 query, and got: 52.277 Secs + 9.123 Secs (v1.5, with your patch) 52.276 Secs + 39.688 Secs (v1.4.1) Wow! Very impressive. > The table class I created contains the bare minimum > functionality I needed > to make it work. To help improve the speed, I designed the > class to use as > few memory allocations as possible. OK. > As with the previous patch, there are still focus issues that make the > clipboard support flakey. The issue seems to be the grid > control doesn't > seem to hold onto the focus the way the old list object did, making it > difficult to decide which object should handle a Copy request. I don't > know very much about wxWidgets, so I'm stumped on the issue. > Someone else > will have to help out. I have been unable to make it misbehave on XP. I haven't tried OSX or Linux as I don't have either to hand atm. Can you give me a precise example to test? I have had complete success copying individual cells, groups of rows, groups of columns and arbitrary blocks of cells. Other than that, my only comment is the column headers which look even messier than they did previously (because the text appears to be centered rather than left justified). Can you alter it to place the data type on the line below, as per the Edit Grid please? Regards, Dave.
Dave Page wrote: > I have been unable to make it misbehave on XP. I haven't tried OSX or > Linux as I don't have either to hand atm. Can you give me a precise > example to test? I have had complete success copying individual cells, > groups of rows, groups of columns and arbitrary blocks of cells. I'm current building a version of pgadmin3-trunk with this patch included. Should show up as pgadmin3-test in the snapshots folder on developer.pgadmin.org in an hour or two (That G3 really takes ages to compile pgadmin... ;-) ). greetings, Florian Pflug
> -----Original Message----- > From: Florian G. Pflug [mailto:fgp@phlo.org] > Sent: 27 February 2006 12:13 > To: Dave Page > Cc: Edward Di Geronimo Jr.; pgadmin-hackers@postgresql.org > Subject: Re: [pgadmin-hackers] Query results in grid, new patch > > Dave Page wrote: > > I have been unable to make it misbehave on XP. I haven't > tried OSX or > > Linux as I don't have either to hand atm. Can you give me a precise > > example to test? I have had complete success copying > individual cells, > > groups of rows, groups of columns and arbitrary blocks of cells. > > I'm current building a version of pgadmin3-trunk with this > patch included. Should show up as pgadmin3-test in the > snapshots folder on developer.pgadmin.org in an hour or > two (That G3 really takes ages to compile pgadmin... ;-) ). :-) My build was actually quite fast on battery power last night. I wonder if it's because when I rehashed the Makefiles recently I included the headers in pgadmin3_SOURCES rather than noinst_HEADERS (as recommended by the automake docs). /D
> I got a couple of warnings building this that should be cleaned up: Fixed. Didn't realize I missed a few. > I have been unable to make it misbehave on XP. I haven't tried OSX or > Linux as I don't have either to hand atm. Can you give me a precise > example to test? I have had complete success copying individual cells, > groups of rows, groups of columns and arbitrary blocks of cells. Hmmm, it does seem better now than it used to. Still one problem I can find. Run a query. Click somewhere in the edit text box. Click on a column or row header to select an entire row. The focus stays in the edit text box. If you click on a cell first then select the row or column it works ok, but it would be nicer if this worked. > Other than that, my only comment is the column headers which look even > messier than they did previously (because the text appears to be > centered rather than left justified). Can you alter it to place the data > type on the line below, as per the Edit Grid please? I changed it to left justify the text and display the data type on the line below. New patch attached. Ed
Attachment
> -----Original Message----- > From: Edward Di Geronimo Jr. [mailto:edigeronimo@xtracards.com] > Sent: 27 February 2006 18:39 > To: Dave Page > Cc: Edward Di Geronimo Jr.; pgadmin-hackers@postgresql.org > Subject: RE: [pgadmin-hackers] Query results in grid, new patch > > Hmmm, it does seem better now than it used to. Still one problem I can > find. Run a query. Click somewhere in the edit text box. > Click on a column > or row header to select an entire row. The focus stays in the > edit text > box. If you click on a cell first then select the row or > column it works > ok, but it would be nicer if this worked. OK, can this be solved with wx ctlGrid class derived from wxGrid that does a SetFocus on itself when a column or row is selected? > > Other than that, my only comment is the column headers > which look even > > messier than they did previously (because the text appears to be > > centered rather than left justified). Can you alter it to > place the data > > type on the line below, as per the Edit Grid please? > > I changed it to left justify the text and display the data type on the > line below. > > New patch attached. Thanks, looking good. We should probably tweak the edit grid to match the left justified style when this gets accepted. Note to Andreas: unless you have any concrete technical reasons why this patch should not be applied, I'm fully intending to accept it once the current wrinkles are ironed out. Please shout now rather than later!! Regards, Dave.
> OK, can this be solved with wx ctlGrid class derived from wxGrid that > does a SetFocus on itself when a column or row is selected? I registered a handle for the selection event, and put in a call to SetFocus. Seems to work now. > Thanks, looking good. We should probably tweak the edit grid to match > the left justified style when this gets accepted. Added that change in. Ed
Attachment
On 28/2/06 19:00, "Edward Di Geronimo Jr." <edigeronimo@xtracards.com> wrote: >> OK, can this be solved with wx ctlGrid class derived from wxGrid that >> does a SetFocus on itself when a column or row is selected? > > I registered a handle for the selection event, and put in a call to > SetFocus. Seems to work now. > >> Thanks, looking good. We should probably tweak the edit grid to match >> the left justified style when this gets accepted. > > Added that change in. Looks good and all works perfectly as far as I can see on OSX. I'll test on XP tomorrow and apply sometime following that if there are no objections. Thanks Ed - nice work! Regards, Dave.
Dave Page wrote: > > > Note to Andreas: unless you have any concrete technical reasons why this > patch should not be applied, I'm fully intending to accept it once the > current wrinkles are ironed out. Please shout now rather than later!! I won't find time to review it before the weekend. Regards, Andreas
On 28/2/06 21:46, "Andreas Pflug" <pgadmin@pse-consulting.de> wrote: > Dave Page wrote: >> > >> >> Note to Andreas: unless you have any concrete technical reasons why this >> patch should not be applied, I'm fully intending to accept it once the >> current wrinkles are ironed out. Please shout now rather than later!! > > I won't find time to review it before the weekend. OK, will keep that in mind. /D
Florian G. Pflug wrote: > Dave Page wrote: >> I have been unable to make it misbehave on XP. I haven't tried OSX or >> Linux as I don't have either to hand atm. Can you give me a precise >> example to test? I have had complete success copying individual cells, >> groups of rows, groups of columns and arbitrary blocks of cells. > I'm current building a version of pgadmin3-trunk with this > patch included. Should show up as pgadmin3-test in the > snapshots folder on developer.pgadmin.org in an hour or > two (That G3 really takes ages to compile pgadmin... ;-) ). Hi I've finally come around to testing the new grid on osx, I must say that I'm really impressed. It seems much more responsive than the old one to me, and being able to copy rows, cols and fields to the clipboard really rockts. I have to rather small suggestions, though .) Ctrl-A (or Cmd-A on OSX) should select the whole table - it's a rather well established UI-standard I believe to let Ctrl-A/Cmd-A select the whole "whatever" you are currently editing. .) Could the initial size of the columns be choosen so, that the captions are not cut off? Or, even better, pgadmin could remember the with of the columns between selects, at least if the captions stay the same... .) On OSX, the gray background looks rather strange, white would be a better choice I believe. Still, great work Greetings, Florian Pflug
Florian G. Pflug wrote: > > .) Could the initial size of the columns be choosen so, that the captions > are not cut off? Or, even better, pgadmin could remember the with of the > columns > between selects, at least if the captions stay the same... The old control's behaviour was like that. Regards, Andreas
> -----Original Message----- > From: Andreas Pflug [mailto:pgadmin@pse-consulting.de] > Sent: 06 March 2006 09:42 > To: Florian G. Pflug > Cc: Dave Page; Edward Di Geronimo Jr.; pgadmin-hackers@postgresql.org > Subject: Re: [pgadmin-hackers] Query results in grid, new patch > > Florian G. Pflug wrote: > > > > > .) Could the initial size of the columns be choosen so, > that the captions > > are not cut off? Or, even better, pgadmin could remember > the with of the > > columns > > between selects, at least if the captions stay the same... > > The old control's behaviour was like that. Yes, it was, but it might be a nice fix. It's bugged me for a while. /D
> -----Original Message----- > From: Edward Di Geronimo Jr. [mailto:edigeronimo@xtracards.com] > Sent: 28 February 2006 19:00 > To: Dave Page > Cc: pgadmin-hackers@postgresql.org > Subject: RE: [pgadmin-hackers] Query results in grid, new patch > > > OK, can this be solved with wx ctlGrid class derived from > wxGrid that > > does a SetFocus on itself when a column or row is selected? > > I registered a handle for the selection event, and put in a call to > SetFocus. Seems to work now. > > > Thanks, looking good. We should probably tweak the edit > grid to match > > the left justified style when this gets accepted. > > Added that change in. Thanks, patch applied. Regards, Dave.
Dave Page wrote: > > > >>-----Original Message----- >>From: Andreas Pflug [mailto:pgadmin@pse-consulting.de] >>Sent: 06 March 2006 09:42 >>To: Florian G. Pflug >>Cc: Dave Page; Edward Di Geronimo Jr.; pgadmin-hackers@postgresql.org >>Subject: Re: [pgadmin-hackers] Query results in grid, new patch >> >>Florian G. Pflug wrote: >> >> >>>.) Could the initial size of the columns be choosen so, >> >>that the captions >> >>>are not cut off? Or, even better, pgadmin could remember >> >>the with of the >> >>>columns >>>between selects, at least if the captions stay the same... >> >>The old control's behaviour was like that. > > > Yes, it was, but it might be a nice fix. It's bugged me for a while. Hu? I meant the control *does* remember column sizing for repeated execution of similar queries since 1.2!!! This is not a "nice fix", it is *required*. I don't give a cent for copy/paste from the result (in 10 years on SQL RDBMS, I might have done that 5 times), I need repeatable results from query tuning. Trying to autosize the column from content doesn't work satisfying. Regards, Andreas
> -----Original Message----- > From: Andreas Pflug [mailto:pgadmin@pse-consulting.de] > Sent: 06 March 2006 13:12 > To: Dave Page > Cc: Florian G. Pflug; Edward Di Geronimo Jr.; > pgadmin-hackers@postgresql.org > Subject: Re: [pgadmin-hackers] Query results in grid, new patch > > Hu? I meant the control *does* remember column sizing for repeated > execution of similar queries since 1.2!!! This is not a "nice > fix", it > is *required*. Chill out - I meant that default sizing based on the width of the headers would be a nice fix so that the names weren't truncated. I will look at the size retention issue - I hadn't noticed that was broken. > I don't give a cent for copy/paste from the > result (in 10 > years on SQL RDBMS, I might have done that 5 times), I need > repeatable > results from query tuning. I on the other hand copy and paste data on a regular basis in SQL Enterprise Manager for example and will find this feature particularly useful in pgAdmin, as will other people I know both at work and in the community. As for repeatable query result timing, I see nothing in the patch that has made the timings non-repeatable, or even for that matter altered the initial query timing at all. > Trying to autosize the column from content doesn't work satisfying. No, and noone has suggested doing so. The headers on the other hand tend to have much more regular lengths. Regards, Dave
Andreas Pflug wrote: > Dave Page wrote: >>> -----Original Message----- >>> From: Andreas Pflug [mailto:pgadmin@pse-consulting.de] Sent: 06 March >>> 2006 09:42 >>> To: Florian G. Pflug >>> Cc: Dave Page; Edward Di Geronimo Jr.; pgadmin-hackers@postgresql.org >>> Subject: Re: [pgadmin-hackers] Query results in grid, new patch >>> >>> Florian G. Pflug wrote: >>> >>> .) Could the initial size of the columns be choosen so, >>> that the captions are not cut off? Or, even better, >>> pgadmin could remember the with of the >>> columns between selects, at least if the captions stay the same... >>> >>> The old control's behaviour was like that. >> >> Yes, it was, but it might be a nice fix. It's bugged me for a while. > > Hu? I meant the control *does* remember column sizing for repeated > execution of similar queries since 1.2!!! This is not a "nice fix", it > is *required*. I don't give a cent for copy/paste from the result (in 10 > years on SQL RDBMS, I might have done that 5 times), I need repeatable > results from query tuning. > Trying to autosize the column from content doesn't work satisfying. I've seem to have never noticed that.. ;-) I guess it's because most times i switch between two or more different queries, and the code doesn't handle that. I just experimented a bith with the latest 1.4 snapshot, and found a bug - it seems like a columns with is only remembered when the column "moves to the right", not when it "moves to the left". Another usefull feature (If it isn't already there, just under my nose ;-) ) would be to have tooltips displaying the full name of a column. I often resize the columns to be just wide enough to show the data, and more often than not, the caption is far longer than that. greetings, Florian Pflug
> -----Original Message----- > From: Florian G. Pflug [mailto:fgp@phlo.org] > Sent: 06 March 2006 03:37 > To: Florian G. Pflug > Cc: Dave Page; Edward Di Geronimo Jr.; pgadmin-hackers@postgresql.org > Subject: Re: [pgadmin-hackers] Query results in grid, new patch > > > I've finally come around to testing the new grid on osx, I must > say that I'm really impressed. It seems much more responsive than > the old one to me, and being able to copy rows, cols and fields > to the clipboard really rockts. I have to rather small > suggestions, though > > .) Ctrl-A (or Cmd-A on OSX) should select the whole table - it's > a rather well established UI-standard I believe to let Ctrl-A/Cmd-A > select the whole "whatever" you are currently editing. Fixed. > .) Could the initial size of the columns be choosen so, that > the captions > are not cut off? Possible todo for anyone that wants to try it! > Or, even better, pgadmin could remember the > with of the columns > between selects, at least if the captions stay the same... Fixed. > .) On OSX, the gray background looks rather strange, white > would be a better choice I believe. That seems to be the default for the grid control on OSX (see the Edit Grid which is also gray). It is white on other platforms though - I'll see if I can find a safe way to change it which won't affect people with 'negative' color schemes. Cheers, Dave.
Dave Page wrote: > > > >> -----Original Message----- >> From: Florian G. Pflug [mailto:fgp@phlo.org] >> Sent: 06 March 2006 03:37 >> To: Florian G. Pflug >> Cc: Dave Page; Edward Di Geronimo Jr.; pgadmin-hackers@postgresql.org >> Subject: Re: [pgadmin-hackers] Query results in grid, new patch >> >> >> I've finally come around to testing the new grid on osx, I must >> say that I'm really impressed. It seems much more responsive than >> the old one to me, and being able to copy rows, cols and fields >> to the clipboard really rockts. I have to rather small >> suggestions, though >> >> .) Ctrl-A (or Cmd-A on OSX) should select the whole table - it's >> a rather well established UI-standard I believe to let Ctrl-A/Cmd-A >> select the whole "whatever" you are currently editing. >> > > Fixed. > > >> .) Could the initial size of the columns be choosen so, that >> the captions >> are not cut off? >> > > Possible todo for anyone that wants to try it! > NOT a todo. I found the result something between annoying and useless when I implemented column sizing preserving, esp. in case of many columns. I still didn't have the time to have a look at the patch, and I will be very unhappy if I find the result a regression because its functions are extended in a direction the control never was intended and designed for. Regards, Andreas
On 6/3/06 19:04, "Andreas Pflug" <pgadmin@pse-consulting.de> wrote: > Dave Page wrote: >> > > NOT a todo. I found the result something between annoying and useless > when I implemented column sizing preserving, esp. in case of many columns. > I still didn't have the time to have a look at the patch, and I will be > very unhappy if I find the result a regression because its functions are > extended in a direction the control never was intended and designed for. If you find any breakages then do report them and I will look at them, but please do not forget that pgAdmin is not designed entirely for your usage patterns - just because you don't copy'n'paste query results doesn't mean there aren't a thousand people that do (or will do). Regards, Dave.
Dave Page wrote: > > On 6/3/06 19:04, "Andreas Pflug" <pgadmin@pse-consulting.de> wrote: > > >> Dave Page wrote: >> >>> >>> >> NOT a todo. I found the result something between annoying and useless >> when I implemented column sizing preserving, esp. in case of many columns. >> I still didn't have the time to have a look at the patch, and I will be >> very unhappy if I find the result a regression because its functions are >> extended in a direction the control never was intended and designed for. >> > > If you find any breakages then do report them and I will look at them, but > please do not forget that pgAdmin is not designed entirely for your usage > patterns - just because you don't copy'n'paste query results doesn't mean > there aren't a thousand people that do (or will do). > We have discussed this previously. Trying to extend the query tool to a multi purpose data manipulating tool is a dead end. Finally, don't forget: My initial impulse to code on pga3 was dissatisfaction with pga2's query tool, so I'm most sensitive on that topic. Regards, Andreas
Andreas Pflug wrote: > Dave Page wrote: >> On 6/3/06 19:04, "Andreas Pflug" <pgadmin@pse-consulting.de> wrote: >>> Dave Page wrote: >>> NOT a todo. I found the result something between annoying and useless >>> when I implemented column sizing preserving, esp. in case of many >>> columns. >>> I still didn't have the time to have a look at the patch, and I will be >>> very unhappy if I find the result a regression because its functions are >>> extended in a direction the control never was intended and designed for. >> If you find any breakages then do report them and I will look at them, >> but >> please do not forget that pgAdmin is not designed entirely for your usage >> patterns - just because you don't copy'n'paste query results doesn't mean >> there aren't a thousand people that do (or will do). > We have discussed this previously. Trying to extend the query tool to a > multi purpose data manipulating tool is a dead end. Finally, don't > forget: My initial impulse to code on pga3 was dissatisfaction with > pga2's query tool, so I'm most sensitive on that topic. Well, but that is what it is used for (as a "multi purpose data manipulation tool"). In the company I work for, nearly all kinds of postgres maintenance is done using pgadmin - from creating databases and schema manipulation, to debugging when some customers believes that some piece of data is wrong. So, I'd say, at least for us the query-window is the most important piece of pgadmin. And, most interesting, it's not big features that people miss in their daily work, it's the small useability things (like copying a over-long value of a textfield into an editor to be able to read it properly). greetings, Florian Pflug
> -----Original Message----- > From: Andreas Pflug [mailto:pgadmin@pse-consulting.de] > Sent: 06 March 2006 22:56 > To: Dave Page > Cc: Florian G. Pflug; Edward Di Geronimo Jr.; > pgadmin-hackers@postgresql.org > Subject: Re: [pgadmin-hackers] Query results in grid, new patch > > > We have discussed this previously. Trying to extend the query > tool to a > multi purpose data manipulating tool is a dead end. We discussed it, but we certainly didn't agree on that point. Besides, I fail to see how speeding up the grid and allowing copy/paste to be more flexible can possibly cause you any problems with checking query results and timing? The original argument was about 2 things: 1) Editting in the grid. Well, that's not gonna happen as we have a specialised tool for that. 2) Using a cursor based query to allow querying of very large data sets. Yes, I can see how that would prevent you (and others) from using the query tool in certain use cases, so I would definitely not advocate such a change *unless* it could be an option that unless selected did not change the current behaviour. > Finally, don't > forget: My initial impulse to code on pga3 was dissatisfaction with > pga2's query tool, so I'm most sensitive on that topic. I haven't forgotten, but I would ask you not to forget that pgAdmin is a community project written for general use and we shall not avoid implementing features that the community might want just because any one of us might not use them. That does not mean we should break things for existing use cases though. Regards, Dave.