Re: "set schema" patch

On 30/12/2007, Guillaume Lelarge <guillaume(at)lelarge(dot)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.