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

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