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

[Florian Klaar]

Hi all,

Recently I wrote a small patch for frmQuery.cpp in order to
auto-generate a "SET search_path TO ..." statement when opening the
query editor with a schema-related object selected in the object
browser, e.g. a table, function or sequence (or their respective
collections).
The reason being that in our company, after migrating from MSSQL Server
to pg, we use a single database for many of our customers, with one
schema per customer and identically named objects within those schemas.
E.g. cust_schema_01.table_x, cust_schema_02.table_x, cust_schema_03.table_x.
For maintenance and debugging, we connect to the database using the
postgres superuser account in pgAdmin3. Now in order to access table_x
within a certain customer's schema in the query editor, we always have
to prepend the schema name to the table name or issue a "SET search_path
TO cust_schema_nn,public" statement.
This is rather tedious, so I came up with a patch for pgAdmin3 that
tries to intelligently generate a search_path depending on the currently
selected object in the object browser as well as depending on the
existing search_path configured for the current database connection.
That way, we can easily open query editors under different schemas
without bothering about the search_path ourselves.

This is what my code does when opening a new query editor window:
- Check whether the currently selected object in the object browser is
of type pgSchema, pgSchemaObject or pgSchemaObjCollection or one of
their descendants which (if I'm not mistaken) means it does have a
schema associated with it.
- If so, it checks whether the schema belonging to this object is
already contained in the user's search_path (case-sensitively and
considering the $user placeholder).
- If the schema isn't already in the user's search_path, the code
generates a "SET search_path TO
<selected_schema>,<existing_search_path>" statement and has it written
into the newly opened query editor window.
- After that, it places the cursor to the end of the sql text so the
user can begin typing right away.

Example: the user's search_path is set via ALTER ROLE to
"foobar,public". Now the user selects a single table in the object
browser underneath the schema "cust_schema_03" and opens a new query
editor window. pgAdmin will now pre-fill the editor's input field with
"SET search_path TO cust_schema_03,foobar,public;" and place the cursor
two lines beneath that statement.

Tested on Windows XP Pro SP3 and Windows7 Pro only - I didn't bother to
create a build environment on my Linux box yet.
In case you deem this patch useful, find the diff output based on the
1.16.1 release source code attached below. 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.
Beware also that though being a developer, I'm really inexperienced in
C/C++ and completely new to wxWidgets and to the inner workings of
pgAdmin, so there may well be room for improvement in my code. Having
said that, I'd be willing to dig further into the pgAdmin3 code in order
to make this a configurable option, integrate it better into the
existing code etc. if need be. For now I egoistically tried to keep a
small footprint for easier patchability in future pgAdmin releases (just
in case this feature for one reason or another won't make it into the
release branch anytime soon).

What are your thoughts on this?

Cheers from Germany and thanks a lot for all your nice work on pgAdmin
and PostgreSQL.
Florian




frm\frmQuery.cpp, based on 1.16.1 release:

