Re: [pgAdmin4][Patch]: Fix RM1790 - [Web] Support setting a field's value to "null"

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: Fix RM1790 - [Web] Support setting a field's value to "null"
Date: 2017-01-27 10:32:21
Message-ID: CAM5-9D8RAWT3F4CyPT-MxjiJDEUB9zVTGS9w9yTPV5trdvodPw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

Please find updated patch.

On Mon, Jan 16, 2017 at 10:01 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Fri, Jan 13, 2017 at 9:24 AM, Surinder Kumar
> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> > Hi
> >
> > Please find attached patch and review.
> >
> > On Sun, Jan 8, 2017 at 3:27 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >>
> >> Hi
> >>
> >> On Friday, December 23, 2016, Surinder Kumar
> >> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> >>>
> >>> Forgot to attach patch in last thread. please find patch.
> >>
> >>
> >> It looks good for the most part, except:
> >>
> >> 1) You missed the part we discussed about being able to set a value to
> ''
> >> (the literal string containing two single quotes) by entering \'\' (and
> of
> >> course, the follow-on cases to allow setting a value to \'\' by entering
> >> \\'\\' etc).
> >
> > Fixed.
>
> That doesn't seem right to me - the code you've written looks like
> it'll try to escape anything for use in a string literal, not just
> '\'\ or \\'\\' etc.
>
​Now the implementation is that It will find and unescape the string
literals like ​'\'\ or \\'\\' etc.

