Re: Patch: Auto-generate search_path statement for selected schema in query editor - Mailing list pgadmin-hackers

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