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-02-04 05:34:33
Message-ID: CAM5-9D9-HW8pn=CJ4j=Khv2afm+R1Bx26wmU7A9VB9sEvJNeGQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

Please find updated patch and review.

On Fri, Feb 3, 2017 at 2:43 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> On Fri, Feb 3, 2017 at 7:28 AM, Surinder Kumar
> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> > Hi Dave,
> >
> > On Mon, Jan 30, 2017 at 6:18 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >>
> >> On Fri, Jan 27, 2017 at 10:32 AM, Surinder Kumar
> >> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> >> > 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.
> >>
> >> I ran some tests:
> >>
> >> - Setting a field to '' resulted in the following SQL:
> >>
> >> UPDATE public.emp SET
> >> job = '''''' WHERE
> >> empno = 7369;
> >>
> >> - Setting a field to \"\" resulted in the following SQL:
> >>
> >> UPDATE public.emp SET
> >> job = '""' WHERE
> >> empno = 7499;
> >>
> >> - Setting a field to \'\' displayed \'\' in the grid until refreshed
> >> when the value vanished. The SQL it ran was:
> >
> > In current behaviour, we are saving the value provided by user and we are
> > not refreshing the grid with new values.
> > Should we do refresh along with save?
>
> Why would you save the value provided? The point is to escape the
> quotes with the slashes - i.e. to store the literal string '' (two
> single quotes), the user enters \'\' (because entering just two single
> quotes is how we enter an empty string).
>
​Implemented accordingly.​ I have tested all cases you provided and they
are working.
If there is still anything not working. Please let me know. I will fix.

>
> I'm not sure why this is so hard - the original request was to make it
> work like pgAdmin III. That's well defined and documented behaviour -
> I even copied/pasted the description from the docs on this thread.
>
> >>
> >> UPDATE public.emp SET
> >> job = '''''' WHERE
> >> empno = 7499;
> >>
> >> To be clear, here's what I'm expecting:
> >>
> >> Input: <empty>
> >> Display: [null]
> >> SQL: UPDATE t SET c = NULL WHERE k = <val>
> >>
> >> Input: ''
> >> Display:
> >> SQL: UPDATE t SET c = '' WHERE k = <val>
> >>
> >> Input: \'\'
> >> Display: ''
> >> SQL: UPDATE t SET c = '''''' WHERE k = <val>
> >>
> >> Input: \\'\\'
> >> Display: \'\'
> >> SQL: UPDATE t SET c = '\''\''' WHERE k = <val>
> >>
> >> --
> >> 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_v4.patch application/octet-stream 6.0 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-02-04 13:15:14 Re: [pgAdmin4][Patch]: Fix RM1790 - [Web] Support setting a field's value to "null"
Previous Message Atira Odhner 2017-02-03 21:56:02 Re: Acceptance Tests against a browser (WIP)