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

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(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)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
>>>
>>
>>
>