613a614,616
> // Jump to the end of the input field after placing code in it
> sqlQuery->DocumentEnd();
>
3161a3165,3217
>
> // Try to find the schema "obj" belongs to and then get its name.
> // If obj is not of type pgSchema, pgSchemaObjCollection or pgSchemaObject (or one of their descendants),
> // it obviously doesn't belong to any schema.
> wxString schemaName;
> pgSchema *schema = dynamic_cast<pgSchema*>(obj);
> if(schema)
> {
> // We have a schema right there. Easy.
> schemaName = schema->GetName();
> }
> else
> {
> // It's not a schema, so check if it's a descendant of pgSchemaObjCollection (e.g. a "Tables" or "Functions" node)
> pgSchemaObjCollection *schemaObjColl = dynamic_cast<pgSchemaObjCollection*>(obj);
> if(schemaObjColl)
> schemaName = schemaObjColl->GetSchema()->GetName();
> else
> {
> // Not a pgSchemaObjCollection either, so check if it's a descendant of pgSchemaObject (e.g. a single table or function)
> pgSchemaObject *schemaObj = dynamic_cast<pgSchemaObject*>(obj);
> if(schemaObj)
> schemaName = schemaObj->GetSchema()->GetName();
> }
> }
> if(!schemaName.IsEmpty())
> {
> // We found a schema for the selected object, so check if it's already contained in search_path and build a "SET search_path" statement otherwise.
> // First though, check whether the schema name contains upper-case characters and therefore needs to be enclosed in quotation marks.
> if(schemaName != schemaName.Lower())
> schemaName = schemaName.Prepend(wxT("\"")).Append(wxT("\""));
> // Now compare it to the current search_path
> wxString searchPath = obj->GetDatabase()->GetSearchPath();
> wxStringTokenizer searchPathTokenizer(searchPath, wxT(","));
> bool schemaContainedInSearchPath = false;
> while(searchPathTokenizer.HasMoreTokens())
> {
> wxString currentToken = searchPathTokenizer.GetNextToken();
> if(currentToken == schemaName || (currentToken == wxT("\"$user\"") && schemaName == obj->GetConnection()->GetUser()))
> {
> schemaContainedInSearchPath = true;
> break;
> }
> }
> if(!schemaContainedInSearchPath)
> {
> // Schema not contained in search_path, so build a statement for it.
> // Since a user's search_path is never empty (?), we can safely put a comma between our current schema name and the user's current search_path
> // without checking for an empty string.
> qry = wxT("SET search_path TO ") + schemaName + wxT(",") + searchPath + wxT(";\n\n");
> }
> }
>
3163c3219,3220
< qry = obj->GetSql(form->GetBrowser());
---
> qry += obj->GetSql(form->GetBrowser());
>



--
Sent via pgadmin-hackers mailing list (pgadmin...@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

[Dave Page]

Hi

On Fri, Feb 8, 2013 at 3:51 PM, Florian Klaar <flo....@gmx.de> wrote:

Hi all,

Recently I wrote a small patch for frmQuery.cpp in order to
auto-generate a "SET search_path TO ..." statement when opening the
query editor with a schema-related object selected in the object
browser, e.g. a table, function or sequence (or their respective
collections).
The reason being that in our company, after migrating from MSSQL Server
to pg, we use a single database for many of our customers, with one
schema per customer and identically named objects within those schemas.
E.g. cust_schema_01.table_x, cust_schema_02.table_x, cust_schema_03.table_x.
For maintenance and debugging, we connect to the database using the
postgres superuser account in pgAdmin3. Now in order to access table_x
within a certain customer's schema in the query editor, we always have
to prepend the schema name to the table name or issue a "SET search_path
TO cust_schema_nn,public" statement.
This is rather tedious, so I came up with a patch for pgAdmin3 that
tries to intelligently generate a search_path depending on the currently
selected object in the object browser as well as depending on the
existing search_path configured for the current database connection.
That way, we can easily open query editors under different schemas
without bothering about the search_path ourselves.

OK.

 

This is what my code does when opening a new query editor window:
- Check whether the currently selected object in the object browser is
of type pgSchema, pgSchemaObject or pgSchemaObjCollection or one of
their descendants which (if I'm not mistaken) means it does have a
schema associated with it.

Why not just do something like:

if (obj->GetSchema())

    sql = wxT("SET search_path TO ") + obj->GetSchema()->GetName();

 

- If so, it checks whether the schema belonging to this object is
already contained in the user's search_path (case-sensitively and
considering the $user placeholder).
- If the schema isn't already in the user's search_path, the code
generates a "SET search_path TO
<selected_schema>,<existing_search_path>" statement and has it written
into the newly opened query editor window.

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.

 

- After that, it places the cursor to the end of the sql text so the
user can begin typing right away.

OK.

 

Tested on Windows XP Pro SP3 and Windows7 Pro only - I didn't bother to
create a build environment on my Linux box yet.
In case you deem this patch useful, find the diff output based on the
1.16.1 release source code attached below. 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.

 

Beware also that though being a developer, I'm really inexperienced in
C/C++ and completely new to wxWidgets and to the inner workings of
pgAdmin, so there may well be room for improvement in my code. Having
said that, I'd be willing to dig further into the pgAdmin3 code in order
to make this a configurable option, integrate it better into the
existing code etc. if need be.

Cool - thanks. 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)

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[Florian Klaar]

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

