Re: Patch: Auto-generate search_path statement for selected schema in query editor - Mailing list pgadmin-hackers
| From | Dave Page |
|---|---|
| Subject | Re: Patch: Auto-generate search_path statement for selected schema in query editor |
| Date | |
| Msg-id | CA+OCxoy_uTfecn-n-Js-X7a0NvLp1rnz8p2MCsz5W1jwGPELpQ@mail.gmail.com Whole thread Raw |
| In response to | Re: Patch: Auto-generate search_path statement for selected schema in query editor (Florian Klaar <flo.klaar@gmx.de>) |
| Responses |
Re: Patch: Auto-generate search_path statement
for selected schema in query editor
(Florian Klaar <flo.klaar@gmx.de>)
Re: Patch: Auto-generate search_path statement for selected schema in query editor (Florian Klaar <flo.klaar@gmx.de>) |
| List | pgadmin-hackers |
Hi
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, Feb 8, 2013 at 7:34 PM, Florian Klaar <flo.klaar@gmx.de> wrote:
-- Hi,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?Why not just do something like:if (obj->GetSchema())sql = wxT("SET search_path TO ") + obj->GetSchema()->GetName();
You're missing something - specifically that GetSchema is declared as a virtual function. That's done so you can do exactly what I'm proposing.
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.
That code was just to illustrate how to get the schema name. I agree you should leave the existing elements of the search path there (sorry to confuse).
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 :-)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.
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.
Well, moving it (or for that matter prepending it) might still break things from the users application's perspective as you note above (that's another reason why this should be optional behaviour). I certainly prefer not to duplicate entries though.
Thought so... there are 5 options I can think of right now: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)
1. Make Sticky SQL and auto-searchpath mutually exclusive in the options dialog.
No, I don't think that's acceptable.
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.
Hmm. That's a possibility I guess. I'm not sure what the implications may be without playing with it though.
2a. When generating the "SET search_path" statement, make it a comment (prepend it with "-- ") if Sticky SQL is activated at the same time.
Another possibility.
2b. Make the Sticky SQL part a comment (wrap it in /* */) if auto-searchpath is enabled at the same time.
I don't think that's acceptable.
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.
Is it? Doesn't CREATE Script get it's contents from a GetSQL() call in most cases?
I wonder if we're actually looking at it the wrong way, and what we really should consider is allowing the user to define a "template" block of SQL that's always added to any new SQL Query windows. That block could include placeholders that are replaced with context-specific values, or GUC variable values, e.g, the user could specify a template of:
SET search_path TO '%%SCHEMA%%, %%GUC:search_path%%'
Which would replace %%SCHEMA%% with the context-specific schema, and %%GUC:search_path%% with the current value of the search_path GUC.
The nice thing about doing it this way is that it can be used for a lot of different purposes - you can solve your problem, another user might have a default of "BEGIN;" to ensure they always run in an explicit transaction block etc, but perhaps more importantly, it saves us having to worry about what Sticky SQL or XXX Script features do, as it becomes an issue for the user to ensure their templates will work correctly in their environment.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
pgadmin-hackers by date: