Re: [pgAdmin4][Patch]: RM #2849 - Allow editing of data on tables with OIDs but no primary key

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM #2849 - Allow editing of data on tables with OIDs but no primary key
Date: 2017-12-13 10:28:54
Message-ID: CA+OCxoyXu+uEZWaG0FcuH1tqxpxBWXf6Yuv7nb4Hds3FQhn77w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Awesome job - thanks, patch applied.

On Wed, Dec 6, 2017 at 9:27 AM, Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Please find the attached updated patch with some code cleanup.
>
>
> On Wed, Dec 6, 2017 at 2:34 PM, Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave,
>>
>> On Tue, Dec 5, 2017 at 9:43 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Mon, Dec 4, 2017 at 4:01 PM, Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>>
>>>> On Sat, Dec 2, 2017 at 10:42 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Thursday, November 30, 2017, Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please find the attached updated patch.
>>>>>>
>>>>>> On Mon, Nov 27, 2017 at 5:15 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> On Thu, Nov 23, 2017 at 1:28 PM, Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please find the attached patch for RM #2849: Allow editing of data
>>>>>>>> on tables with OIDs but no primary key.
>>>>>>>>
>>>>>>>
>>>>>>> I like that if I add a new row or rows and hit Save, the OIDs are
>>>>>>> fetched immediately. However;
>>>>>>>
>>>>>>> - It marks the row as read-only. We do that currently because we
>>>>>>> don't return the key info on Save, and thus require a Refresh before any
>>>>>>> further editing. However, if we have the OID, we can edit again immediately.
>>>>>>>
>>>>>>> Fixed
>>>>>>
>>>>>>> - If we can return the new OIDs on Save, can't we do the same for
>>>>>>> primary key values? That would be consistent with OIDs, and would remove
>>>>>>> the need to disable rows, leading to a much nicer use experience I think.
>>>>>>>
>>>>>>> Implemented
>>>>>>
>>>>>
>>>>> This looks great, however there is one small issue I spotted; it looks
>>>>> like we're inserting rows in a random order. For example, in the screenshot
>>>>> attached, the last 5 rows were added in the order seen in the key1 column,
>>>>> and then I hit Save and got the id values returned. Is that caused by
>>>>> something we're doing, or the database server?
>>>>>
>>>>> The root cause of the issue is, Python dictionary does not preserve
>>>> the order. To fix this issue I have manually sorted the added rows index
>>>> and stored it into OrderedDict.
>>>> Please find the attached updated patch.
>>>>
>>>
>>> Thanks Khushboo. My apologies, but I found something else when testing.
>>> Instead of just returning and updating the values for the key columns, we
>>> should do it for all columns. This would have 2 benefits (and I suspect,
>>> might actually make the code a little more simple):
>>>
>>> Done
>>
>>> 1) We'd see the values for columns with default values.
>>>
>>> 2) We'd see the formatted values for other columns - e.g. with a JSONB
>>> column, you'll immediately see what the re-generated JSON looks like.
>>>
>>> I assume it's straightforward to update all columns rather than just the
>>> key columns?
>>>
>> The approach is same. Before I was just updating the primary keys/oids,
>> now I update all the columns of a row.
>>
>> Please find the attached updated patch.
>>
>>> Thanks!
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>> Thanks,
>> Khushboo
>>
>
>

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

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

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-12-13 11:53:33 pgAdmin 4 commit: Re-hash the way that we handle rendering of special t
Previous Message Dave Page 2017-12-13 10:28:38 pgAdmin 4 commit: Add support for editing data in tables with OIDs but