Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database. - Mailing list pgadmin-hackers

From Dave Page
Subject Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.
Date
Msg-id CA+OCxoxuS2GXK=LxREpniK6hHQ=d2G2FBpjD7JSohxZJpQwxPA@mail.gmail.com
Whole thread Raw
In response to Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.  (Aditya Toshniwal <aditya.toshniwal@enterprisedb.com>)
Responses Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.
List pgadmin-hackers
Hi

On Tue, Jun 5, 2018 at 2:03 PM, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi

On Tue, Jun 5, 2018 at 6:25 PM, Dave Page <dpage@pgadmin.org> wrote:


On Tue, Jun 5, 2018 at 1:49 PM, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,

The problem of SQL ASCII is solved with the patch, and not related to setting the client encoding of the sql window.

No it's not. It doesn't work for me as I said (and showed the example of).

After setting the client_encoding to SQL_ASCII you got the output. Previously, it used to fail in the back end itself because python encoding failure. That is fixed.
The error ERROR:  invalid byte sequence for encoding "UTF8": 0x80 is thrown by postgres and not python or pgAdmin4. You will get the same error even if you 
connect from psql.

Sure - but that is not a fix. You have no way of running the SET command if you're using "view data" - and in the query tool, users just expect it to work (as it did in pgAdmin 3).
 
 
I can see there is no SET call in pgAdmin3 for client_encoding.  I can remove the SET client_encoding='UNICODE'; that will solve the problem. But, can you please let me know why that was added. 

There is, but it's inside an API call (PQsetClientEncoding):

300             wxLogInfo(wxT("Setting client_encoding to '%s'"), encoding.c_str());
301             if (PQsetClientEncoding(conn, encoding.ToAscii()))
302             {
303                 wxLogError(wxT("%s"), GetLastError().c_str());
304             }

Oops ! Missed that. Apologies. 
 

Will remove the set call and will send you the updated patch if everything works fine.

No, we need to ensure the client encoding is set correctly. It just needs to be set to SQL_ASCII if it's a SQL_ASCII database (I believe).
 
Need to rework on the initialise method. Will come with an updated. patch. Sorry for trouble. 


On Tue, Jun 5, 2018 at 6:05 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Jun 5, 2018 at 1:21 PM, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,


On Tue, Jun 5, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Jun 5, 2018 at 9:50 AM, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Hackers,

PFA updated patch. The sqleditor change is sent separately and removed from current patch as suggested.
The test cases were running fine when the module was specified using --pkg but were failing in complete run. Fixed that.

I did a quick test by creating a SQL_ASCII database containing a simple table:

CREATE TABLE sql_ascii (id serial primary key, data text);

And then populated it with data:

/Library/PostgreSQL/9.4/bin/psql -d sql_ascii -U postgres -c "INSERT INTO sql_acsii (data) VALUES ('[Windows-1252]   Euro: \x80   Double dagger: \x87');"
/Library/PostgreSQL/9.4/bin/psql -d sql_ascii -U postgres -c "INSERT INTO sql_ascii (data) VALUES ('[Latin-1]   Yen: \xa5   Half: \xbd');"
/Library/PostgreSQL/9.4/bin/psql -d sql_ascii -U postgres -c "INSERT INTO sql_ascii (data) VALUES ('[Japanese]   Ship: \xe8\x88\xb9');"
/Library/PostgreSQL/9.4/bin/psql -d sql_ascii -U postgres -c "INSERT INTO sql_ascii (data) VALUES ('[Invalid UTF-8]  Blob: \xf4\xa5\xa3\xa5');"

I then right-clicked the table in the treeview, and selected the option to view all rows, and immediately saw an error:

2018-06-05 12:23:27,319: SQL pgadmin: Execute (async) for server #1 - CONN:1187535 (Query-id: 8522474):
SELECT * FROM public.sql_ascii
ORDER BY id ASC 
2018-06-05 12:23:27,320: ERROR pgadmin: Failed to execute query (execute_async) for the server #1 - CONN:1187535(Query-id: 8522474):
Error Message:ERROR:  invalid byte sequence for encoding "UTF8": 0x80
SQL state: 22021

Running "SELECT * FROM sql_ascii" in the query tool resulted in the same error, however, if I ran "SET client_encoding = 'SQL_ASCII';" first, I do see results.

I have confirmed that I've restarted the server after applying the patch.

What am I missing? Why don't we just set the client_encoding to SQL_ASCII if it's a SQL_ASCII database?
 
It is by default same as the server encoding. But, the following existing code in  web/pgadmin/utils/driver/psycopg2/connection.py makes the client_encoding as UNICODE for every connection. I am not sure it should be removed.

        status = _execute(cur, "SET DateStyle=ISO;"

                               "SET client_min_messages=notice;"

                               "SET bytea_output=escape;"

                               "SET client_encoding='UNICODE';")


It was probably before you joined, but I have said a number of times that pgAdmin 3 handled this differently and that maybe we should do it the same way here. See https://git.postgresql.org/gitweb/?p=pgadmin3.git;a=blob;f=pgadmin/db/pgConn.cpp, in the pgConn::Initialize() function.

Either way, your patch isn't working for me.
 


Note that this testing was on Python 2.7.10 on MacOS.
 

Kindly review.

Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB Software Solutions | Pune
"Don't Complain about Heat, Plant a tree"

On Tue, Jun 5, 2018 at 10:15 AM, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi

On Tue, Jun 5, 2018 at 1:08 AM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Aditya,


There is no change related to notifications in this patch. 
The below code is minor fix related to connection status of sql editor. Can you please share the code snippet if it is not the below.

-        # Check for the asynchronous notifies statements.
-        conn.check_notifies(True)
-        notifies = conn.get_notifies()
+        if status is not None:
+            # Check for the asynchronous notifies statements.
+            conn.check_notifies(True)
+            notifies = conn.get_notifies()


This is a minor fix, but is it related to querying SQL_ASCII database?
No its not. It is something I found when I was working on SQL_ASCII related changes.
Well then, will send a separate patch for it.

Thanks
Victoria && Joao





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

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




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

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




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

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




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

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

pgadmin-hackers by date:

Previous
From: Aditya Toshniwal
Date:
Subject: Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.
Next
From: Victoria Henry
Date:
Subject: Re: [pgAdmin4][patch] Moved 'Notifications' tab before 'QueryHistory' in Query Tool