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

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

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?

Thanks!

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

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