Обсуждение: More refresh issues

Hi developers!

Testing the pgAdmin III 1.8.0  Beta 4 (Sep 4 2007, rev: 6608:6609).
Client Win XP, host: Debian Etch / PG 8.2.4 and Debian Sarge / PG 8.1.8

When making changes via properties dialog, the new policy is to update
_all_ affected places in the object tree. Did I get that right?

There are some more instances, where this is not observed:
 - Changing trigger functions in the table subtree does not update the
twin instance of the object in the "Trigger Functions" collection.
 - Changing / Dropping UNIQUE constraints does not update the table
node. (This worked in the last beta. Though adding comments works now!)
   Could not test this for adding a new UNIQUE constraints due to the
crash I reported earlier.
 - Adding / Changing / Dropping rules does not update the table node.

Losing the focus every time a father / grandfather node is updated is a
rather unwelcome side effect (we talked about it). In the case of a
large tree the whole tree is moved up and down and, more often than not,
the focus wanders out of sight. As far as I am concerned, this is way
more annoying than all of the above.

Other quirks:
- When hitting <DEL> on a collection in a table subtree, the focus jumps
back to the table node (although the "drop" icon in the menu bar is
disabled for collections). Looks like it it trying to refresh the table
node. It should do nothing, like it does on collection nodes outside the
table subtree.
- You can also hit <DEL> on the trigger function of a trigger in the
table subtree, which tries to drop it. But this can never succeed, as
there is a depending object per definition (the trigger). We might as
well disable dropping here.


Regards
Erwin

dpage@postgresql.org wrote:
> Erwin Brandstetter wrote:
>
> (...)
>> Losing the focus every time a father / grandfather node is updated is a
>> rather unwelcome side effect (we talked about it). In the case of a
>> large tree the whole tree is moved up and down and, more often than not,
>> the focus wanders out of sight. As far as I am concerned, this is way
>> more annoying than all of the above.
>>
>
> Fixed :-)
>

You make my day!  :)

(...)
> I've uploaded a binary to
> http://developer.pgadmin.org/~dpage/pgadmin3.zip - would appreciate it
> if you could check I haven't inadvertently broken anything else!!

Gotta go soon, but I will look at it tonight.


Regards
Erwin

Erwin Brandstetter wrote:
> Hi developers!
>
> Testing the pgAdmin III 1.8.0  Beta 4 (Sep 4 2007, rev: 6608:6609).
> Client Win XP, host: Debian Etch / PG 8.2.4 and Debian Sarge / PG 8.1.8
>
> When making changes via properties dialog, the new policy is to update
> _all_ affected places in the object tree. Did I get that right?
>
> There are some more instances, where this is not observed:
> - Changing trigger functions in the table subtree does not update the
> twin instance of the object in the "Trigger Functions" collection.

Yeah, that ones a real pita, and probably not worth the code wart it
would need to fix it. The whole Function under Trigger thing is a kludge
imho :-(

> - Changing / Dropping UNIQUE constraints does not update the table node.
> (This worked in the last beta. Though adding comments works now!)
>   Could not test this for adding a new UNIQUE constraints due to the
> crash I reported earlier.

Fixed.

> - Adding / Changing / Dropping rules does not update the table node.

Fixed.

> Losing the focus every time a father / grandfather node is updated is a
> rather unwelcome side effect (we talked about it). In the case of a
> large tree the whole tree is moved up and down and, more often than not,
> the focus wanders out of sight. As far as I am concerned, this is way
> more annoying than all of the above.

Fixed :-)

> Other quirks:
> - When hitting <DEL> on a collection in a table subtree, the focus jumps
> back to the table node (although the "drop" icon in the menu bar is
> disabled for collections). Looks like it it trying to refresh the table
> node. It should do nothing, like it does on collection nodes outside the
> table subtree.

Fixed.

> - You can also hit <DEL> on the trigger function of a trigger in the
> table subtree, which tries to drop it. But this can never succeed, as
> there is a depending object per definition (the trigger). We might as
> well disable dropping here.

