Re: Lightspeed for frmQuery and other issues.

-----Original Message-----
From: "Andreas Pflug"<pgadmin(at)pse-consulting(dot)de>
Sent: 30/04/06 01:44:21
To: "pgadmin-hackers"<pgadmin-hackers(at)postgresql(dot)org>, "Dave Page"<dpage(at)vale-housing(dot)co(dot)uk>
Subject: Lightspeed for frmQuery and other issues.

> as announced I realized the virtual listview implementation in frmQuery;
> the code became amazingly slim. Since there's no more data retrieval,
> the retrieval time shrinks to precisely 0.00 microseconds which should
> be sufficiently fast to justify omitting this value :-)

Cool, nice work. I assume all the clipboard stuff still works?

> I left grid stuff #ifdef'ed in the source, but it can't work for now.
> When this is reworked, I'd really appreciate if the interface of
> ctlSQLResult isn't altered again (there shouldn't be a need to touch
> frmQuery), and until a different solution is accepted the alternate
> #define USE_LISTVIEW should remain.

If it is fully working, I see no reason to change it again. The only other mod I had in mind was full/multiple row pasting in the edit grid.

> At the same time, I noticed how reporting was added to pgAdmin, and was
> quite horrified. The menu refactoring was done to avoid base class

Had a feeling you would be.

> cluttering, and now it is massively reinvented. Any adding to menu.h is
> plain wrong for frmMain menu, any code adding in events.cpp (beyond
> minor submenu handling, i.e. calling enableSubmenu) too, touching base
> factory classes even more. All reporting stuff should be implemented in
> frmReport, not in pgObject or so. Please do refactor it using factories.

Well the code was modelled as closely as possible on the 'new object' menu code, and given the total lack of comments or other documentaion of the factory code it's not exactly easy to understand either intent or implementation.

Here's (from memory) what was done:

- The new menu was added using the menu factories, per the new object menu.

- menu ids were added in menu.h because multiple object types need to share specific IDs for the same report.

- Each object type, via the base class provides it's own menu, per the new object menu.

- event.cpp has a minimal handler for the menu.

- Each report is produced by the object itself (because that's where the info is, per the main UI views.

- Properties/stats reports etc. Are implemented in pgObject/pgCollection to minimise code duplication.

If there's something wrong with any particular part of that you'll need to explain why, and how you think it should be don in a lot more detail, 'cos as far as I can see I've followed existing design pretty closely.

> BTW, I wonder why the reporting is creating HTML, not XML.

Because XML is good for data exchange, not presentation. You will note that it is XHTML 1.0 Strict though.

> I added a scripting option, which will (for std. objects) call the query
> tool with reengineered script as usually performed with the sticky bit
> set. IMNSHO we can drop the option now, because both behaviours are now
> available at the same time.
Scripting of SELECT, INSERT and UPDATE
> (typing the col names can be
boring) coming soon.

Cool.

Regards, Dave

-----Unmodified Original Message-----
Hi Dave,

as announced I realized the virtual listview implementation in frmQuery;
the code became amazingly slim. Since there's no more data retrieval,
the retrieval time shrinks to precisely 0.00 microseconds which should
be sufficiently fast to justify omitting this value :-)

I left grid stuff #ifdef'ed in the source, but it can't work for now.
When this is reworked, I'd really appreciate if the interface of
ctlSQLResult isn't altered again (there shouldn't be a need to touch
frmQuery), and until a different solution is accepted the alternate
#define USE_LISTVIEW should remain.

At the same time, I noticed how reporting was added to pgAdmin, and was
quite horrified. The menu refactoring was done to avoid base class
cluttering, and now it is massively reinvented. Any adding to menu.h is
plain wrong for frmMain menu, any code adding in events.cpp (beyond
minor submenu handling, i.e. calling enableSubmenu) too, touching base
factory classes even more. All reporting stuff should be implemented in
frmReport, not in pgObject or so. Please do refactor it using factories.
BTW, I wonder why the reporting is creating HTML, not XML.

I added a scripting option, which will (for std. objects) call the query
tool with reengineered script as usually performed with the sticky bit
set. IMNSHO we can drop the option now, because both behaviours are now
available at the same time.
Scripting of SELECT, INSERT and UPDATE (typing the col names can be
boring) coming soon.

Regards,
Andreas