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

From: Florian Klaar <flo(dot)klaar(at)gmx(dot)de>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Patch: Auto-generate search_path statement for selected schema in query editor
Date: 2013-02-11 16:38:32
Message-ID: 51191E88.6020001@gmx.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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?
>
>
> 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.
Hm, okay. It didn't work out when I tested it last week, but I'll have
another go at it this evening if I find the time. Maybe I screwed up
somewhere else when I tested it.


>
> 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.
Well as you already stated, the schema name is always (?) given
explicitly in the Sticky SQL code. But one thing I could come up with
was function results as default values in CREATE TABLE statements; these
apparently aren't automatically prepended with the schema name if they
are located in the public schema and the user's search_path contains the
public schema as well. E.g. this is code generated by pgAdmin with the
user's default search set to [public]:

CREATE TABLE myschema.test_table
(
my_id integer DEFAULT generate_my_id()
)
WITH (
OIDS=FALSE
);

The default search_path is [public] - hence the public schema is not
mentioned in the column default definition. Now let's assume that upon
opening the editor window, the search_path is set to [myschema,public]
instead and a function named generate_my_id() exists in both public and
myschema. The CREATE statement will then use myschema.generate_my_id()
as the default value instead of public.generate_my_id().
Which IMO is perfectly valid (it's just doing as it's told); the more
interesting question is whether the user will be aware of that.

> 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 don't have the code at hand right now so I can't provide any details;
but my code as well as the Sticky SQL part is apparently only executed
when the user opens the SQL Editor using the toolbar button or the main
menu, according to my test runs and from what I could tell looking at
the code.
When opening the window using the toolbar button or the main menu, a
factory method is used for creating the query editor, which is where my
code is at. The various script commands from the menu are taking another
route, it seems.

> 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.
That'd be a nice enhancement indeed. But using your example with the
search_path, we'd still have the inconvenience of a redundant
[public,myschema,public] path resulting from the template. Which OTOH is
just a matter of taste (or is it?).

BTW, after adapting my code for re-ordering the search_path like we
discussed and making it a configurable option (which was really easy,
actually), I tested another unrelated idea: executing a query in the
query editor repeatedly in user-definable intervals. I intended this
primarily for monitoring tasks (pgAdmin's "server status" feature is
great, but sometimes you just have more specific demands) or for
watching logging tables being populated by an external application. This
comes with a few UI-related problems though... all input fields should
be made read-only while the timer is running, and the query editor for
some reason is always brought to the foreground everytime the query has
completed, which can be annoying. Also, certain menu items in the query
editor should be disabled when the timer is running but the statement is
not currently executed.
If this feature might be of broader interest, I'd be willing to look
into some of these issues.
Oh, and when testing this, I found a small bug: open query editor, click
into the SQL notepad, then click on the GQB tab... pgAdmin will crash. I
haven't traced this down yet, just noticed it's reproduceable.

Regards,
Florian

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Florian Klaar 2013-02-11 20:22:27 Re: Patch: Auto-generate search_path statement for selected schema in query editor
Previous Message Dave Page 2013-02-11 11:47:09 Re: Patch: Auto-generate search_path statement for selected schema in query editor