Fix for a problem with auto reconnection

Fix for a problem with auto reconnection

My colleagues and I recently stumbled over a quite perplexing issue with 
pgadmin3 (1.18.1), namely that the pgadmin query tool sometimes did not 
show values in the database which were definitely set.

After further investigation it turned out that this happened when the 
values contained non-ascii characters and after the database connection 
had been reset by pgadmin (which happened sometimes when the database 
was restarted or there was a timeout in the firewall), and if that reset 
happened via the query tool window, not via the main window.

The explanation for this behavior is obvious: The connection is reset 
with conn->Reset() in frm/frmQuery.cpp, but the Reset() method does not 
set the client side encoding for the connection to match the encoding of 
the database, as is done in DoConnect(). It also fails to do some other 
things that DoConnect() cares about, such as setting the DateStyle to 
ISO and setting the default role. This could cause additional problems.

So my solution was to replace the call to conn->Reset() with a call to 
conn->Reconnect() which essentially does the same, but calls DoConnect() 
internally so that everything gets initialized.

This fix works nicely for me. Another solution would be to add the 
initialization code in DoConnect() to the Reset() method as well.

Here are the steps to reproduce the problem:

* Open the SQL query tool and create a test table like this:

   create table t(a varchar , b varchar);
   insert into t values ('Wurst', 'Käse');

* Query the table with "select * from t".
   You should get "Wurst" and "Käse".

* Kill the server process for the connection

* Query the table again with "select * from t".
   You should get a Fatal error: connection lost.

* Rerun the query.
   You should now get a dialog box asking
   "... attempt to reconnect ... ?"

* Answer "Yes".
   You should get the message "Connection reset."

* Rerun the query.
   Now you get the query result again,
   but this time only with the cell "Wurst";
   the cell that should say "Käse" stays empty.

Re: Fix for a problem with auto reconnection

Am 15.11.2013 06:13, schrieb Ashesh Vashi:
 > Sure - I will look into it..

Thanks for looking into it. One more thing that I forgot to mention: The 
problem only appears if the database encoding is not already UNICODE, 
but, for instance, LATIN1 or WIN1252. This is unfortunately still a 
quite frequent setting here in Europe. So you need to create such a 
database to reproduce. If your template1 database is already UTF8, you 
need to create it from template0.

-- Christoph

Re: Fix for a problem with auto reconnection

