Received: from malur.postgresql.org ([2a02:16a8:dc51::56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fPrM3-00028G-EL for pgadmin-hackers@arkaria.postgresql.org; Mon, 04 Jun 2018 15:25:07 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fPrM1-0008W9-R6 for pgadmin-hackers@arkaria.postgresql.org; Mon, 04 Jun 2018 15:25:05 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fPrM1-0008Ve-Dl for pgadmin-hackers@lists.postgresql.org; Mon, 04 Jun 2018 15:25:05 +0000 Received: from mx0a-00296801.pphosted.com ([148.163.150.38]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fPrLu-0001Lm-6z for pgadmin-hackers@postgresql.org; Mon, 04 Jun 2018 15:25:00 +0000 Received: from pps.filterd (m0114583.ppops.net [127.0.0.1]) by mx0a-00296801.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w54FKOPl017646 for ; Mon, 4 Jun 2018 15:24:56 GMT Received: from mail-io0-f200.google.com (mail-io0-f200.google.com [209.85.223.200]) by mx0a-00296801.pphosted.com with ESMTP id 2jbj0ma7ty-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Mon, 04 Jun 2018 15:24:56 +0000 Received: by mail-io0-f200.google.com with SMTP id n24-v6so25598680ioh.12 for ; Mon, 04 Jun 2018 08:24:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fw+/cK/MhahFF0UU6i70nEaTmDuHHPuEUxdjLouMf4Y=; b=KI10g9lWzvTkKLQmgjWIEVyw28aIB2bRBDCQl9g8DyCco0nRlrL4ulhuI1Xjx9f6US pAdr8Da8nSgIXEPJxct/f1hsy+iqXasyfwmWNWqLeRxGSPV1TElFl9UUTMX0Sd0lBUeJ CRQKfAbupK+DCSZX6g8gOKdYG/gAX6CeJbObTysvSKISDWAJqfwlIzOQ3z2tmA8ecRJr fLYdTM3ztur/DUX+6EP7ajQ37OcUp8ZO4/0z5J4zmSToWTQjjOp98hDwLn+FiuAxZiSj Zhz3ePap6m7Tzm9VCTF+DNXRsT9Lf3iePXRX0yfJcqaK+Z4sOLyxn0DLwXUMaCzJ9h+R xSuQ== X-Gm-Message-State: APt69E3MjmDoNZoHswP0GhqEveK2nDzMp5+HViFqRu71uXxXrjrDE1FY LBw2baoZznqzIWmFP24Em9Pk6JPnVzQXYsopcqZI10LKTOI735FNw7NWRwCrHh2P5rgROP6DYHE BGMtJnQp+t7EVwsdqUHA0PyUR1Qsgr+WecW6Cz7MjeqFqenT13UKF0FJihOYKcpM1N3aV X-Received: by 2002:a24:90d:: with SMTP id 13-v6mr5892322itm.69.1528125894836; Mon, 04 Jun 2018 08:24:54 -0700 (PDT) X-Google-Smtp-Source: ADUXVKK3gxFarTQ9LlK94bgjHkwB8lhfen3Pqh62/pL5stEA9oDAZNf/Nd8x/IS257u19FoyToz9bZaajDRHRRt/b30= X-Received: by 2002:a24:90d:: with SMTP id 13-v6mr5892287itm.69.1528125894568; Mon, 04 Jun 2018 08:24:54 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Joao De Almeida Pereira Date: Mon, 4 Jun 2018 11:24:18 -0400 Message-ID: Subject: Re: [pgAdmin4][RM#3289] Can't query SQL_ASCII database. To: Aditya Toshniwal Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000001e43ff056dd289d7" X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-06-04_11:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=10 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1805220000 definitions=main-1806040181 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000001e43ff056dd289d7 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 drop= s >> 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 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 charact= ers. >>>> Postgres will translate characters from Server Encoding to Client Enco= ding, >>>> and will throw error like mentioned previously. This link will help be= tter >>>> - 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 futu= re >>>>> change. >>>>> >>>> >>>> It is not possible adding test cases for encoding related stuff, as >>>> Postgres support a lot many different types of encoding and conversion= s >>>> (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 on= ly >>> 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 b= e 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. >>>>>> >>>>>> >>>>>> >>>>>> You need to change client_encoding to the appropriate. After changin= g >>>>>> client_encoding using command - set client_encoding=3D'XYZ', it will= give not >>>>>> give error. >>>>>> >>>>>> >>>>>> >>>>>> 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 >>>>>>> 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 c= an't help >>>>>>>>> with that. >>>>>>>>> >>>>>>>>> Traceback (most recent call last): >>>>>>>>> File >>>>>>>>> "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keybo= ard_shortcut_test.py", >>>>>>>>> line 52, in runTest >>>>>>>>> self._check_shortcuts() >>>>>>>>> File >>>>>>>>> "/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keybo= ard_shortcut_test.py", >>>>>>>>> line 77, in _check_shortcuts >>>>>>>>> ") and contains(@class, 'open')]") >>>>>>>>> File >>>>>>>>> "/root/.pyenv/versions/pgadmin36/lib/python3.6/site-packages/sele= nium/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 >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hi >>>>>>>>>> >>>>>>>>>> Pivotal's buildbot is showing problems with this patch: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pga= dmin-patch/jobs/run-linter/builds/66 >>>>>>>>>> (linter failed) >>>>>>>>>> >>>>>>>>>> https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pga= dmin-patch/jobs/run-tests/builds/84 >>>>>>>>>> (tests failed) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> 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 on= ly on >>>>>>>>>>> windows. >>>>>>>>>>> Sample insert - insert into test_tab values ('=C3=A9'); >>>>>>>>>>> >>>>>>>>>>> psycopg2 has a encodings dictionary where Postgres Database >>>>>>>>>>> Encodings are mapped to python equivalent. It uses 'ascii' deco= der 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 execu= ting "SET >>>>>>>>>>> CLIENT_ENCODING=3D'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 chec= k. >>>>>>>>>>> >>>>>>>>>>> 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 >>> >> >> > --0000000000001e43ff056dd289d7 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Aditya,
Looks like there are changes in this patch = that are related to notifications, these should go into a separate commit a= s they are not related to encoding.

Thanks
Victoria && = Joao

On Mon, Jun= 4, 2018 at 5:55 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com&= gt; wrote:
Hi Hack= ers,

Please ignore previous patch. Fixed some linter iss= ues. PFA updated patch.

Thanks and Regards,
Aditya Toshniwal
Software Engineer |=C2=A0EnterpriseDB Software Solutions |=C2= =A0Pune
"Don't Complain about Heat, Plant a tree"

On Mon, Jun 4, 2018 at 10:53 AM, Aditya Tosh= niwal <aditya.toshniwal@enterprisedb.com> wr= ote:
Hi Hackers,
PFA the updated patch on this. I have tried to add test cases t= o 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 t= he database.
Kindly review.

Thanks and Regards,
= Aditya Toshniwal<= /div>
Software Engineer |= =C2=A0EnterpriseDB Software Solutions |=C2=A0Pune
"Don't = Complain about Heat, Plant a tree"

On Thu, May 31, 2018 at 6:42 PM, Dave Page <= dpage@pgadmin.org> wrote:
<= div dir=3D"ltr">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 P= ereira <jdealmeidapereira@pivotal.io> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">
Hello Adi= tya,

It looks ok and it passes CI.

<= div>We have some recommendations:
- These look like 2 different c= hanges so they should be in separated commits
= =C2=A0
If you are talking of set= client_encoding, then its not a bug. Its a choice given to Postgres DB use= r to change the encoding of the characters. Postgres will translate charact= ers from Server Encoding to Client Encoding, and will throw error like ment= ioned previously. This link will help better -=C2=A0https://www.postgresql.org/docs/10/static/multibyte.html
The actual bug was, even after changing the cli= ent encoding to SQL_ASCII, pgAdmin4 was not able to show the output as it w= as failing in encoding by psycopg2. The patch is for resolving that.=
=C2=A0
- Do we have test= coverage for the bug that you are talking about? If not we should, to ensu= re 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 e= ncoding and conversions (refer above link)=C2=A0

I was going to ask the same thing. Per= =C2=A0https://www.postgresql.org/docs/10/static/m= ultibyte.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 char= actersets besides SQL_ASCII work, and then separately that SQL_ASCII with c= haracters known not to be in UTF-8 work.
=C2=A0

Thanks
Victoria && Joao

On Wed, May 30, 2018 at 3:06 AM Aditya Toshniwal <aditya.toshniw= al@enterprisedb.com> wrote:
Also fixed a small glitch for sql editor connect= ion status check.

Please note,=C2=A0ERROR: invalid byte sequence fo= r encoding "UTF8": 0xe9 0x73 is a Postgres DB error and not pgAdm= in4 error.

<Image Deleted>
=

You need to change cl= ient_encoding to the appropriate. After changing client_encoding using comm= and - set client_encoding=3D'XYZ', it will give not give error.

<Image D= eleted>

Thanks and Regar= ds,
Aditya Toshniwal=
= Softwa= re Engineer |=C2=A0EnterpriseDB Software Solutions |=C2=A0Pune
&qu= ot;Don't Complain about Heat, Plant a tree"

On Wed, May 23, 2018 at 10:13 AM, Aditya Tos= hniwal <aditya.toshniwal@enterprisedb.com> w= rote:
Thank you Victoria, Anthony.

Thanks and Regards,
Aditya Toshniwal
Software Engineer |=C2=A0Enter= priseDB Software Solutions |=C2=A0Pune<= /span>
"Don't Complain ab= out 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.=C2=A0 Attached is the change we made.
<= div>Everything else looks good.

Sincerely,

Victoria & Anthony

On Tue, May 22, 2018 at 4= :46 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
H= i,

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 time= out exception. I am sorry I can't help with that.

T= raceback (most recent call last):
=C2=A0 File "/tmp/build/a453582b/= pgadmin-repo/web/pgadmin/feature_tests/keyboard_shortcut_test.py", lin= e 52, in runTest
=C2=A0 =C2=A0 self._check_shortcuts()
=C2=A0 File &q= uot;/tmp/build/a453582b/pgadmin-repo/web/pgadmin/feature_tests/keyboard_sho= rtcut_test.py", line 77, in _check_shortcuts
=C2=A0 =C2=A0 ") = and contains(@class, 'open')]")
=C2=A0 File "/root/.py= env/versions/pgadmin36/lib/python3.6/site-packages/selenium/webdriver/suppo= rt/wait.py", line 80, in until
=C2=A0 =C2=A0 raise TimeoutException= (message, screen, stacktrace)
selenium.common.exceptions.TimeoutExceptio= n: Message:

Thanks and Regards,<= /font>
Aditya Toshniwal
Software E= ngineer |=C2=A0EnterpriseDB Software Solutions |=C2=A0Pune
"D= on't Complain about Heat, Plant a tree"
<= /div>

On Tue, May 22, 2018 at 1:37 PM, Dave Page <= span dir=3D"ltr"><dpage@pgadmin.org> wrote:

On Tue, May 22, 2018 at 7:05 AM, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Hack= ers,

PFA patch for RM#3289 where decode error was = thrown on querying a SQL_ASCII database table. Please note, this problem oc= curs only on windows.
Sample insert -=C2=A0insert into test_tab v= alues ('=C3=A9');

psycopg2 has a encodings= dictionary where Postgres Database Encodings are mapped to python equivale= nt. It uses 'ascii' decoder of python to decode for SQL_ASCII encod= ing. If data has characters beyond the limit of ascii then it failed. The s= olution would be to use utf_8 decoder instead of ascii. I tried setting the= client_encoding using set_client_encoding('UTF8') method of a psyc= opg2 connection but no luck (also its not allowed for async connection). I = also tried executing "SET CLIENT_ENCODING=3D'UTF8'" but i= t didn't work too.
So, as in the patch, I had to set encoding= s 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.

<= div>Kindly review.

Thanks and Regards,
Aditya Toshniwal
Software Engineer |=C2=A0EnterpriseDB Software Solutions |=C2= =A0Pune
"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 Compan= y







=
--
Dave Page
Blog: <= a href=3D"http://pgsnake.blogspot.com" target=3D"_blank">http://pgsnake.blo= gspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.comThe Enterprise PostgreSQL Company


--0000000000001e43ff056dd289d7--