Hi, On Tue, Jun 11, 2019 at 10:21 AM Dave Page <dp...@pgadmin.org> wrote:
> Hi > > On Mon, Jun 10, 2019 at 10:51 PM Yosry Muhammad <yosry...@gmail.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/