See previous comments about code warts :-(

I've uploaded a binary to
http://developer.pgadmin.org/~dpage/pgadmin3.zip - would appreciate it
if you could check I haven't inadvertently broken anything else!!

Thanks,Dave

Hi Dave!

Testing the pgAdmin III 1.8.0  Beta 4 (Sep 6 2007, rev: 6608:6622).
Client Win XP, host: Debian Etch / PG 8.2.4 and Debian Sarge / PG 8.1.8

I have tested every item of the preceding mail. For brevity I left out
what's working. Once again, you only the nagging. :)


dpage@postgresql.org wrote:
> Erwin Brandstetter wrote:
>
>> (...)
>> There are some more instances, where this is not observed:
>> - Changing trigger functions in the table subtree does not update the
>> twin instance of the object in the "Trigger Functions" collection.
>>
>
> Yeah, that ones a real pita, and probably not worth the code wart it
> would need to fix it. The whole Function under Trigger thing is a kludge
> imho :-(
>

It IS convenient to have the trigger function right there under the
trigger node, though. Makes managing triggers a lot more convenient.


>> Losing the focus every time a father / grandfather node is updated is a
>> rather unwelcome side effect (we talked about it). In the case of a
>> large tree the whole tree is moved up and down and, more often than not,
>> the focus wanders out of sight. As far as I am concerned, this is way
>> more annoying than all of the above.
>>
>
> Fixed :-)
>

MUCH better now! The tree is also preserved on a manual refresh, which
should make Raymond O'Donnell happy, amongst others. :)
There is still room for improvement, but these are just minor wishlist
items now:
 - After deleting an object in the table subtree, the focus jumps back
to the table node. Ideally, the focus would go to the the nearest
brother object or father node.
 - After creating a new object, the focus would end up on the new object
in an ideal world.
 - When refreshing the database collection, all databases except the
maintainance db are closed, and the trees collapsed. Is this necessary
for some reason?
 - The tree is a bit hyperactive at times (going up / down, blanking
out), before it settles after changes - especially after deleting
objects. That's really a small thing, though.


>> - You can also hit <DEL> on the trigger function of a trigger in the
>> table subtree, which tries to drop it. But this can never succeed, as
>> there is a depending object per definition (the trigger). We might as
>> well disable dropping here.
>>
>
> See previous comments about code warts :-(
>

Well, it is not important. Nothing bad can happen here.



New issues:
~~~~~~~~

 - Dropping constraints by pressing <DEL> fails, because the dot between
schema and table name is missing in the executed SQL (SQL pane is
correct, though!):
    ALTER TABLE myschemamytable DROP CONSTRAINT xxx;
Should be:
    ALTER TABLE myschema.mytable DROP CONSTRAINT xxx;
Affects check and foreign key constraints.


- Creating a constraint per dialog offers a checkbox "Deferrable". This,
however, is only applicable for foreign key constraints. With all other
types it produces a syntax error if you use it:
    ALTER TABLE cpflat.cpkat ADD CONSTRAINT xxx UNIQUE (nummer, katcode)
DEFERRABLE INITIALLY IMMEDIATE;
http://www.postgresql.org/docs/8.2/interactive/sql-createtable.html says:
>
> DEFERRABLE
> (...) Only foreign key constraints currently accept this clause. All
> other constraint types are not deferrable.
>


- More refresh issues:
 - After renaming a SEQUENCE, connected tables are out of date.
 - After renaming a TABLESPACE, all indexes, tables, databases, primary
key and unique constraints using are out of date.
    This actually can have severe side effects. After renaming a
tablespace, the combobox "Tablespace" in the properties dialog already
uses the new name, but the table still has the old one, so the display
defaults to "pg_default". If you use the properties dialog to do
anything in such a situation, the table is silently moved to
"pg_default". Evil, evil.
Maybe it would be a good strategy to perform a refresh on objects right
before opening the properties dialog? This would certainly take care of
the above situation.
I think we have identified most things now, but there may be more. This
is a bit of a can of worms. :(


- A related case:
When you refresh any part of the tree manually, changes may turn up,
while other objects, which are also affected, keep their obsolete status.
If you have a way to probe for changes, this might be easy to catch.
Otherwise it may be tricky / expensive. For instance ..
   .. if you refresh the sequence collection, in theory, you would have
to refresh all table objects connected with a sequence.
   .. if you refresh any part of the table subtree, you would always
have to refresh the table as well. (This should be easy, actually.)
   .. if you refresh the Trigger Functions collections -> all tables
with triggers.
   .. more cases
This is an even bigger can of worms. It is probably better to just leave
it to the user and not try to cascade manual refreshes.


Regards
Erwin

Erwin Brandstetter wrote:
> - After deleting an object in the table subtree, the focus jumps back to
> the table node. Ideally, the focus would go to the the nearest brother
> object or father node.

OK, now jumps to the parent.

> - After creating a new object, the focus would end up on the new object
> in an ideal world.

No straightforward way to change that as far as I can see. No-one has
ever complained though so I'm not going to bust a gut to change that.

> - When refreshing the database collection, all databases except the
> maintainance db are closed, and the trees collapsed. Is this necessary
> for some reason?

The way the code is currently architected, refreshing a database will
not re-open its connection. Again, I'm not going to worry about that now
as it's always been that way (yes, it's slightly inconsistent with other
refreshes, but then it's obvious that that node is different anyway).

> - The tree is a bit hyperactive at times (going up / down, blanking
> out), before it settles after changes - especially after deleting
> objects. That's really a small thing, though.

In some situations we were refreshing the node and it's parent
collection, and then the grandparent. I've changed it so that if we're
going to refresh the grandparent, we don't bother explicitly doing the
children/grandchildren first.

> - Dropping constraints by pressing <DEL> fails, because the dot between
> schema and table name is missing in the executed SQL (SQL pane is
> correct, though!):
>    ALTER TABLE myschemamytable DROP CONSTRAINT xxx;
> Should be:
>    ALTER TABLE myschema.mytable DROP CONSTRAINT xxx;
> Affects check and foreign key constraints.

Fixed.

> - Creating a constraint per dialog offers a checkbox "Deferrable". This,
> however, is only applicable for foreign key constraints. With all other
> types it produces a syntax error if you use it:
>    ALTER TABLE cpflat.cpkat ADD CONSTRAINT xxx UNIQUE (nummer, katcode)
> DEFERRABLE INITIALLY IMMEDIATE;
> http://www.postgresql.org/docs/8.2/interactive/sql-createtable.html says:
>>
>> DEFERRABLE
>> (...) Only foreign key constraints currently accept this clause. All
>> other constraint types are not deferrable.

Wierd. Wonder why they were included. Oh well, removed now.

> - More refresh issues:
> - After renaming a SEQUENCE, connected tables are out of date.

I think that'll have to stay as it is for now. Refreshing all the tables
in the database (because it might affect tables in more than one schema
of course) could be *extremely* expensive. I've seen extreme cases where
it could take a minute or more to build a tables tree as it is :-(

> - After renaming a TABLESPACE, all indexes, tables, databases, primary
> key and unique constraints using are out of date.
>    This actually can have severe side effects. After renaming a
> tablespace, the combobox "Tablespace" in the properties dialog already
> uses the new name, but the table still has the old one, so the display
> defaults to "pg_default". If you use the properties dialog to do
> anything in such a situation, the table is silently moved to
> "pg_default". Evil, evil.
> Maybe it would be a good strategy to perform a refresh on objects right
> before opening the properties dialog? This would certainly take care of
> the above situation.
> I think we have identified most things now, but there may be more. This
> is a bit of a can of worms. :(

I was able to modify things here to track each object's tablespace by
OID rather than name. It's still possible to rename a tablespace under
an open properties dialogue which results in an error, but that applies
to all sorts of other things as well. In any case, it's predictable and
well behaved now.

> - A related case:
> When you refresh any part of the tree manually, changes may turn up,
> while other objects, which are also affected, keep their obsolete status.
> If you have a way to probe for changes, this might be easy to catch.
> Otherwise it may be tricky / expensive. For instance ..
>   .. if you refresh the sequence collection, in theory, you would have
> to refresh all table objects connected with a sequence.
>   .. if you refresh any part of the table subtree, you would always have
> to refresh the table as well. (This should be easy, actually.)
>   .. if you refresh the Trigger Functions collections -> all tables with
> triggers.
>   .. more cases
> This is an even bigger can of worms. It is probably better to just leave
> it to the user and not try to cascade manual refreshes.

I'm not convinced there's any sensible way of fixing these issue unless
we switch to a 'refresh-on-select' type design. That would actually
allow us to get rid of a fair bit of the code I've written over the last
few days I imagine, but would be far less bandwidth friendly.

Anyhoo, thanks for the bug reports; very precise as usual. We're
starting to run short on time now so I'm going to call a halt to all
further behavioural fixes for this release *except* for blatant errors
or regressions.

Usual update for testing is at
http://developer.pgadmin.org/~dpage/pgadmin3.zip. Please note that this
was built on my home PC which has a slightly different environment than
my normal machine. IF you get any dependency errors, let me know and
I'll whip the laptop out.

/D

dpage@postgresql.org wrote:
> Erwin Brandstetter wrote:
>> - After deleting an object in the table subtree, the focus jumps back
>> to the table node. Ideally, the focus would go to the the nearest
>> brother object or father node.
>
> OK, now jumps to the parent.

That doesn't seem to work in the table subtree, yet. :/
After deleting ...  --> focus goes  to ...
... table column  --> a random collection node in this table subtree.
... a check or fk constraint --> a unique constraint
... a unique constraint --> the pk constraint
... a trigger --> a table column
... an index --> the colums collection
Other results with other tables. Behavior seems random. In pg 8.1 and pg
8.2 alike.

Also, if deletion fails (because of fk constraint or something) the
focus will still wander. Could this be constricted to successful deletion?
This is not constricted to the table subtree.


(...)
>> - More refresh issues:
>> - After renaming a SEQUENCE, connected tables are out of date.
>
> I think that'll have to stay as it is for now. Refreshing all the
> tables in the database (because it might affect tables in more than
> one schema of course) could be *extremely* expensive. I've seen
> extreme cases where it could take a minute or more to build a tables
> tree as it is :-(

Well, renaming sequences should be a pretty rare thing, not really worth
the trouble. I was only covering it for the sake of completeness.


>> - After renaming a TABLESPACE, all indexes, tables, databases,
>> primary key and unique constraints using are out of date.
>>    This actually can have severe side effects. After renaming a
>> tablespace, the combobox "Tablespace" in the properties dialog
>> already uses the new name, but the table still has the old one, so
>> the display defaults to "pg_default". If you use the properties
>> dialog to do anything in such a situation, the table is silently
>> moved to "pg_default". Evil, evil.
>> Maybe it would be a good strategy to perform a refresh on objects
>> right before opening the properties dialog? This would certainly take
>> care of the above situation.
>> I think we have identified most things now, but there may be more.
>> This is a bit of a can of worms. :(
>
> I was able to modify things here to track each object's tablespace by
> OID rather than name. It's still possible to rename a tablespace under
> an open properties dialogue which results in an error, but that
> applies to all sorts of other things as well. In any case, it's
> predictable and well behaved now.


It was the silent side effect that made me uneasy. If people get an
error for these rare cases, that should be just fine. Nothing lost,
nothing broken. So I consider this a proper fix.


>> - A related case:
>> When you refresh any part of the tree manually, changes may turn up,
>> while other objects, which are also affected, keep their obsolete
>> status.
>> If you have a way to probe for changes, this might be easy to catch.
>> Otherwise it may be tricky / expensive. For instance ..
>>   .. if you refresh the sequence collection, in theory, you would
>> have to refresh all table objects connected with a sequence.
>>   .. if you refresh any part of the table subtree, you would always
>> have to refresh the table as well. (This should be easy, actually.)
>>   .. if you refresh the Trigger Functions collections -> all tables
>> with triggers.
>>   .. more cases
>> This is an even bigger can of worms. It is probably better to just
>> leave it to the user and not try to cascade manual refreshes.
>
> I'm not convinced there's any sensible way of fixing these issue
> unless we switch to a 'refresh-on-select' type design. That would
> actually allow us to get rid of a fair bit of the code I've written
> over the last few days I imagine, but would be far less bandwidth
> friendly.

Bandwidth friendly is a good thing. And as there is no sure & easy way
to cascade refreshes, leave it to the user.

To sum it up, the policy on refreshing objects in the tree is now:
pgAdmin updates all (well, almost) affected objects after changes made
per dialog. This does not cover changes via manual SQL or from other
users or other clients. I would generally recommend to refresh objects
before working on them in such environments.
Might be worth a note in the help file somewhere.


> Anyhoo, thanks for the bug reports; very precise as usual. We're
> starting to run short on time now so I'm going to call a halt to all
> further behavioural fixes for this release *except* for blatant errors
> or regressions.

I'd say, we have come a long way. :)
Working with object tree and dialogs has become a lot more usable,
reliable and comfortable over the last couple of days. :)


> Usual update for testing is at
> http://developer.pgadmin.org/~dpage/pgadmin3.zip. Please note that
> this was built on my home PC which has a slightly different
> environment than my normal machine. IF you get any dependency errors,
> let me know and I'll whip the laptop out.

Didn't run into any problems.


Regards
Erwin

Erwin Brandstetter wrote:
> That doesn't seem to work in the table subtree, yet. :/
> After deleting ...  --> focus goes  to ...
> ... table column  --> a random collection node in this table subtree.
> ... a check or fk constraint --> a unique constraint
> ... a unique constraint --> the pk constraint
> ... a trigger --> a table column
> ... an index --> the colums collection
> Other results with other tables. Behavior seems random. In pg 8.1 and pg
> 8.2 alike.

Gah, stupid error - and pure chance it worked OK when I tested it!!
Fixed in SVN.

> Also, if deletion fails (because of fk constraint or something) the
> focus will still wander. Could this be constricted to successful deletion?
> This is not constricted to the table subtree.

Also fixed.

> To sum it up, the policy on refreshing objects in the tree is now:
> pgAdmin updates all (well, almost) affected objects after changes made
> per dialog. This does not cover changes via manual SQL or from other
> users or other clients. I would generally recommend to refresh objects
> before working on them in such environments.
> Might be worth a note in the help file somewhere.

Patches are welcome :-)

Cheers, Dave.

dpage@postgresql.org wrote:
> Erwin Brandstetter wrote:
>
> (...)
>> To sum it up, the policy on refreshing objects in the tree is now:
>> pgAdmin updates all (well, almost) affected objects after changes made
>> per dialog. This does not cover changes via manual SQL or from other
>> users or other clients. I would generally recommend to refresh objects
>> before working on them in such environments.
>> Might be worth a note in the help file somewhere.
>>
>
> Patches are welcome :-)
Not sure what the preferred form for that patch is, as I have not messed
with source code yet.

I would add a paragraph to
    /trunk/pgadmin3/docs/en_US/main.html
before the line "<H3>Getting started</H3>":

<P>pgAdmin is bandwidth friendly. The status of objects in the browser
is only refreshed on request or after changes made with the built-in
tools. Be aware that this does not cover changes made via manual SQL or
from other users or other clients. It is generally advisable to refresh
objects before working on them in such environments.</P>

We could add more detail, but I tried to be brief. Also, I am sure you
are aware that I am not a native English speaker.


Regards
Erwin

Erwin Brandstetter wrote:
> dpage@postgresql.org wrote:
>> Erwin Brandstetter wrote:
>>   (...)
>>> To sum it up, the policy on refreshing objects in the tree is now:
>>> pgAdmin updates all (well, almost) affected objects after changes made
>>> per dialog. This does not cover changes via manual SQL or from other
>>> users or other clients. I would generally recommend to refresh objects
>>> before working on them in such environments.
>>> Might be worth a note in the help file somewhere.
>>>
>>
>> Patches are welcome :-)
> Not sure what the preferred form for that patch is, as I have not messed
> with source code yet.

diff -u works for me (svn doesn't seem to do -c which is unfortunate).
But hey, whatever the format, I got some code out of Erwin - today is a
*good* day :-)

> I would add a paragraph to
>    /trunk/pgadmin3/docs/en_US/main.html
> before the line "<H3>Getting started</H3>":
>
> <P>pgAdmin is bandwidth friendly. The status of objects in the browser
> is only refreshed on request or after changes made with the built-in
> tools. Be aware that this does not cover changes made via manual SQL or
> from other users or other clients. It is generally advisable to refresh
> objects before working on them in such environments.</P>

Committed - thanks.

> We could add more detail, but I tried to be brief. Also, I am sure you
> are aware that I am not a native English speaker.

Not that you'd notice :-)

Thanks, Dave

dpage@postgresql.org wrote
>
> diff -u works for me (svn doesn't seem to do -c which is unfortunate).
> But hey, whatever the format, I got some code out of Erwin - today is a
> *good* day :-)
>

Hey, you are forgetting that other occasion when I sent in a line for
the help file on query options! ;)


>> We could add more detail, but I tried to be brief. Also, I am sure you
>> are aware that I am not a native English speaker.
>>
>
> Not that you'd notice :-)

Well, thanks. :) But It might still be a good idea to be wary of my
Austrian English style.


Regards
Erwin

dpage@postgresql.org wrote:
> diff -u works for me (svn doesn't seem to do -c which is unfortunate).
>

To check if I got that right: the preferred form for the patch would
have been this:
(Created with diff -u (GNU diffutils) 2.8.1 on the command line,
revision added manually.)

--- main.html (revision 3584)
+++ main.html
@@ -33,6 +33,11 @@
 <P>You can resize the main window, and change the sizes of the three
 main regions as you prefer. These adjustments will be preserved when
 you exit the program.</P>
+<P>pgAdmin is bandwidth friendly. The status of objects in the browser
+is only refreshed on request or after changes made with the built-in
+tools. Be aware that this does not cover changes made via manual SQL or
+from other users or other clients. It is generally advisable to refresh
+objects before working on them in such environments.</P>
 <H3>Getting started</H3>
 <P>After you have added the desired server(s) to the tree on the left
 side using the <A HREF="connect.html">Add server</A> menu or toolbar



Regards
Erwin

> ------- Original Message -------
> From: Erwin Brandstetter <brandstetter@falter.at>
> To: dpage@postgresql.org
> Sent: 10/09/07, 17:07:27
> Subject: Re: [pgadmin-hackers] More refresh issues
>
> dpage@postgresql.org wrote:
> > diff -u works for me (svn doesn't seem to do -c which is unfortunate).
> >
>
> To check if I got that right: the preferred form for the patch would
> have been this:
> (Created with diff -u (GNU diffutils) 2.8.1 on the command line,
> revision added manually.)

That works. It might be easier get the source from the svn repo, then just use svn diff -u. Saves mucking about with
copiesof files and helps you work with the latest code revs. 

/D

> ------- Original Message -------
> From: Erwin Brandstetter <brandstetter@falter.at>
> To: dpage@postgresql.org
> Sent: 10/09/07, 16:51:35
> Subject: Re: [pgadmin-hackers] More refresh issues
>
> Well, thanks. :) But It might still be a good idea to be wary of my
> Austrian English style.

<shrug>

Much of the documentation was written by a German guy...

/D