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-04 05:23:20
Message-ID: CAM9w-_nEDycbeEEeUhKwXjxmkNC39YzqhggJR3Frc4pq6WcJ0Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Hackers,

PFA the updated patch on this. I have tried to add test cases to check for
different encoding database. In the test run, it will create a database,
fire a query for a string, check if we get the output and drops the
database.
Kindly review.

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

On Thu, May 31, 2018 at 6:42 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Thu, May 31, 2018 at 1:20 AM, Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> Hi Victoria/Joao,
>>
>> On Thu, May 31, 2018 at 2:06 AM, Joao De Almeida Pereira <
>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>
>>> Hello Aditya,
>>>
>>> It looks ok and it passes CI.
>>>
>>> We have some recommendations:
>>> - These look like 2 different changes so they should be in separated
>>> commits
>>>
>>
>> If you are talking of set client_encoding, then its not a bug. Its a
>> choice given to Postgres DB user to change the encoding of the characters.
>> Postgres will translate characters from Server Encoding to Client Encoding,
>> and will throw error like mentioned previously. This link will help better
>> - https://www.postgresql.org/docs/10/static/multibyte.html
>> The actual bug was, even after changing the client encoding to SQL_ASCII,
>> pgAdmin4 was not able to show the output as it was failing in encoding by
>> psycopg2. The patch is for resolving that.
>>
>>
>>> - Do we have test coverage for the bug that you are talking about? If
>>> not we should, to ensure this problem does not happen again in a future
>>> change.
>>>
>>
>> It is not possible adding test cases for encoding related stuff, as
>> Postgres support a lot many different types of encoding and conversions
>> (refer above link)
>>
>
> I was going to ask the same thing. Per https://www.postgresql.
> org/docs/10/static/multibyte.html#id-1.6.10.5.7, every characterset
> except SQL_ASCII can be converted to UTF-8, so we only need to test that
> UTF-8 and some other charactersets besides SQL_ASCII work, and then
> separately that SQL_ASCII with characters known not to be in UTF-8 work.
>
>
>>
>>> Thanks
>>> Victoria && Joao
>>>
>>> On Wed, May 30, 2018 at 3:06 AM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> PFA updated patch after all the permutations, combinations for encoding
>>>> for SQL_ASCII database. Also fixed a small glitch for sql editor
>>>> connection status check.
>>>>
>>>> Please note, ERROR: invalid byte sequence for encoding "UTF8": 0xe9
>>>> 0x73 is a Postgres DB error and not pgAdmin4 error.
>>>>
>>>> <Image Deleted>
>>>>
>>>> You need to change client_encoding to the appropriate. After changing
>>>> client_encoding using command - set client_encoding='XYZ', it will give not
>>>> give error.
>>>>
>>>> <Image Deleted>
>>>>
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>> "Don't Complain about Heat, Plant a tree"
>>>>
>>>> On Wed, May 23, 2018 at 10:13 AM, Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Thank you Victoria, Anthony.
>>>>>
>>>>> Thanks and Regards,
>>>>> Aditya Toshniwal
>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>>> "Don't Complain about Heat, Plant a tree"
>>>>>
>>>>> On Tue, May 22, 2018 at 7:15 PM, Victoria Henry <vhenry(at)pivotal(dot)io>
>>>>> wrote:
>>>>>
>>>>>> Hi Aditya,
>>>>>>
>>>>>> We made a minor change to make the patch so the python linter can
>>>>>> pass. Attached is the change we made.
>>>>>> Everything else looks good.
>>>>>>
>>>>>> Sincerely,
>>>>>>
>>>>>> Victoria & Anthony
>>>>>>
>>>>>> On Tue, May 22, 2018 at 4:46 AM Aditya Toshniwal <
>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> PFA updated patch. Linter issues are fixed ( we dont have any linter
>>>>>>> setup for python :-( )
>>>>>>> Regarding test cases, they run successfully on my system and the
>>>>>>> reason it failed for pivotal is timeout exception. I am sorry I can't help
>>>>>>> with that.
>>>>>>>
>>>>>>> Traceback (most recent call last):
>>>>>>> File "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keyboard_shortcut_test.py",
>>>>>>> line 52, in runTest
>>>>>>> self._check_shortcuts()
>>>>>>> File "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keyboard_shortcut_test.py",
>>>>>>> line 77, in _check_shortcuts
>>>>>>> ") and contains(@class, 'open')]")
>>>>>>> File "/root/.pyenv/versions/pgadmin36/lib/python3.6/site-packages
>>>>>>> /selenium/webdriver/support/wait.py", line 80, in until
>>>>>>> raise TimeoutException(message, screen, stacktrace)
>>>>>>> selenium.common.exceptions.TimeoutException: Message:
>>>>>>>
>>>>>>> Thanks and Regards,
>>>>>>> Aditya Toshniwal
>>>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>>>>> "Don't Complain about Heat, Plant a tree"
>>>>>>>
>>>>>>> On Tue, May 22, 2018 at 1:37 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> Pivotal's buildbot is showing problems with this patch:
>>>>>>>>
>>>>>>>> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines
>>>>>>>> /pgadmin-patch/jobs/run-linter/builds/66 (linter failed)
>>>>>>>> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines
>>>>>>>> /pgadmin-patch/jobs/run-tests/builds/84 (tests failed)
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, May 22, 2018 at 7:05 AM, Aditya Toshniwal <
>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Hackers,
>>>>>>>>>
>>>>>>>>> PFA patch for RM#3289 where decode error was thrown on querying a
>>>>>>>>> SQL_ASCII database table. Please note, this problem occurs only on windows.
>>>>>>>>> Sample insert - insert into test_tab values ('é');
>>>>>>>>>
>>>>>>>>> psycopg2 has a encodings dictionary where Postgres Database
>>>>>>>>> Encodings are mapped to python equivalent. It uses 'ascii' decoder of
>>>>>>>>> python to decode for SQL_ASCII encoding. If data has characters beyond the
>>>>>>>>> limit of ascii then it failed. The solution would be to use utf_8 decoder
>>>>>>>>> instead of ascii. I tried setting the client_encoding using
>>>>>>>>> set_client_encoding('UTF8') method of a psycopg2 connection but no luck
>>>>>>>>> (also its not allowed for async connection). I also tried executing "SET
>>>>>>>>> CLIENT_ENCODING='UTF8'" but it didn't work too.
>>>>>>>>> So, as in the patch, I had to set encodings dict value directly to
>>>>>>>>> 'utf_8' and it seems to be working. Please note, the same is added to
>>>>>>>>> psycopg3 milestones
>>>>>>>>> https://github.com/psycopg/psycopg2/milestone/4
>>>>>>>>>
>>>>>>>>> Also fixed a small glitch for sql editor connection status check.
>>>>>>>>>
>>>>>>>>> Kindly review.
>>>>>>>>>
>>>>>>>>> Thanks and Regards,
>>>>>>>>> Aditya Toshniwal
>>>>>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>>>>>>> "Don't Complain about Heat, Plant a tree"
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> 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
>

Attachment Content-Type Size
RM3289.patch text/x-patch 4.0 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2018-06-04 05:35:49 Re: [pgadmin4][Patch]: Test cases for the backup module
Previous Message Dave Page 2018-06-02 02:07:18 Re: [pgadmin4][patch] Use pytest test runner for unit tests