Re: "set schema" patch - Mailing list pgadmin-hackers
From | Dave Page |
---|---|
Subject | Re: "set schema" patch |
Date | |
Msg-id | 937d27e10801090325r2e01775u494d26c109305b8c@mail.gmail.com Whole thread Raw |
In response to | Re: "set schema" patch (Guillaume Lelarge <guillaume@lelarge.info>) |
List | pgadmin-hackers |
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.
pgadmin-hackers by date: