Re: Patch: Auto-generate search_path statement for selected schema in query editor

Hi,

> Why not just do something like:
>
> if (obj->GetSchema())
> sql = wxT("SET search_path TO ") + obj->GetSchema()->GetName();
1. Unless I'm missing something here, obj->GetSchema() will never be
non-zero because obj is of type pgObject and pgObject's implementation
of GetSchema() always returns zero. Thus, obj has to be casted to a
different type before we can call GetSchema(). And since I couldn't find
a common ancestor implementing GetSchema() for all object types, I
resorted to the dynamic_cast approach. Come to think of it, might it be
worthwile to move this logic into pgObject's GetSchema() implementation?
2. Setting the search_path just to the schema in question will cause any
reference to an object contained in the public or $user schema (which
are comprising the search_path by default) to fail. That was my main
concern. Also, a user's search_path may be configured to contain other
schemas that are needed for application-specific queries. So in order
not to break expected behaviour, my goal was to preserve the original
search_path and just complement it.

> Shouldn't you just check that it's not at the front of the search
> path? Otherwise, if it's further back then queries might still be
> directed to a different check.
Good point. But just comparing the first element could lead to results
like [public,foobar,public] - call me a pedant, but I wouldn't like that :-)
How about something like: if schemaName is not contained in search_path
then prepend it; else if schemaName is already contained in search_path,
but not at the front, then move it to the front. Or, in other words: if
schemaName is contained in search_path, then remove it; afterwards,
always prepend schemaName to search_path.

> There may occur usability
> problems in combination with the existing "sticky SQL" option
> though. We
> don't use the "sticky SQL" feature in our environment, so for now I
> didn't spend too much thought on it.
>
>
> It's essential the patch works with that, if it's to have any hope of
> being committed.
Ok.

> I definitely think it needs to be a configurable option - though, I
> wonder how it would work alongside Sticky SQL. That just copies the
> SQL from the SQL pane into the query tool - but, that may have schemas
> in it. If the search path is set, we almost certainly wouldn't want
> that (related to that, are the various "XXXX Script" options on each
> object, which have a similar issue)
Thought so... there are 5 options I can think of right now:
1. Make Sticky SQL and auto-searchpath mutually exclusive in the options
dialog.
2. Leave the choice up to the user; if the user activates both options
at the same time, then be it. The user can still manually delete those
parts she doesn't want.
2a. When generating the "SET search_path" statement, make it a comment
(prepend it with "-- ") if Sticky SQL is activated at the same time.
2b. Make the Sticky SQL part a comment (wrap it in /* */) if
auto-searchpath is enabled at the same time.
3. Make 2 & 2a & 2b configurable according to the user's personal
preference :-)

As for the "XXXX script" feature, that's taking a different route in the
code, so currently it doesn't collide with this search_path thing anyway.

Regards,
Florian