I posted this bug with suggested fix in 2013 
(http://www.postgresql.org/message-id/52851ED9.1050003@online.de), but 
it seems nobody has looked into it until now. As far as I understand, 
there is no real bug-tracker for PgAdmin (which is unfortunate), so I'm 
re-raising the issue here.

-- Christoph

Re: Fix for a problem with auto reconnection

Thanks for looking into this. The problem with the wrong encoding is now 
gone, but another problem appeared:

When the connection is reestablished, I get an additional alert with an 
error message "Column not found in pgSet: "datlastsysoid".

The problem seems to be that needColQuoting is not reset to false in the 
Reset() method as it is done in the Reconnect() method.

Wenn I add the line "needColQuoting = false" before the call to 
Initialize() in the Reset() method, everything works as expected.

-- Christoph


Am 17.06.2014 08:35, schrieb Ashesh Vashi:
> Dave,
>
> PFA - moved the initialisation code in a separate function, and called
> that function from the "Reset" function too.
> Please take a look at it.
>
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi

Re: Fix for a problem with auto reconnection

Am 27.06.2014 09:57, schrieb Dave Page:
> Ashesh - your thoughts on this? I'm not sure I see how resetting
> needColQuoting would prevent a datlastsysoid error, though I admit I
> haven't had a chance to dive into the code yet.

Some more detail:

The error happens in line 285 of pgadmin/db/pgConn.cpp:

     if (set->ColNumber(wxT("\"datlastsysoid\"")) >= 0)
         needColQuoting = true;

This assumes that needColQuoting was set to false before. Another 
solution might be to modify that check to something like this:

     if (!needColQuoting &&
             set->ColNumber(wxT("\"datlastsysoid\"")) >= 0)
         needColQuoting = true;

-- Christoph

Re: Fix for a problem with auto reconnection

Ashesh - your thoughts on this? I'm not sure I see how resetting
needColQuoting would prevent a datlastsysoid error, though I admit I
haven't had a chance to dive into the code yet.

On Sun, Jun 22, 2014 at 3:57 PM, Christoph Zwerschke  wrote:
> Thanks for looking into this. The problem with the wrong encoding is now
> gone, but another problem appeared:
>
> When the connection is reestablished, I get an additional alert with an
> error message "Column not found in pgSet: "datlastsysoid".
>
> The problem seems to be that needColQuoting is not reset to false in the
> Reset() method as it is done in the Reconnect() method.
>
> Wenn I add the line "needColQuoting = false" before the call to Initialize()
> in the Reset() method, everything works as expected.
>
> -- Christoph
>
>
> Am 17.06.2014 08:35, schrieb Ashesh Vashi:
>
>> Dave,
>>
>> PFA - moved the initialisation code in a separate function, and called
>> that function from the "Reset" function too.
>> Please take a look at it.
>>
>>
>> --
>>
>> Thanks & Regards,
>>
>> Ashesh Vashi



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

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

Re: Fix for a problem with auto reconnection

Re: Fix for a problem with auto reconnection

On Fri, Jun 27, 2014 at 9:58 AM, Christoph Zwerschke  wrote:
> Am 27.06.2014 09:57, schrieb Dave Page:
>
>> Ashesh - your thoughts on this? I'm not sure I see how resetting
>> needColQuoting would prevent a datlastsysoid error, though I admit I
>> haven't had a chance to dive into the code yet.
>
>
> Some more detail:
>
> The error happens in line 285 of pgadmin/db/pgConn.cpp:
>
>     if (set->ColNumber(wxT("\"datlastsysoid\"")) >= 0)
>         needColQuoting = true;
>
> This assumes that needColQuoting was set to false before. Another solution
> might be to modify that check to something like this:
>
>     if (!needColQuoting &&
>             set->ColNumber(wxT("\"datlastsysoid\"")) >= 0)
>         needColQuoting = true;

Huh, that a kinda weird way of doing things. I think in the longer
term we should consider refactoring the handling of that variable
entirely.

In any case, I've committed a fix now, doing what your first suggested
(we do that elsewhere it seems).

Thanks!

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

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

Re: Fix for a problem with auto reconnection

Ashesh, can you look at this please?

Thanks for the report and investigation Christoph.

On Thu, Nov 14, 2013 at 7:04 PM, Christoph Zwerschke  wrote:
> My colleagues and I recently stumbled over a quite perplexing issue with
> pgadmin3 (1.18.1), namely that the pgadmin query tool sometimes did not show
> values in the database which were definitely set.
>
> After further investigation it turned out that this happened when the values
> contained non-ascii characters and after the database connection had been
> reset by pgadmin (which happened sometimes when the database was restarted
> or there was a timeout in the firewall), and if that reset happened via the
> query tool window, not via the main window.
>
> The explanation for this behavior is obvious: The connection is reset with
> conn->Reset() in frm/frmQuery.cpp, but the Reset() method does not set the
> client side encoding for the connection to match the encoding of the
> database, as is done in DoConnect(). It also fails to do some other things
> that DoConnect() cares about, such as setting the DateStyle to ISO and
> setting the default role. This could cause additional problems.
>
> So my solution was to replace the call to conn->Reset() with a call to
> conn->Reconnect() which essentially does the same, but calls DoConnect()
> internally so that everything gets initialized.
>
> This fix works nicely for me. Another solution would be to add the
> initialization code in DoConnect() to the Reset() method as well.
>
> Here are the steps to reproduce the problem:
>
> * Open the SQL query tool and create a test table like this:
>
>   create table t(a varchar , b varchar);
>   insert into t values ('Wurst', 'Käse');
>
> * Query the table with "select * from t".
>   You should get "Wurst" and "Käse".
>
> * Kill the server process for the connection
>
> * Query the table again with "select * from t".
>   You should get a Fatal error: connection lost.
>
> * Rerun the query.
>   You should now get a dialog box asking
>   "... attempt to reconnect ... ?"
>
> * Answer "Yes".
>   You should get the message "Connection reset."
>
> * Rerun the query.
>   Now you get the query result again,
>   but this time only with the cell "Wurst";
>   the cell that should say "Käse" stays empty.
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers



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

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

Re: Fix for a problem with auto reconnection

Ashesh, did you get a chance to look at this?

On Thu, May 29, 2014 at 8:05 PM, Christoph Zwerschke  wrote:
> I posted this bug with suggested fix in 2013
> (http://www.postgresql.org/message-id/52851ED9.1050003@online.de), but it
> seems nobody has looked into it until now. As far as I understand, there is
> no real bug-tracker for PgAdmin (which is unfortunate), so I'm re-raising
> the issue here.
>
>
> -- Christoph
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers



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

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

Re: Fix for a problem with auto reconnection

Re: Fix for a problem with auto reconnection

Re: Fix for a problem with auto reconnection