[Dave Page]

Hi

On Fri, Feb 8, 2013 at 7:34 PM, Florian Klaar <flo....@gmx.de> wrote:

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.

 

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).

 

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.

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.

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.

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.

 

[Florian Klaar]

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

[Florian Klaar]

Hey,

> 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.

Now i've tested it again and of course you were right, just my testing
was flawed. Probably I tested by selecting a schema and opening the
query window, not taking into account that your example code only works
below the schema level of course.
So I've boiled it down to this now:

wxString schemaName;
if(obj->GetTypeName() == wxT("Schema"))
schemaName = obj->GetName();
else if (obj->GetSchema())
schemaName = obj->GetSchema()->GetName();
if(!schemaName.IsEmpty())
{
// the works...
}

Much better. Thanks for your input.

Florian

[Dave Page]

Apologies for the delay in replying...

On Mon, Feb 11, 2013 at 4:38 PM, Florian Klaar <flo....@gmx.de> wrote:

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?).

Yes, you would. I think that's a price you'd pay for flexibility.

On further thought, I'm fairly convinced that this is probably the best way to implement this. It's infinitely more flexible and could be useful in so many different cases, and it avoids the need for many special cases in the code for handling scenarios like those we've discussed, but shifting that responsibility onto the user (bwhahahaha :-) ).

 

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.

Wouldn't it be easiest to do this by adding a new procedure (WAIT) to the pgScript language, and then just running the query in a WHILE TRUE style loop, with a WAIT call in it?

 

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.

I can't reproduce that here on Mac. Can you get a backtrace? 

[Dave Page]

On Mon, Feb 11, 2013 at 8:22 PM, Florian Klaar <flo....@gmx.de> wrote:

Hey,

> 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.

Now i've tested it again and of course you were right, just my testing
was flawed. Probably I tested by selecting a schema and opening the
query window, not taking into account that your example code only works
below the schema level of course.
So I've boiled it down to this now:

wxString schemaName;
if(obj->GetTypeName() == wxT("Schema"))

if (obj->GetMetaType() == PGM_SCHEMA)

...

...

 

    schemaName = obj->GetName();
else if (obj->GetSchema())
    schemaName = obj->GetSchema()->GetName();
if(!schemaName.IsEmpty())
{
  // the works...
}

Much better. Thanks for your input.

Florian

--

[Florian Klaar]

> Apologies for the delay in replying...

No problem at all...

> On further thought, I'm fairly convinced that this is probably the
> best way to implement this. It's infinitely more flexible and could be
> useful in so many different cases, and it avoids the need for many
> special cases in the code for handling scenarios like those we've
> discussed, but shifting that responsibility onto the user (bwhahahaha
> :-) ).

Agreed :-)

> Wouldn't it be easiest to do this by adding a new procedure (WAIT) to
> the pgScript language, and then just running the query in a WHILE TRUE
> style loop, with a WAIT call in it?

To be honest, I haven't looked into pgScript yet, but it sounds like a
good idea to do so.

> I can't reproduce that here on Mac. Can you get a backtrace?

Not that easily, it seems. If I'm not mistaken you'd have to tell me how
to build a debug target for wxWidgets 2.8 in VC2010 because without it,
I can't build the pgAdmin debug target. From what I can tell when
running the release target though, it seems to be related to the UI - VC
breaks in frmQuery::OnChangeNotebook() at
viewMenu->Enable(MNU_OUTPUTPANE, false);
and the stack trace leads into wxWidgets territory. Sorry I can't
provide more information, but I'm really new to C/C++ and Visual Studio.
The crash seems to be 100% reproduceable on XP Pro SP3 and Win7 Pro SP1
with both the binary distribution and the custom built binary with
wxWidgets 2.8.12, if that's of any help. To reproduce, open pgAdmin
1.16.1, connect to server, select a database, open query editor, click
into sql notepad, click on GQB tab.

Regards,
Florian

[Florian Klaar]

> if (obj->GetMetaType() == PGM_SCHEMA)

Ha, better still :-)
Thanks.

[Dave Page]

