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

Hi Dave

I have modified the condition, it won't through any exception. Attached is
the updated patch, please review it.

On Fri, Dec 21, 2018 at 4:22 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:

> 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
>

--
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*