>
> >> 2) Could you please update the appropriate doc page to explain how this
> >> all works? You can lift the text from the pgAdmin 3 docs for the most
> part.
> >
> > I will send a separate patch for it.
> >>
> >>
> >> Thanks.
> >>
> >>>
> >>>
> >>> On Fri, Dec 23, 2016 at 11:24 AM, Surinder Kumar
> >>> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> >>>>
> >>>> On Fri, Dec 23, 2016 at 11:11 AM, Surinder Kumar
> >>>> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> >>>>>
> >>>>> Hi Dave,
> >>>>>
> >>>>> Please find updated patch.
> >>>>>
> >>>>> Changes implemented:
> >>>>>
> >>>>> 1) To enter an empty string in string/character type, user need to
> >>>>> enter '' (two single quotes).
> >>>>> 2) To enter null values in Integer/String type, user need to keep the
> >>>>> field blank.
> >>>>> 3) Null values will be represented as [null].
> >>>>>
> >>>>> Please find attached patch and review.
> >>>>>
> >>>>> On Fri, Dec 9, 2016 at 2:23 PM, Surinder Kumar
> >>>>> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Mon, Dec 5, 2016 at 11:09 PM, Dave Page <dpage(at)pgadmin(dot)org>
> wrote:
> >>>>>>>
> >>>>>>> Hi
> >>>>>>>
> >>>>>>> On Friday, December 2, 2016, Surinder Kumar
> >>>>>>> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> >>>>>>>>
> >>>>>>>> Hi
> >>>>>>>>
> >>>>>>>> Issue:
> >>>>>>>> - On viewing table data, If we edit a column and set value of
> >>>>>>>> column(type: text) to "null", It always takes it as empty string.
> It doesn't
> >>>>>>>> honour null values.
> >>>>>>>>
> >>>>>>>> Solution:
> >>>>>>>> - Take a flag "is_null" for columns with data type 'text', then on
> >>>>>>>> GUI, whilst user edits a text field, an additional option with
> >>>>>>>> checkbox(is_null ?) is given to take null values. If checkbox is
> checked, on
> >>>>>>>> JS side we check "is_null" flag and pass field value to null if
> selected.
> >>>>>>>>
> >>>>>>>> Please find patch and review.
> >>>>>>>
> >>>>>>>
> >>>>>>> A nice solution, but there are some problems I think;
> >>>>>>>
> >>>>>>> - How do I set a field that doesn't use the text editor to null?
> e.g.
> >>>>>>> an integer? If I try to set one to blank, I get an error that it's
> invalid
> >>>>>>> input syntax for an integer.
> >>>>>>
> >>>>>> It seems possible by writing custom editor which will convert empty
> >>>>>> string to null before save operation.
> >>>>>
> >>>>> Now If you set blank for integer field, field will set to null.
> >>>>>>>
> >>>>>>>
> >>>>>>> - When null values are first displayed, they are shown as blank
> >>>>>>> entries. If I then set one to null, it displays "null". It should
> always
> >>>>>>> display consistently - I'd suggest "[null]"
> >>>>>>
> >>>>>> Ok. But the issue is if we display "[null]" in cell for null entry.
> >>>>>> How would we distinguish If it is user entered string(as user can
> also enter
> >>>>>> "[null]") or it represent null value ? (Ashesh's concern)
> >>>>>
> >>>>> Now null values will be represented in field as [null].
> >>>>>>>
> >>>>>>>
> >>>>>>> Whilst I like the way this works in part, I think it's going to be
> >>>>>>> inconsistent in the way it would be displayed. I think we need to
> follow the
> >>>>>>> pgAdmin III way of handling this. The docs say the following:
> >>>>>>>
> >>>>>>> ====
> >>>>>>> If an SQL NULL is to be written to the table, simply leave the
> field
> >>>>>>> empty. If you store a new row, this will let the server fill in
> the default
> >>>>>>> value for that column. If you store a change to an existing row,
> the value
> >>>>>>> NULL will explicitly be written.
> >>>>>>>
> >>>>>>>
> >>>>>>> ...
> >>>>>>>
> >>>>>>> If you want pgAdmin III to write an empty string to the table, you
> >>>>>>> enter the special string ‘’ (two single quotes) in the field. If
> you want to
> >>>>>>> write a string containing solely two single quotes to the table,
> you need to
> >>>>>>> escape these quotes, by typing \‘\’
> >>>>>
> >>>>> To write an empty string, now user can enter '' (two single quotes),
> it
> >>>>> will be treated as empty string.
> >>>>>>>
> >>>>>>> ====
> >>>>>>>
> >>>>>>> In other words, if an empty value is included for a new row, that
> >>>>>>> column will be omitted from the INSERT statement, allowing the
> server to use
> >>>>>>> a default, or set it to blank.
> >>>>>>>
> >>>>>>> For existing rows, an empty value for any data type is updated as
> >>>>>>> NULL - e.g. col = NULL.
> >>>>>>>
> >>>>>>> For character/string types, if the user enters '', then an empty
> >>>>>>> string is written to the column when either inserting or updating.
> >>>>>>>
> >>>>>>>
> >>>>>>> If the user wishes to insert the literal string '' (i.e. 2 single
> >>>>>>> quotes), then \'\' must be entered, and pgAdmin converts that to
> ''.
> >>>>>>>
> >>>>>>>
> >>>>>>> To enter a literal string of \'\', then the user enters \\'\\', for
> >>>>>>> \\'\\' they enter \\\\'\\\\' and so on.
> >>>>>
> >>>>> If a user enters a literal string \'\', this value is escaped by
> adding
> >>>>> slashes on python side and unescaped by removing added slashes when
> returned
> >>>>> to display.
> >>>>> the entered values are already escaped, user need not to escape
> values.
> >>>>>>
> >>>>>> This behaviour seems to working wine in pgAdmin3 query tool but not
> on
> >>>>>> viewing data by right click context menu.
> >>>>>>
> >>>>>>
> >>>>>> In view data:
> >>>>>> When user enter literal strings like '', \'\'
> >>>>>> &
> >>>>>> \\'\\', it displays these strings as it is after saving. It seems
> >>>>>> conversion doesn't happen.
> >>>>>> so which one is correct to follow in pgAdmin4?
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Thanks.
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Dave Page
> >>>>>>> Blog: http://pgsnake.blogspot.com
> >>>>>>> Twitter: @pgsnake
> >>>>>>>
> >>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
> >>>>>>> The Enterprise PostgreSQL Company
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >>
> >> --
> >> Dave Page
> >> Blog: http://pgsnake.blogspot.com
> >> Twitter: @pgsnake
> >>
> >> EnterpriseDB UK: http://www.enterprisedb.com
> >> The Enterprise PostgreSQL Company
> >>
> >
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
RM1790_v3.patch application/octet-stream 9.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-01-27 11:04:35 Re: [pgAdmin4] [PATCH] Simplify Server's python setup
Previous Message Neel Patel 2017-01-27 10:30:11 [pgAdmin4][patch][FileManager]: RM-2110 - Invalid path error displayed