Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database. - Mailing list pgadmin-hackers

From Joao De Almeida Pereira
Subject Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.
Date
Msg-id CAE+jjametYGjStNFZFW544Jcm_by1OABtb7yFQWKGzyfk08QiA@mail.gmail.com
Whole thread Raw
In response to Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.  (Aditya Toshniwal <aditya.toshniwal@enterprisedb.com>)
Responses Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database.  (Aditya Toshniwal <aditya.toshniwal@enterprisedb.com>)
List pgadmin-hackers
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.

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@pgadmin.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@pivotal.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.toshniwal@enterprisedb.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@pivotal.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.toshniwal@enterprisedb.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@pgadmin.org> wrote:

On Tue, May 22, 2018 at 7:05 AM, Aditya Toshniwal <aditya.toshniwal@enterprisedb.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

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


pgadmin-hackers by date:

Previous
From: Joao De Almeida Pereira
Date:
Subject: Re: [pgadmin4][Patch]: Test cases for the backup module
Next
From: Dave Page
Date:
Subject: Re: [pgadmin4][patch] Use pytest test runner for unit tests