Обсуждение: ALTER VIEW
Hi Jean-Michel, I just took a look at the latest ALTER VIEW code. Sorry, but I have a number of problems with this version: 1) Don't create the view twice, create the fake one then rename it - it's less expensive and avoids the potential for the second create to fail. 2) The szOldDefintion stuff can now be removed. 3) The OID and Definition could be retrieved in one operation rather than 2. 4) A class library such as this should _never_ display any user interface. That is the job of the application. 5) The modifications to the SQL property are messy & mainly unnecessary (certainly with #1 above - that just needs bDrop & szViewName). 6) The iLogEvent "Entering..." code should be immediately after the On Error code, not in the middle of the function. 7) The Comment cache is not being invalidated. Regards, Dave.
Le Vendredi 22 Février 2002 23:06, Dave Page a écrit : > 1) Don't create the view twice, create the fake one then rename it - it's > less expensive and avoids the potential for the second create to fail. OK > 2) The szOldDefintion stuff can now be removed. OK > 3) The OID and Definition could be retrieved in one operation rather than > 2. OK > 4) A class library such as this should _never_ display any user interface. > That is the job of the application. OK > 5) The modifications to the SQL property are messy & mainly unnecessary > (certainly with #1 above - that just needs bDrop & szViewName). Will have a look at it tomorrow. I though interesting to write SQL using a single property. This is easier to maintain code. Don't flame me... > 6) The iLogEvent "Entering..." code should be immediately after the On > Error code, not in the middle of the function. OK > 7) The Comment cache is not being invalidated. OK \Later
> -----Original Message----- > From: Jean-Michel POURE [mailto:jm.poure@freesurf.fr] > Sent: 22 February 2002 22:11 > To: Dave Page; pgadmin-hackers@postgresql.org > Subject: Re: ALTER VIEW > > > Le Vendredi 22 Février 2002 23:06, Dave Page a écrit : > > > 5) The modifications to the SQL property are messy & mainly > > unnecessary (certainly with #1 above - that just needs bDrop & > > szViewName). > Will have a look at it tomorrow. I though interesting to > write SQL using a > single property. This is easier to maintain code. Don't flame me... I'm not flaming you :-) That code took hundreds of hours for me to write, and I just want to keep it how I think is right. I'm liable to be overprotective for a while :-) My problem with your code, is not how you chose to write it - you are correct, a single SQL function is easier to maintain, but: - Some of what you wrote is not yet required and for simplicity shouldn't be there - szViewDefinition/Comment - With the create/rename approach, all that is required is the option to drop and an alternate name. These functions are also APIs available to the non-pgAdmin programmer. I want to keep them as clean and intuitive as possible. One other thing I noticed (while I'm flaming you :-p ) can you use 2 char tabs to match the rest of the code please. Cheers, Dave.
Hi Dave, I commited some changes to CVS. Now, I need more info to understand : A. VIEWS > 1) Don't create the view twice, create the fake one then rename it - it's > less expensive and avoids the potential for the second create to fail. Done. > 2) The szOldDefintion stuff can now be removed. Done > 3) The OID and Definition could be retrieved in one operation rather than > 2. Done > 4) A class library such as this should _never_ display any user interface. > That is the job of the application. I agree. Could you outline the code, please? > 5) The modifications to the SQL property are messy & mainly unnecessary > (certainly with #1 above - that just needs bDrop & szViewName). On the converse, I think it is a very powerfull way to write SQL code. Actually, I was going to ask you to apply this kind of modification to all SQL clauses in pgSchema. SQL (bDrop, bCreate, bACL, bComment, ....) is quite straightforward. In the future, we could ask a distinct object to build SQL queries, with batch processing and transactions. This would be very powerfull and would allow the querying of MySQL, MSsql server, etc... OK, let's not go so far, I am a day-dreamer. Also, I would like to point out there should not be any SQL code in Views, Tables, etc.. All should be performed at object level in pgViews, pgTables, pg** using ***precisely the new SQL clause. The less SQL we write by hand, the easier we will maintain pgAdmin2. > 6) The iLogEvent "Entering..." code should be immediately after the On > Error code, not in the middle of the function. Done > 7) The Comment cache is not being invalidated. Why should I invalidate comment cache? The fake view was created with comments and acl. Do I miss something? B. Triggers The new code allows to move a trigger from one table to another. How can I refresh display after modification? By the way, should I CC you or not? pgadmin_hackers is sometimes too slow for discussion. Cheers, Jean-Michel
> -----Original Message----- > From: Jean-Michel POURE [mailto:jm.poure@freesurf.fr] > Sent: 23 February 2002 18:30 > To: Dave Page > Cc: pgadmin-hackers@postgresql.org > Subject: Re: [pgadmin-hackers] ALTER VIEW > > > > > 4) A class library such as this should _never_ display any user > > interface. That is the job of the application. > I agree. Could you outline the code, please? Dunno, all the other code just lets errors from the backend trickle up. > > 5) The modifications to the SQL property are messy & mainly > > unnecessary (certainly with #1 above - that just needs bDrop & > > szViewName). > > On the converse, I think it is a very powerfull way to write > SQL code. > Actually, I was going to ask you to apply this kind of > modification to all > SQL clauses in pgSchema. > > SQL (bDrop, bCreate, bACL, bComment, ....) is quite > straightforward. In the > future, we could ask a distinct object to build SQL queries, > with batch > processing and transactions. Yes, these are fine, but add them when we need them, not just for the sake of it and don't try to modify comments and definitions in this way. If you do, with complex object you will end up with arguments lists that are far too long. For a simple object like a view, I think you had got up to about 8 arguments. > > Also, I would like to point out there should not be any SQL > code in Views, > Tables, etc.. All should be performed at object level in > pgViews, pgTables Then pgSchema will not work at all. You cannot build a collection of object if all the code is in the objects. > > 7) The Comment cache is not being invalidated. > Why should I invalidate comment cache? The fake view was created with > comments and acl. Do I miss something? The entry in the cache will be for the old view oid - there will be none for the new view. You must invalidate the cache so it is repopulated with up to date data the next time *any* comment is accessed. > B. Triggers > The new code allows to move a trigger from one table to > another. How can I > refresh display after modification? Don't know, never done it. You might want to check out how we update object names at present though - that'll show how we locate the relevant node. > By the way, should I CC you or not? pgadmin_hackers is > sometimes too slow for discussion. Marc did some work on the server (Wednesday?) that sped things up no end. I get both copies pretty much together now, so no CC is necessary. If we need to actually chat, then we can always use ICQ or something. Regards, Dave.