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

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.

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