Subject: [GSoC] Check if a query resultset is update-able

From: Yosry Muhammad <[email-masked]>
Date: Mon, 10 Jun 2019 23:51:04 +0200

Dear all,

After some research and review of the code, I have arrived at this method
to check whether a query resultset is update-able. I would like to know
your feedback before I start implementing this technique:

- Query results are not be update-able (initially at least) except if all
the primary keys are selected and all the columns belong to the same table
(no joins or aggregations).
- When the query results is ready (polling is successful) I can check the
results in the back-end using the transaction connection cursor.
- The transaction cursor description attribute includes a list of Column
objects, each of which has attributes pointing to its original table in the
system catalog table *pg_class* (if available) and its attribute number
within the table. [1]
- From this information, the system catalog tables *pg_class, pg_attribute *and
*pg_constraint *can be queried to check that all the columns belong to a
single table and that all the primary keys are available. [2][3][4]
- This can be used as an indicator to whether the resultset is updatable
(similar to the View Table mode, where tables are only editable if they
have primary keys).

I will modify the following parts in the code:
1- *web/tools/sqleditor/command.py*
QueryToolCommand class will be modified to contain an attribute indicating
whether the query results are update-able for the last successful query.

2- A new file will be added in* web/tools/sqleditor/utils/* containing the
function that will check if the query results are update-able.

3-
*web/tools/sqleditor/__init__.py *
The poll endpoint will be modified to check if the results are update-able
(in case the results are ready), then the session object primary keys and
the transaction object can_edit attribute will be updated (the primary keys
are checked in the frontend, if they exist table modifications are allowed).

This is the first step, to check if a query resultset is update-able. The
upcoming steps will include switching the mode in the frontend to allow for
editing the results and checking what options should be enabled or disabled
and any needed modifications (I think allowing for only editing and
deleting rows makes sense).

Sorry for the long email, looking forward to your feedback!

[1] http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.Column
[2] https://www.postgresql.org/docs/current/catalog-pg-class.html
[3] https://www.postgresql.org/docs/current/catalog-pg-attribute.html
[4] https://www.postgresql.org/docs/current/catalog-pg-constraint.html

-- 
*Yosry Muhammad Yosry*

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.
https://www.linkedin.com/in/yosrym93/

---

From: Dave Page <[email-masked]>
Date: Tue, 11 Jun 2019 09:21:27 +0100

Hi

> (no joins or aggregations).

What are all the conditions that prevent a query being updateable? Joins,
aggregates, lack of pkey, computed columns, function calls....?

> system catalog table *pg_class* (if available) and its attribute number

Hmm, at first glance it seems to me that psycopg2.extensions.Column might
make this much easier than previously expected:

- Is Column.table_oid the same for all columns?
- Is Column.table_column present for all columns?

then:

- Do we have all the primary key columns in the resultset?

In fact, we could even support queries such as:

SELECT col1 || col2, col1, col2 FROM table;

because (assuming col1 or col2 is the pkey) we can just make the first
column read-only in the grid, and only allow editing of the second and
third. That requires that the table_oid matches of course.

> - From this information, the system catalog tables *pg_class,
> pg_attribute *and *pg_constraint *can be queried to check that all the
> columns belong to a single table and that all the primary keys are
> available. [2][3][4]
> 2- A new file will be added in* web/tools/sqleditor/utils/* containing
> the function that will check if the query results are update-able.
>
> 3-

You mean the endpoint that is polled for the results, or the transaction
status poll endpoint? I'm assuming the former.

> deleting rows makes sense).

I don't see any obvious reason why we couldn't add rows as well. Of course,
they may fail if a not null column isn't present in the query, but the user
can fix that (we should probably not just make the grid read-only if we
detect that, unless we can come up with a nice way to tell the user why
they can't edit anything).

> [1]
> http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.Column

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

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

---

From: Yosry Muhammad <[email-masked]>
Date: Tue, 11 Jun 2019 16:05:54 +0200

Hi,

> third. That requires that the table_oid matches of course.

According to my understanding, Column.table_oid is the same for all columns
belonging to the same table and is None if the column does not directly
reference an original column in a table (an aggregation or a result of a
mathematical operation or concatenation for example). It follows that
Column.table_column is only present for the columns that have a table_oid
attribute that isn't None.

For the first version, the query resultset can be update-able if (and only
if):
- All the columns either belong to the same table or no tables (such as the
concatenation case you pointed out).
- All the primary keys of that column are available.
columns that do not belong to the table can be made read-only.

Please keep in mind that including columns that do not belong directly to
the table will require more modifications in the front-end part when
modifying the table (to drop any read-only columns and so).

>> QueryToolCommand class will be modified to contain an attribute
>> indicating whether the query results are update-able for the last
>> successful query.
>>
>> The poll endpoint will be modified to check if the results are
>> update-able (in case the results are ready), then the session object
>> primary keys and the transaction object can_edit attribute will be updated
>> (the primary keys are checked in the frontend, if they exist table
>> modifications are allowed).
>>
>

I did mean the endpoint that is polled for the results indeed.

> I don't see any obvious reason why we couldn't add rows as well. Of
> course, they may fail if a not null column isn't present in the query, but
> the user can fix that (we should probably not just make the grid read-only
> if we detect that, unless we can come up with a nice way to tell the user
> why they can't edit anything).
>

Inserting the rows can be supported I didn't look into this part yet, was
just letting you know of the upcoming steps in my point of view. For now I
will work on the aforementioned implementation. It can be extended later
according to what you see suitable, for example allow editing result-sets
containing columns from different tables).

Looking forward for your feedback to start working on that at once.

-- 
*Yosry Muhammad Yosry*

---

From: Dave Page <[email-masked]>
Date: Tue, 11 Jun 2019 16:47:06 +0100

Hi

On Tue, Jun 11, 2019 at 3:06 PM Yosry Muhammad <[email-masked]> wrote:

>>> After some research and review of the code, I have arrived at this
>>> method to check whether a query resultset is update-able. I would like to
>>> know your feedback before I start implementing this technique:
>>>
>>> - Query results are not be update-able (initially at least) except if
>>> all the primary keys are selected and all the columns belong to the same
>>> table (no joins or aggregations).
>>>
>>
>>> - When the query results is ready (polling is successful) I can check
>>> the results in the back-end using the transaction connection cursor.
> According to my understanding, Column.table_oid is the same for all
> columns belonging to the same table and is None if the column does not
> directly reference an original column in a table (an aggregation or a
> result of a mathematical operation or concatenation for example). It
> follows that Column.table_column is only present for the columns that have
> a table_oid attribute that isn't None.
>
> - All the columns either belong to the same table or no tables (such as
> the concatenation case you pointed out).

I assume you mean table, and not column.

> modifying the table (to drop any read-only columns and so).

I'd start by handling the "only columns from the table" case. Handling
individual read-only columns can be handled if/when there is time.

> I did mean the endpoint that is polled for the results indeed.

:-)

>
>
>>
>>
>>>
>>> This is the first step, to check if a query resultset is update-able.
>>> The upcoming steps will include switching the mode in the frontend to allow
>>> for editing the results and checking what options should be enabled or
>>> disabled and any needed modifications (I think allowing for only editing
>>> and deleting rows makes sense).
>>>
>>

I would suggest working on IUD for resultsets that are entirely from a
single table. I think that insert is as important as update and delete, and
less important that supporting cases like the example I gave.

Otherwise, sounds good to me!