Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.
Date: 2018-06-05 13:12:37
Message-ID: CA+OCxoxuS2GXK=LxREpniK6hHQ=d2G2FBpjD7JSohxZJpQwxPA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

On Tue, Jun 5, 2018 at 2:03 PM, Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi
>
> On Tue, Jun 5, 2018 at 6:25 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Tue, Jun 5, 2018 at 1:49 PM, Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)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
>> <https://git.postgresql.org/gitweb/?p=pgadmin3.git;a=blob;f=pgadmin/db/pgConn.cpp#l300>
>> wxLogInfo(wxT("Setting client_encoding to '%s'")
>> , encoding.c_str());
>> 301
>> <https://git.postgresql.org/gitweb/?p=pgadmin3.git;a=blob;f=pgadmin/db/pgConn.cpp#l301>
>> if (PQsetClientEncoding(conn, encoding.ToAscii()))
>> 302
>> <https://git.postgresql.org/gitweb/?p=pgadmin3.git;a=blob;f=pgadmin/db/pgConn.cpp#l302>
>> {
>> 303
>> <https://git.postgresql.org/gitweb/?p=pgadmin3.git;a=blob;f=pgadmin/db/pgConn.cpp#l303>
>> wxLogError(wxT("%s"), GetLastError().c_str());
>> 304
>> <https://git.postgresql.org/gitweb/?p=pgadmin3.git;a=blob;f=pgadmin/db/pgConn.cpp#l304>
>> }
>> 305
>> <https://git.postgresql.org/gitweb/?p=pgadmin3.git;a=blob;f=pgadmin/db/pgConn.cpp#l305>
>>
>> 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(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Tue, Jun 5, 2018 at 1:21 PM, Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Dave,
>>>>>
>>>>>
>>>>> On Tue, Jun 5, 2018 at 4:56 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Tue, Jun 5, 2018 at 9:50 AM, Aditya Toshniwal <
>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Tue, Jun 5, 2018 at 1:08 AM, Joao De Almeida Pereira <
>>>>>>>> jdealmeidapereira(at)pivotal(dot)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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Victoria Henry 2018-06-05 13:34:33 Re: [pgAdmin4][patch] Moved 'Notifications' tab before 'Query History' in Query Tool
Previous Message Aditya Toshniwal 2018-06-05 13:03:21 Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.