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

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
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-07 11:05:46
Message-ID: CAM9w-_mk04Rz6KVvQtmwiHs4s+OFDsDgCKebUO-vT0W2DfGi4w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

On Thu, Jun 7, 2018 at 4:07 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Wed, Jun 6, 2018 at 2:02 PM, Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Hackers,
>>
>> PFA updated patch as the previous one was not working as expected. I have
>> tried to make it similar to that of pgAdmin3 and you do not need to change
>> client_encoding as it is set now based on server encoding. It works fine
>> with "view data" also.
>>
>
> - In connection.py, at ~409, shouldn't we set the client_encoding to
> SQL_ASCII? Otherwise it could be overridden with something unexpected if
> the client has PGCLIENTENCODING set for example.
>
Yeah I agree, it would be better to add. Will add the change.

>
> - With or without that change, I get the following test failure on macOS
> with Python 2.7.10:
>
It works fine on my machine with Python 2.7 and macOS. Could you please let
me know the Postgres DB version also.
Will test on few more machines.

>
> ======================================================================
> ERROR: runTest (pgadmin.tools.sqleditor.tests.test_encoding_charset.
> TestEncodingCharset)
> With Encoding SQL_ASCII
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/Users/dpage/git/pgadmin4/web/pgadmin/tools/sqleditor/
> tests/test_encoding_charset.py", line 86, in runTest
> response = self.tester.get(url)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/werkzeug/test.py",
> line 830, in get
> return self.open(*args, **kw)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/testing.py",
> line 127, in open
> follow_redirects=follow_redirects)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/werkzeug/test.py",
> line 803, in open
> response = self.run_wsgi_app(environ, buffered=buffered)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/werkzeug/test.py",
> line 716, in run_wsgi_app
> rv = run_wsgi_app(self.application, environ, buffered=buffered)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/werkzeug/test.py",
> line 923, in run_wsgi_app
> app_rv = app(environ, start_response)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1997, in __call__
> return self.wsgi_app(environ, start_response)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1985, in wsgi_app
> response = self.handle_exception(e)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1540, in handle_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1982, in wsgi_app
> response = self.full_dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1614, in full_dispatch_request
> rv = self.handle_user_exception(e)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1517, in handle_user_exception
> reraise(exc_type, exc_value, tb)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1612, in full_dispatch_request
> rv = self.dispatch_request()
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1598, in dispatch_request
> return self.view_functions[rule.endpoint](**req.view_args)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask_login.py",
> line 792, in decorated_view
> return func(*args, **kwargs)
> File "/Users/dpage/git/pgadmin4/web/pgadmin/tools/sqleditor/__init__.py",
> line 576, in poll
> 'oids': oids
> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/ajax.py", line 61, in
> make_json_response
> separators=(',', ':')),
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/simplejson/__init__.py", line 399, in dumps
> **kw).encode(obj)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/simplejson/encoder.py", line 291, in encode
> chunks = self.iterencode(o, _one_shot=True)
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/simplejson/encoder.py", line 373, in iterencode
> return _iterencode(o, 0)
> UnicodeDecodeError: 'utf8' codec can't decode byte 0xad in position 0:
> invalid start byte
>
> ----------------------------------------------------------------------
> Ran 317 tests in 30.692s
>
> FAILED (errors=1, skipped=21)
>
>
>
>> The only problem is, I cannot find equivalent codec for wxConvLibc in
>> python. The closest one I could find is raw_unicode_escape. So, in a
>> SQL_ASCII database, non ASCII characters may differ in pgAdmin4 and
>> pgAdmin3, but it will display results.
>>
>
> Yeah, I think that's fine. For the small number of people with SQL_ASCII
> databases, seeing escaped characters is better than nothing.
>
>
>>
>>
>> Dave,
>> You need to add "E" before the string to be inserted, otherwise \x will
>> be considered as a plain string.
>> INSERT INTO sql_ascii (data) VALUES (E'[Invalid UTF-8] Blob:
>> \xf4\xa5\xa3\xa5');
>>
>
> Yeah, sorry - I copied the wrong version of the query :-(
>
>
>>
>>
>> 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 6:42 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> 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
>>>
>>
>>
>
>
> --
> 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 Dave Page 2018-06-07 11:11:16 Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.
Previous Message Dave Page 2018-06-07 10:37:28 Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.