Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fix RM1790 - [Web] Supportsetting a field's value to "null" - Mailing list pgadmin-hackers

From Dave Page
Subject Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fix RM1790 - [Web] Supportsetting a field's value to "null"
Date
Msg-id CA+OCxoyi+SkJU454fbm8w_ZLKZwn9h-E7R1G+NFSqt0Qo0tZfg@mail.gmail.com
Whole thread Raw
In response to Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fix RM1790 - [Web] Supportsetting a field's value to "null"  (Surinder Kumar <surinder.kumar@enterprisedb.com>)
Responses Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fix RM1790 - [Web] Supportsetting a field's value to "null"
List pgadmin-hackers
Hi,

This seems to work nicely now for text and int fields etc. but you
haven't handled booleans. They should have a 3 state checkbox in edit
mode (checked == true, clear == false, blurred == null (see pgAdmin
3).

I've committed the patch, but can you please get me an additional one
for boolean support for Monday morning my time please?

Thanks!

On Sat, Feb 4, 2017 at 5:34 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find updated patch and review.
>
> On Fri, Feb 3, 2017 at 2:43 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Fri, Feb 3, 2017 at 7:28 AM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi Dave,
>> >
>> > On Mon, Jan 30, 2017 at 6:18 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> On Fri, Jan 27, 2017 at 10:32 AM, Surinder Kumar
>> >> <surinder.kumar@enterprisedb.com> wrote:
>> >> > Hi Dave,
>> >> >
>> >> > Please find updated patch.
>> >> >
>> >> > On Mon, Jan 16, 2017 at 10:01 PM, Dave Page <dpage@pgadmin.org>
>> >> > wrote:
>> >> >>
>> >> >> Hi
>> >> >>
>> >> >> On Fri, Jan 13, 2017 at 9:24 AM, Surinder Kumar
>> >> >> <surinder.kumar@enterprisedb.com> wrote:
>> >> >> > Hi
>> >> >> >
>> >> >> > Please find attached patch and review.
>> >> >> >
>> >> >> > On Sun, Jan 8, 2017 at 3:27 PM, Dave Page <dpage@pgadmin.org>
>> >> >> > wrote:
>> >> >> >>
>> >> >> >> Hi
>> >> >> >>
>> >> >> >> On Friday, December 23, 2016, Surinder Kumar
>> >> >> >> <surinder.kumar@enterprisedb.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
>
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


pgadmin-hackers by date:

Previous
From: Surinder Kumar
Date:
Subject: Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fix RM1790 - [Web] Supportsetting a field's value to "null"
Next
From: Dave Page
Date:
Subject: Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fix RM1790 - [Web] Supportsetting a field's value to "null"