Thread: [pgAdmin4][Patch]: RM #2849 - Allow editing of data on tables withOIDs but no primary key

Hi

On Thursday, November 30, 2017, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached updated patch.

On Mon, Nov 27, 2017 at 5:15 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, Nov 23, 2017 at 1:28 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.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?

Thanks! 

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

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

Hi

On Mon, Dec 4, 2017 at 4:01 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

On Sat, Dec 2, 2017 at 10:42 AM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thursday, November 30, 2017, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached updated patch.

On Mon, Nov 27, 2017 at 5:15 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, Nov 23, 2017 at 1:28 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.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

Hi Dave,

On Tue, Dec 5, 2017 at 9:43 AM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Dec 4, 2017 at 4:01 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

On Sat, Dec 2, 2017 at 10:42 AM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thursday, November 30, 2017, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:

Hi,

Please find the attached updated patch.

On Mon, Nov 27, 2017 at 5:15 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, Nov 23, 2017 at 1:28 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.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