Hi On Tue, Jun 11, 2019 at 3:06 PM Yosry Muhammad <yosry...@gmail.com> wrote:
> 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. > 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
