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

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.

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