Hi On Thu, May 31, 2018 at 1:20 AM, Aditya Toshniwal < aditya.toshni...@enterprisedb.com> wrote:
> Hi Victoria/Joao, > > On Thu, May 31, 2018 at 2:06 AM, Joao De Almeida Pereira < > jdealmeidapere...@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.toshni...@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.toshni...@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 <vhe...@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.toshni...@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 <dp...@pgadmin.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.toshni...@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 >>>>>>>> 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
