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

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

Hi Victoria/Joao,

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()

The test cases work perfectly fine on my machine. I will try if I can
simulate the failure on some other machines.

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

On Mon, Jun 4, 2018 at 8:54 PM, Joao De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)io> wrote:

> Hi Aditya,
> Looks like there are changes in this patch that are related to
> notifications, these should go into a separate commit as they are not
> related to encoding.
> Also the tests are failing in our pipeline: https://gpdb-dev.
> bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-
> patch/jobs/run-tests/builds/110
>
> Thanks
> Victoria && Joao
>
> On Mon, Jun 4, 2018 at 5:55 AM Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Please ignore previous patch. Fixed some linter issues. PFA updated patch.
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>> On Mon, Jun 4, 2018 at 10:53 AM, Aditya Toshniwal <aditya.toshniwal@
>> enterprisedb.com> wrote:
>>
>>> 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.toshniwal@
>>>>>> enterprisedb.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
>>>>
>>>
>>>
>>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Joao De Almeida Pereira 2018-06-04 19:38:25 Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.
Previous Message Dave Page 2018-06-04 16:34:52 pgAgent 4.0 patch