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

Hi

On Tue, Jun 11, 2019 at 3:06 PM Yosry Muhammad <yosrym93(at)gmail(dot)com> wrote:

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

I assume you mean table, and not column.

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

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.

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

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!

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

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

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