public inbox for [email protected]  
help / color / mirror / Atom feed
From: Akshay Joshi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch] - RM 3780 pgAdmin4 lacks ability to specify NULL values in CSV export
Date: Thu, 20 Dec 2018 15:38:55 +0530
Message-ID: <CANxoLDcoc3aXym5SZMO0ENKHmqNPWf4Kut9KupT5QCPzTtLfZg@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoxbiW-aULQP4ftBw3eeAz+cgB6krHntGZBV_4MoxTV=bA@mail.gmail.com>
References: <CANxoLDdQr19YWNRgYU+ARs4BgnJGwNBu_51+g0AXWbCC6ZpNNA@mail.gmail.com>
	<CA+OCxoxno88VLcWxc5ybfDxg=yZNpUFdNCXmKgF4hk_sN50HUQ@mail.gmail.com>
	<CANxoLDfg0xmVoQQ8JU4OAaBmWOxuCWz0Z2eY-ZWPWHDBn2uJ7w@mail.gmail.com>
	<CANxoLDca1xaaS+T2oAgNhWxQVJf7N+KPm89KX4jzNiy8x2MrZg@mail.gmail.com>
	<CA+OCxoxbiW-aULQP4ftBw3eeAz+cgB6krHntGZBV_4MoxTV=bA@mail.gmail.com>

Hi Dave

On Thu, Dec 20, 2018 at 3:08 PM Dave Page <[email protected]> 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@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 <
> [email protected]> wrote:
>
>> Hi Dave
>>
>> Attached is the modified patch to fix review comments.
>>
>> On Tue, Dec 18, 2018 at 3:00 PM Akshay Joshi <
>> [email protected]> wrote:
>>
>>>
>>>
>>> On Tue, Dec 18, 2018 at 2:49 PM Dave Page <[email protected]> wrote:
>>>
>>>> Hi
>>>>
>>>> On Tue, Dec 18, 2018 at 3:45 AM Akshay Joshi <
>>>> [email protected]> 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*


Attachments:

  [image/png] Table_Data.png (43.9K, 3-Table_Data.png)
  download | view image

  [image/png] Preferences.png (93.9K, 4-Preferences.png)
  download | view image

  [image/png] Downloaded_File.png (43.8K, 5-Downloaded_File.png)
  download | view image

view thread (20+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: [pgAdmin4][Patch] - RM 3780 pgAdmin4 lacks ability to specify NULL values in CSV export
  In-Reply-To: <CANxoLDcoc3aXym5SZMO0ENKHmqNPWf4Kut9KupT5QCPzTtLfZg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox