Re: [pgAdmin4][Patch] - RM 3780 pgAdmin4 lacks ability to specify NULL values in CSV export

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch] - RM 3780 pgAdmin4 lacks ability to specify NULL values in CSV export
Date: 2018-12-21 12:22:20
Message-ID: CA+OCxowKR--3-Ar4HPU-22h12WJTGKQ+UEVdkxBcvNpbmK9cxQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Sorry! Here is is.

On Fri, Dec 21, 2018 at 12:12 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Dave
>
> Can you please attached the updated patch with your changes. I'll try to
> fix the exception.
>
> On Fri, Dec 21, 2018 at 3:57 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> Here's an updated patch as I've tweaked some of the wording. The
>> screenshot probably isn't the right resolution, but as we're replacing them
>> anyway it doesn't seem overly important. Feel free to fix if you like :-)
>>
>> With quoting set to either All or Strings, everything looks good. With it
>> set to None, I still get an exception (below). The query I'm using is this:
>>
>> SELECT NULL::text, 1234::int, 'Foo bar'::text, E'Foo\nBar'::text
>>
>> Field separator: ,
>> Quote character: "
>> Replace null's with: NULL
>>
>> Steps:
>>
>> 1) Run pgAdmin in Desktop mode. I'm running from within PyuCharms, using
>> the venv detailed below.
>> 2) Open the Query Tool on a PostgreSQL 9.6.10 database, running on MacOS
>> 10.14.1
>> 3) Run the above query, wit quoting set to All and check the result in
>> the grid.
>> 4) Download the CSV file and check.
>> 5) Open Preferences and set quoting to Strings.
>> 6) Download the CSV file and check.
>> 7) Open Preferences and set quoting to None.
>> 8) Download the CSV file *exception occurs*.
>>
>>
>> System info:
>>
>> (pgadmin4) dpage(at)hal:*~/git/pgadmin4*$ python --version
>>
>> Python 3.6.7
>>
>> (pgadmin4) dpage(at)hal:*~/git/pgadmin4*$ pip freeze
>>
>> alabaster==0.7.11
>>
>> alembic==1.0.0
>>
>> asn1crypto==0.24.0
>>
>> Babel==2.6.0
>>
>> bcrypt==3.1.4
>>
>> blinker==1.4
>>
>> certifi==2018.8.24
>>
>> cffi==1.11.5
>>
>> chardet==3.0.4
>>
>> chromedriver-installer==0.0.6
>>
>> click==6.7
>>
>> cryptography==2.3
>>
>> docutils==0.14
>>
>> extras==1.0.0
>>
>> fixtures==3.0.0
>>
>> Flask==0.12.4
>>
>> Flask-BabelEx==0.9.3
>>
>> Flask-Gravatar==0.5.0
>>
>> Flask-HTMLmin==1.3.2
>>
>> Flask-Login==0.3.2
>>
>> Flask-Mail==0.9.1
>>
>> Flask-Migrate==2.1.1
>>
>> Flask-Paranoid==0.2.0
>>
>> Flask-Principal==0.4.0
>>
>> Flask-Security==3.0.0
>>
>> Flask-SQLAlchemy==2.3.2
>>
>> Flask-WTF==0.14.2
>>
>> html5lib==1.0.1
>>
>> htmlmin==0.1.12
>>
>> idna==2.7
>>
>> imagesize==1.1.0
>>
>> itsdangerous==0.24
>>
>> Jinja2==2.10
>>
>> linecache2==1.0.0
>>
>> Mako==1.0.7
>>
>> MarkupSafe==1.0
>>
>> packaging==18.0
>>
>> paramiko==2.4.1
>>
>> passlib==1.7.1
>>
>> pbr==3.1.1
>>
>> psutil==5.4.8
>>
>> psycopg2==2.7.5
>>
>> pyasn1==0.4.4
>>
>> pycodestyle==2.3.1
>>
>> pycparser==2.18
>>
>> pycrypto==2.6.1
>>
>> Pygments==2.2.0
>>
>> PyNaCl==1.2.1
>>
>> pyparsing==2.2.2
>>
>> pyperclip==1.6.4
>>
>> pyrsistent==0.14.2
>>
>> python-dateutil==2.7.3
>>
>> python-editor==1.0.3
>>
>> python-mimeparse==1.6.0
>>
>> pytz==2018.3
>>
>> requests==2.19.1
>>
>> selenium==3.14.1
>>
>> simplejson==3.13.2
>>
>> six==1.11.0
>>
>> snowballstemmer==1.2.1
>>
>> speaklater==1.3
>>
>> Sphinx==1.8.2
>>
>> sphinxcontrib-websupport==1.1.0
>>
>> SQLAlchemy==1.2.10
>>
>> sqlparse==0.2.4
>>
>> sshtunnel==0.1.4
>>
>> testscenarios==0.5.0
>>
>> testtools==2.3.0
>>
>> traceback2==1.4.0
>>
>> unittest2==1.1.0
>>
>> urllib3==1.23
>>
>> webencodings==0.5.1
>>
>> Werkzeug==0.14.1
>>
>> WTForms==2.1
>>
>> Exception:
>>
>> 2018-12-21 11:47:28,995: SQL pgadmin: Execute (with server cursor) for
>> server #2 - CONN:8760231 (Query-id: 8649354):
>> SELECT NULL::text, 1234::int, 'Foo bar'::text, E'Foo\nBar'::text
>> 2018-12-21 11:47:29,001: INFO werkzeug: 127.0.0.1 - - [21/Dec/2018
>> 11:47:29] "GET
>> /sqleditor/query_tool/download/2133388?query=SELECT%20NULL%3A%3Atext%2C%201234%3A%3Aint%2C%20%27Foo%20bar%27%3A%3Atext%2C%20E%27Foo%5CnBar%27%3A%3Atext&filename=data-1545392848979.csv
>> HTTP/1.1" 500 -
>> 2018-12-21 11:47:29,003: ERROR werkzeug: Error on request:
>> Traceback (most recent call last):
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/serving.py",
>> line 270, in run_wsgi
>> execute(self.server.app)
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/serving.py",
>> line 260, in execute
>> for data in application_iter:
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/wsgi.py",
>> line 870, in __next__
>> return self._next()
>> File
>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/wrappers.py",
>> line 82, in _iter_encoded
>> for item in iterable:
>> File
>> "/Users/dpage/git/pgadmin4/web/pgadmin/utils/driver/psycopg2/connection.py",
>> line 848, in gen
>> csv_writer.writerows(results)
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line 761, in
>> writerows
>> return self.writer.writerows(map(self._dict_to_list, rowdicts))
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line 268, in
>> writerows
>> self.writerow(row)
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line 261, in
>> writerow
>> row = [self.strategy.prepare(field, only=only) for field in row]
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line 261, in
>> <listcomp>
>> row = [self.strategy.prepare(field, only=only) for field in row]
>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line 142, in
>> prepare
>> raise Error('No escapechar is set')
>> _csv.Error: No escapechar is set
>>
>> On Thu, Dec 20, 2018 at 1:05 PM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Dave
>>>
>>> On Thu, Dec 20, 2018 at 5:12 PM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>>
>>>> On Thu, Dec 20, 2018 at 4:48 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Thu, Dec 20, 2018 at 10:09 AM Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Dave
>>>>>>
>>>>>> On Thu, Dec 20, 2018 at 3:08 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> When testing with quoting set to None, quote = " and delimiter = , I
>>>>>>> get the following exception when I try to download:
>>>>>>>
>>>>>>> 2018-12-20 09:34:02,547: SQL pgadmin: Execute (with server cursor)
>>>>>>> for server #2 - CONN:354106 (Query-id: 4121147):
>>>>>>> SELECT NULL::text, 1234::int, 'Foo bar'::text, E'Foo\nBar'::text
>>>>>>> 2018-12-20 09:34:02,570: INFO werkzeug: 127.0.0.1 - - [20/Dec/2018
>>>>>>> 09:34:02] "GET
>>>>>>> /sqleditor/query_tool/download/5610522?query=SELECT%20NULL%3A%3Atext%2C%201234%3A%3Aint%2C%20%27Foo%20bar%27%3A%3Atext%2C%20E%27Foo%5CnBar%27%3A%3Atext&filename=data-1545298442530.csv
>>>>>>> HTTP/1.1" 500 -
>>>>>>> 2018-12-20 09:34:02,572: ERROR werkzeug: Error on request:
>>>>>>> Traceback (most recent call last):
>>>>>>> File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/serving.py",
>>>>>>> line 270, in run_wsgi
>>>>>>> execute(self.server.app)
>>>>>>> File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/serving.py",
>>>>>>> line 260, in execute
>>>>>>> for data in application_iter:
>>>>>>> File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/wsgi.py",
>>>>>>> line 870, in __next__
>>>>>>> return self._next()
>>>>>>> File
>>>>>>> "/Users/dpage/.virtualenvs/pgadmin4/lib/python3.6/site-packages/werkzeug/wrappers.py",
>>>>>>> line 82, in _iter_encoded
>>>>>>> for item in iterable:
>>>>>>> File
>>>>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/utils/driver/psycopg2/connection.py",
>>>>>>> line 820, in gen
>>>>>>> csv_writer.writerows(results)
>>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line
>>>>>>> 748, in writerows
>>>>>>> return self.writer.writerows(map(self._dict_to_list, rowdicts))
>>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line
>>>>>>> 256, in writerows
>>>>>>> self.writerow(row)
>>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line
>>>>>>> 249, in writerow
>>>>>>> row = [self.strategy.prepare(field, only=only) for field in row]
>>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line
>>>>>>> 249, in <listcomp>
>>>>>>> row = [self.strategy.prepare(field, only=only) for field in row]
>>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/csv.py", line
>>>>>>> 136, in prepare
>>>>>>> raise Error('No escapechar is set')
>>>>>>> _csv.Error: No escapechar is set
>>>>>>>
>>>>>>
>>>>>> Not able to reproduce the above issue. I have tested it with the
>>>>>> same setting as you mentioned. Please refer all the attached screenshots.
>>>>>> Please specify the steps if they are different.
>>>>>>
>>>>>>>
>>>>>>> When I have quoting set to All, the first column is returned as ""
>>>>>>>
>>>>>>> dpage(at)hal:*~/Downloads*$ more data-1545298598112.csv
>>>>>>>
>>>>>>> "text","int4","text-2","text-3"
>>>>>>>
>>>>>>> "","1234","Foo bar","Foo
>>>>>>>
>>>>>>> Bar"
>>>>>>>
>>>>>>> Isn't the point for it to be NULL?
>>>>>>>
>>>>>>
>>>>>> while quoting is set to ALL, all the data types has been quoted,
>>>>>> so I thought null values should be replaced by "" instead of blank. But if
>>>>>> you think null values shouldn't be quoted even if user select quote ALL,
>>>>>> I'll fix it and resend the patch.
>>>>>>
>>>>>
>>>>> So how would you distinguish NULL from an empty string? Isn't that
>>>>> exactly what the bug is about?
>>>>>
>>>>> I still think we need a "Replace NULLs with" config option, and
>>>>> regardless of quoting settings we always replace NULL values with whatever
>>>>> that is set to - for which the user could then choose options like:
>>>>>
>>>>> NULL
>>>>> "NULL"
>>>>> ""
>>>>> ''
>>>>> <empty string>
>>>>>
>>>>> We would never quote the NULL replacement value - if the user wanted
>>>>> it to be quoted, they would include the quotes in the configured string.
>>>>>
>>>>
>>>>
>>>> OK, Will work on it and send the modified patch again.
>>>>
>>>
>>> Attached is the modified patch as per your suggestion.
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> On Tue, Dec 18, 2018 at 11:13 AM Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Dave
>>>>>>>>
>>>>>>>> Attached is the modified patch to fix review comments.
>>>>>>>>
>>>>>>>> On Tue, Dec 18, 2018 at 3:00 PM Akshay Joshi <
>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Dec 18, 2018 at 2:49 PM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> On Tue, Dec 18, 2018 at 3:45 AM Akshay Joshi <
>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>
>>>>>>>>>>> Attached is the patch to fix RM #3780 pgAdmin4 lacks ability to
>>>>>>>>>>> specify NULL values in CSV export.
>>>>>>>>>>>
>>>>>>>>>>> Please review it.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> A few points;
>>>>>>>>>>
>>>>>>>>>> - You've included code from backports.csv, but per the licence
>>>>>>>>>> you need to include a description of the changes made.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sure. In that case I'll copy the complete file and will do
>>>>>>>>> my changes which is of two lines only. With my patch I have remove all the
>>>>>>>>> unwanted code from backport.csv.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - Shouldn't backports.csv be removed from requirements.txt, or is
>>>>>>>>>> it used elsewhere?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yes. Will do that.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - If the previous point is true, then I'm fairly sure there is
>>>>>>>>>> code in one or more of the many package build scripts that adds an
>>>>>>>>>> __init__.py file to backports.csv in the venv that's created.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'll remove that code as well.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Dave Page
>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>
>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> *Akshay Joshi*
>>>>>>>>>
>>>>>>>>> *Sr. Software Architect *
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> *Akshay Joshi*
>>>>>>>>
>>>>>>>> *Sr. Software Architect *
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Dave Page
>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>> Twitter: @pgsnake
>>>>>>>
>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Akshay Joshi*
>>>>>>
>>>>>> *Sr. Software Architect *
>>>>>>
>>>>>>
>>>>>>
>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Dave Page
>>>>> Blog: http://pgsnake.blogspot.com
>>>>> Twitter: @pgsnake
>>>>>
>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
RM_3780_v4.patch application/octet-stream 112.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2018-12-21 12:29:42 Re: pgAdmin 4 commit: Improvement in the look and feel of the whole applica
Previous Message Dave Page 2018-12-21 12:21:39 Re: pgAdmin 4 commit: Improvement in the look and feel of the whole applica