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