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

Hi,

On Tue, Jun 11, 2019 at 10:21 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Mon, Jun 10, 2019 at 10:51 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
> wrote:
>
>> 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).
>>
>
> What are all the conditions that prevent a query being updateable? Joins,
> aggregates, lack of pkey, computed columns, function calls....?
>
>
>> - 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]
>>
>
> 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.
>

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

>
>
>> - 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).
>>
>
> You mean the endpoint that is polled for the results, or the transaction
> status poll endpoint? I'm assuming the former.
>

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 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.

>
>
>>
>> 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/
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
*Yosry Muhammad Yosry*

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