Thread: "set schema" patch
Hi, This patch adds a schema combobox on some objets' dialog : aggregate, conversion, domain, function, sequence, table, type, view. There are two EDB objects that probably need it too : package and synonym. Dave, can you tell me if they are dependants on schema ? and do they use the same SQL statement to alter their schema ? thanks. The patch attached is not finished. I have one big issue with it. It seems I can't alter the schema of a function. The call to ShowObject() in dlgProperty::apply() fails. It seems this statement pgObject *newData=data->Refresh(mainForm->GetBrowser(), item); crashes pgAdmin (core dump), but I have no clue to explain this. It don't even know why it crashes with functions, but works with the other objects. If I can find a solution to this issue, it'll probably solve this second (and last) issue. I've put some of my code in comments in OnOK() function. I wrote this code to refresh the "Schemas" node. It seemed to work fine Thursday morning, but didn't want to work since. Is there something wrong with my way to refresh the "Schemas" node? Is there another way ? Thanks for any tips. Regards. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Attachment
Hi Guillaume
Apologies, but I'm *really* busy right now (with upcoming pgAdmin,
PostgreSQL and EnterpriseDB releases) so haven't yet had a chance to
properly review your patches. I'll answer what I can for now - it I
haven't reviewed within a week or so of the pgAdmin release, please feel
free to give me a verbal kick.
Guillaume Lelarge wrote:
> Hi,
>
> This patch adds a schema combobox on some objets' dialog : aggregate,
> conversion, domain, function, sequence, table, type, view. There are two
> EDB objects that probably need it too : package and synonym. Dave, can
> you tell me if they are dependants on schema ? and do they use the same
> SQL statement to alter their schema ? thanks.
Public Synonyms live at the same logical level as Schemas, so they re
unaffected by this patch.
Packages are modified using CREATE OR REPLACE, which of course won't
allow you to change the schema in itself, but you could create the new
package in the new schema, and then drop the old. The problem with that
(and changing the schema of other objects) is figuring out what nodes
to refresh.
> The patch attached is not finished. I have one big issue with it. It
> seems I can't alter the schema of a function. The call to ShowObject()
> in dlgProperty::apply() fails. It seems this statement
> pgObject *newData=data->Refresh(mainForm->GetBrowser(), item);
> crashes pgAdmin (core dump), but I have no clue to explain this. It
> don't even know why it crashes with functions, but works with the other
> objects.
>
> If I can find a solution to this issue, it'll probably solve this second
> (and last) issue. I've put some of my code in comments in OnOK()
> function. I wrote this code to refresh the "Schemas" node. It seemed to
> work fine Thursday morning, but didn't want to work since. Is there
> something wrong with my way to refresh the "Schemas" node? Is there
> another way ?
Refreshing is a real pita - the Refresh function in the pgObject and
derived classes was originally designed to do exactly that, but these
days has subtly changed such that it no longer works as you need (and
how even I expect in most cases).
We should really add a sure-fire way to refresh the tree as you expect
in SVN-Trunk (or figure out what the heck Refresh() really does these
days). Recently though, I used code like this to refresh the Functions
collection node in the case that the debugger finds a function no longer
exists:
ctlTree *browser = form->GetBrowser();
wxTreeItemId item=browser->GetSelection();
if (obj == browser->GetObject(item))
{
pgCollection *coll=browser->GetParentCollection(obj->GetId());
browser->DeleteChildren(coll->GetId());
coll->ShowTreeDetail(browser);
}
/D
Hi,
Dave Page wrote:
> Apologies, but I'm *really* busy right now (with upcoming pgAdmin,
> PostgreSQL and EnterpriseDB releases) so haven't yet had a chance to
> properly review your patches. I'll answer what I can for now - it I
> haven't reviewed within a week or so of the pgAdmin release, please feel
> free to give me a verbal kick.
>
I completely understand. I was more thinking this was a holidays season,
so I'm not waiting for a fast answer :)
So, no problem at all.
> Guillaume Lelarge wrote:
>> Hi,
>>
>> This patch adds a schema combobox on some objets' dialog : aggregate,
>> conversion, domain, function, sequence, table, type, view. There are two
>> EDB objects that probably need it too : package and synonym. Dave, can
>> you tell me if they are dependants on schema ? and do they use the same
>> SQL statement to alter their schema ? thanks.
>
> Public Synonyms live at the same logical level as Schemas, so they re
> unaffected by this patch.
>
OK.
> Packages are modified using CREATE OR REPLACE, which of course won't
> allow you to change the schema in itself, but you could create the new
> package in the new schema, and then drop the old. The problem with that
> (and changing the schema of other objects) is figuring out what nodes
> to refresh.
>
Yes, that's a big problem.
>> The patch attached is not finished. I have one big issue with it. It
>> seems I can't alter the schema of a function. The call to ShowObject()
>> in dlgProperty::apply() fails. It seems this statement
>> pgObject *newData=data->Refresh(mainForm->GetBrowser(), item);
>> crashes pgAdmin (core dump), but I have no clue to explain this. It
>> don't even know why it crashes with functions, but works with the other
>> objects.
>>
>> If I can find a solution to this issue, it'll probably solve this second
>> (and last) issue. I've put some of my code in comments in OnOK()
>> function. I wrote this code to refresh the "Schemas" node. It seemed to
>> work fine Thursday morning, but didn't want to work since. Is there
>> something wrong with my way to refresh the "Schemas" node? Is there
>> another way ?
>
> Refreshing is a real pita - the Refresh function in the pgObject and
> derived classes was originally designed to do exactly that, but these
> days has subtly changed such that it no longer works as you need (and
> how even I expect in most cases).
>
> We should really add a sure-fire way to refresh the tree as you expect
> in SVN-Trunk (or figure out what the heck Refresh() really does these
> days). Recently though, I used code like this to refresh the Functions
> collection node in the case that the debugger finds a function no longer
> exists:
>
> ctlTree *browser = form->GetBrowser();
> wxTreeItemId item=browser->GetSelection();
> if (obj == browser->GetObject(item))
> {
> pgCollection *coll=browser->GetParentCollection(obj->GetId());
> browser->DeleteChildren(coll->GetId());
> coll->ShowTreeDetail(browser);
> }
>
OK, I'll try to have a better understanding of the Refresh function.
I'll look at this in the next few days.
Thanks.
--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com
On 30/12/2007, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> >> This patch adds a schema combobox on some objets' dialog : aggregate,
> >> conversion, domain, function, sequence, table, type, view.
OK, some more detailed feedback on this
In dlgProperty::AddSchemas, you need to filter out temp/system
schemas, and honour any schema restrictions defined for the database.
> >> The patch attached is not finished. I have one big issue with it. It
> >> seems I can't alter the schema of a function. The call to ShowObject()
> >> in dlgProperty::apply() fails. It seems this statement
> >> pgObject *newData=data->Refresh(mainForm->GetBrowser(), item);
> >> crashes pgAdmin (core dump), but I have no clue to explain this. It
> >> don't even know why it crashes with functions, but works with the other
> >> objects.
That happens for a couple of reasons:
1) When pgFunction::Refresh calls pgFunctionFactory::AppendFunctions,
it passes NULL in the browser param which then gets used by
pgSchemaObject::UpdateSchema which is called by
pgFunctionFactory::AppendFunctions.
2) The error doesn't normally occur because UpdateSchema only really
does anything if the schema OID isn't what is expected.
Now a good question is 'why do we pass NULL in the browser param, when
we have the real value in that function anyway', to which my answer is
that when it is null, the Create/AppendObject functions know that
they're doing an update and don't bother to add a new node to the tree
or expect to find more than one record to update.
To fix that (correctly and consistently), I think you'll need to
change all the Refresh functions to pass the browser parameter
properly, and add an extra parameter to the Create/AppendObject
functions to tell them they're only refereshing a single object. It
could be done just for functions, but I think we need to keep it
consistent.
> > ctlTree *browser = form->GetBrowser();
> > wxTreeItemId item=browser->GetSelection();
> > if (obj == browser->GetObject(item))
> > {
> > pgCollection *coll=browser->GetParentCollection(obj->GetId());
> > browser->DeleteChildren(coll->GetId());
> > coll->ShowTreeDetail(browser);
> > }
> >
>
> OK, I'll try to have a better understanding of the Refresh function.
> I'll look at this in the next few days.
The other thing this needs is a way to refresh the target schema as
well as the source. I think we need two things.
1) A reliable way to fully refresh a given node (thought: why can't we
just use frmMain::Refresh ?)
2) A way to reliably find the target schema. It seems to me that
pgSchemaObject::UpdateSchema might provide a suitable starting point -
though in the long term, perhaps we need a 'node map' in which we can
store pointers to all nodes, and provide a simple API for searching by
server, database, schema, table etc.
Regards, Dave.