Обсуждение: [pgAdmin4][Patch]: Fix RM1790 - [Web] Support setting a field's value to "null"

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.

Forgot to attach patch in last thread. please find patch.

On Fri, Dec 23, 2016 at 11:24 AM, Surinder Kumar <surinder.kumar@enterprisedb.com> wrote:

On Fri, Dec 23, 2016 at 11:11 AM, Surinder Kumar <surinder.kumar@enterprisedb.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.kumar@enterprisedb.com> wrote:

On Mon, Dec 5, 2016 at 11:09 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Friday, December 2, 2016, Surinder Kumar <surinder.kumar@enterprisedb.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

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.

>> 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.kumar@enterprisedb.com> wrote:
>>>>
>>>> On Fri, Dec 23, 2016 at 11:11 AM, Surinder Kumar
>>>> <surinder.kumar@enterprisedb.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.kumar@enterprisedb.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Dec 5, 2016 at 11:09 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> On Friday, December 2, 2016, Surinder Kumar
>>>>>>> <surinder.kumar@enterprisedb.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

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:

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

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

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

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

BTW, this may help: https://css-tricks.com/indeterminate-checkboxes/

On Sat, Feb 4, 2017 at 1:15 PM, Dave Page <dpage@pgadmin.org> wrote:
> 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



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

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

Thanks - patch applied!

On Sat, Feb 4, 2017 at 8:58 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi Dave,
>
> I have overriden the checkbox editor to add 'indeterminate' state of
> checkbox other than checked and unchecked which gives the user capability to
> set [null] value
> for boolean types.
>
> Please find updated patch and review.
>
> On Sat, Feb 4, 2017 at 6:47 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> BTW, this may help: https://css-tricks.com/indeterminate-checkboxes/
>
> That's actually the solution. It works.
> Thanks.
>>
>>
>>
>> On Sat, Feb 4, 2017 at 1:15 PM, Dave Page <dpage@pgadmin.org> wrote:
>> > 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.
>>
>> >
>> > 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
>>
>>
>>
>> --
>> 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