Re: Patch: Add support for execution of jobs on a remote database
| От | Ashesh Vashi |
|---|---|
| Тема | Re: Patch: Add support for execution of jobs on a remote database |
| Дата | |
| Msg-id | 494B69E1.1010800@enterprisedb.com обсуждение исходный текст |
| Ответ на | Re: Patch: Add support for execution of jobs on a remote database ("Dave Page" <dpage@pgadmin.org>) |
| Ответы |
Re: Patch: Add support for execution of jobs on a remote database
|
| Список | pgadmin-hackers |
Hi Dave Page,
Dave Page wrote:
I will do the needful.
I will certainly do it from now onwards.
And hence declared it as a struct instead of class.
But later while implementation, I have introduced some function in it.
I will convert it to a class.
Hence, I did not make any checks for columns exists or not.
If you say, I can go ahead and make the change for checking for column (jstconnstr) exists or not.
Shouldn't we return a text instead of integer x.x.x support?
This might be useful in future release too.
What everybody has to say?
Thanks & Regards,
Ashesh Vashi
Dave Page wrote:
I haven't tested fully at this stage, just eyeball reviewed the code, made sure it builds (on Windows) and checked out the UI. First impression is that it's well thought out and nicely implemented.
Thanks
make sense- The pgTable::HasColumn() static function seems like an unusual fit in that class. Yes, it's a table object, but that class is primarily concerned with representing a table in the treeview. It seems to me that a non-static pgConn::TableHasColumn() function might make more sense, and would probably look cleaner at the call sites.
It is.- The database icon in dlgSelectDatabase seems to be missing (I just get a blank space where it should be).
I will do the needful.
sure- The height of the connection string textbox is inconsistent with all other textboxes. It should be sized as per the other one line text boxes, and the button altered to match it.
Oops I did not know about that :(- Any new headers should be added to include/precomp.h (we all forget to do that!)
I will certainly do it from now onwards.
Earlier, I wanted to use it as a storage for data like user, dbname, host, etc...- In pgAgent, why is connInfo declared as a struct and not a class?
And hence declared it as a struct instead of class.
But later while implementation, I have introduced some function in it.
I will convert it to a class.
In fact in pgagent when you try to get the value from the given pgSet, if that column does not exist in the set, it will return a empty string.- pgAdmin seems to be able to handle connecting to an old-style schema (which is good), but I didn't see any code in pgAgent to do similar. I would suggest we do something like the following, which should allow for future changes:
Hence, I did not make any checks for columns exists or not.
If you say, I can go ahead and make the change for checking for column (jstconnstr) exists or not.
I thought of this options too. :)* Add an SQL function pgagent_schema_version() which returns an int (with a value of 3 for this version - we'll bump the package to 3.0.0).
Shouldn't we return a text instead of integer x.x.x support?
This might be useful in future release too.
What everybody has to say?
sure.* Add a check to pgAgent in (MainLoop()) - upon setup of the primary connection, if pgagent_schema_version() does not exist, then exit with an error asking the user to upgrade the schema.
ok. sounds good.* If it does exist, check the value it returns against MAJOR_VERSION - a pre-processor macro we can set in cmake from CPACK_PACKAGE_VERSION_MAJOR - if it doesn't match, exit with an error telling the user there is a schema version mismatch.
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: http://www.enterprisedb.com
В списке pgadmin-hackers по дате отправления: