Обсуждение: Patch: Auto-generate search_path statement for selected schema in query editor
Patch: Auto-generate search_path statement for selected schema in query editor
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 tableor 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 "SETsearch_path" statement otherwise. > // First though, check whether the schema name contains upper-case characters and therefore needs to be enclosedin 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 nameand 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()); >
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.
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.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: Patch: Auto-generate search_path statement for selected schema in query editor
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();
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.
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.
Ok.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.
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.
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
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();
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.
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.
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.
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.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: Patch: Auto-generate search_path statement for selected schema in query editor
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.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.
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]: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.
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.
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.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?
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.
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?).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.
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
Re: Patch: Auto-generate search_path statement for selected schema in query editor
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
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?).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.
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.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hey,Now i've tested it again and of course you were right, just my testing
> 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.
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
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: Patch: Auto-generate search_path statement for selected schema in query editor
> 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
Re: Patch: Auto-generate search_path statement for selected schema in query editor
> if (obj->GetMetaType() == PGM_SCHEMA) Ha, better still :-) Thanks. Florian
On Mon, Feb 18, 2013 at 11:10 PM, Florian Klaar <flo.klaar@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
Re: Patch: Auto-generate search_path statement for selected schema in query editor
On Mon, Feb 18, 2013 at 11:10 PM, Florian Klaar <flo.klaar@gmx.de> wrote:If you build wxWidgets using the script (which I recall you had
>
>> 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.
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.OK, I can reproduce it on Windows. It's actually the line before - the
> 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.
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
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
![](http://s.wisestamp.com/pixel.png?p=mozilla&v=2.0.3&t=1285751372076&u=4060523&e=777)
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.
Thanks,
Neel Patel
On Tue, Feb 19, 2013 at 6:53 PM, Dave Page <dpage@pgadmin.org> wrote:On Mon, Feb 18, 2013 at 11:10 PM, Florian Klaar <flo.klaar@gmx.de> wrote:If you build wxWidgets using the script (which I recall you had
>
>> 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.
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.OK, I can reproduce it on Windows. It's actually the line before - the
> 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.
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.Neel,Can you please take a look at this?-- Ashesh--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--
--Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
Вложения
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.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
HiOn Mon, Mar 18, 2013 at 1:09 PM, Neel Patel <neel.patel@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
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения
Hi On Mon, Mar 25, 2013 at 9:02 AM, Neel Patel <neel.patel@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
HiThe crash isn't there any more, but it behaves differently now - the
On Mon, Mar 25, 2013 at 9:02 AM, Neel Patel <neel.patel@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.
>
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
Hi Dave,I will check and let you know.Thanks,Neel PatelOn Tue, Mar 26, 2013 at 12:22 AM, Dave Page <dpage@pgadmin.org> wrote:HiThe crash isn't there any more, but it behaves differently now - the
On Mon, Mar 25, 2013 at 9:02 AM, Neel Patel <neel.patel@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.
>
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
Вложения
On Thu, Mar 28, 2013 at 12:04 PM, Neel Patel <neel.patel@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
2) Connect to a database
3) Open the query tool
4) Click in the scratch pane
5) Click the GQB tab.
On Thu, Mar 28, 2013 at 12:04 PM, Neel Patel
<neel.patel@enterprisedb.com> wrote:
> Hi Dave,
>> I looked in the code and attached is the updated patch.Hi,
>
> 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.
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
Вложения
On Tue, Apr 16, 2013 at 12:18 PM, Neel Patel <neel.patel@enterprisedb.com> wrote: > Hi Dave, > > Please find the patch in attachement. > > I have followed the below steps to test and it is not crashing now. > > 1) Start pgAdmin > 2) Connect to a database > 3) Open the query tool > 4) Click in the scratch pane > 5) Click the GQB tab. > > Can you please verify at your environment setup and let me know the output. For the sake of the archives (I've already chatted with Neel about this in our daily meeting), I spent an hour or so working on this myself yesterday. The updated patch fixes the crash, but introduces some other weird behaviour: - Run the query tool, and open the GQB. - Add a table, and select a couple of columns in it. - Generate the SQL query, and note that the entire output tabset has vanished, so there's nowhere to see results or server messages etc. I've yet to be able to figure out what's going on here exactly - wxWidgets seems to be calling the event handler in which the crash occurs recursively, but through events, which makes it really hard to debug. Debugging is ongoing though, albeit at low priority as the issue is trivial to work around. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company