On Mon, Feb 18, 2013 at 11:10 PM, Florian Klaar <flo....@gmx.de> wrote:
>
>> I can't reproduce that here on Mac. Can you get a backtrace?
> Not that easily, it seems. If I'm not mistaken you'd have to tell me how
> to build a debug target for wxWidgets 2.8 in VC2010 because without it,
> I can't build the pgAdmin debug target.

If you build wxWidgets using the script (which I recall you had
problems with - which I'll be looking into soon as I need to build a
new Windows VM), then it'll automatically create debug binaries. If
you use the IDE, then just do a batch build and ensure you select both
the debug and release targets for all the sub-projects.

> From what I can tell when
> running the release target though, it seems to be related to the UI - VC
> breaks in frmQuery::OnChangeNotebook() at
> viewMenu->Enable(MNU_OUTPUTPANE, false);
> and the stack trace leads into wxWidgets territory.

OK, I can reproduce it on Windows. It's actually the line before - the
manager.Update() call that it's crashing in. Not sure why though; I
guess it doesn't like being called while the tab is updating on
Windows.

Ashesh; can you or one of the team take a look at this please?

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[Dave Page]

Hi

On Mon, Mar 18, 2013 at 1:09 PM, Neel Patel <neel....@enterprisedb.com> wrote:

Hi,

We have checked the bug and below is our finding.

As we are getting the pane information from the manager and pane could not be found in the manager so at that time wxAuiPaneInfo structure is not valid and when we call Update() then it was crashing. So before calling to Update() we put check for validating the wxAuxPaneInfo structure as below.

wxAuiPaneInfo outputPaneInfo = manager.GetPane(wxT("outputPane")).Show(false);

wxAuiPaneInfo scratchPad = manager.GetPane(wxT("scratchPad")).Show(false);

viewMenu->Enable(MNU_OUTPUTPANE, false);

viewMenu->Enable(MNU_SCRATCHPAD, false);

if (outputPaneInfo.IsOk() == true && scratchPad.IsOk() == true)

    manager.Update();

Please find the patch along with this email and let me know.

It still crashes for me I'm afraid. Testing on Windows, debug build, VC++ 2010. 

[Dave Page]

Hi

On Mon, Mar 25, 2013 at 9:02 AM, Neel Patel <neel....@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the attached new patch file for fix. I have tested in Windows,
> debug build, VC++ 2010.
> After this fix i have continuously tested it with half an hour to reproduce
> the crash but i am not able to reproduce it.
>
> Can you please test and check wether crash is again reproducible at your
> end.
>

The crash isn't there any more, but it behaves differently now - the
scratch pad and the output pane are now shown on the GQB pane, whereas
they're not in the existing code - they get automatically hidden. That
causes a further problem (aside from the lack of space) - if you try
to grab the sash between the GQB and the output pane, the top tab set
switches back to the main query text pane.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[Neel Patel]

Hi Dave,

I will check and let you know.

Thanks,

Neel Patel

[Dave Page]

On Thu, Mar 28, 2013 at 12:04 PM, Neel Patel
<neel....@enterprisedb.com> wrote:
> Hi Dave,
>
> I looked in the code and attached is the updated patch.
>
> I have tested with my environment and it is not crashing and also now
> "output pane" and "scratch pad" will not shown on GDB pane.
>
> Please verify at your end for both the issues and let me know if any
> modification is required.

Hi,

I'm still seeing a crash I'm afraid:

1) Start pgAdmin
2) Connect to a database
3) Open the query tool
4) Click in the scratch pane
5) Click the GQB tab.
6) *Boom*

> ntdll.dll!774115de()
[Frames below may be incorrect and/or missing, no symbols loaded for
ntdll.dll]
ntdll.dll!774115de()
ntdll.dll!7740014e()
wxmsw28ud_core_vc_custom.dll!wxSizer::Layout() Line 885 C++
wxmsw28ud_core_vc_custom.dll!wxSizer::SetDimension(int x, int y, int
width, int height) Line 981 C++
wxmsw28ud_core_vc_custom.dll!wxSizerItem::SetDimension(const wxPoint
& pos_, const wxSize & size_) Line 389 C++
0018ef10()
wxmsw28ud_core_vc_custom.dll!wxSizer::Layout() Line 885 C++

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company