Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hahPi-0004Zk-H6 for pgadmin-hackers@arkaria.postgresql.org; Tue, 11 Jun 2019 14:06:14 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hahPg-0006sh-1z for pgadmin-hackers@arkaria.postgresql.org; Tue, 11 Jun 2019 14:06:12 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hahPf-0006sV-Dg for pgadmin-hackers@lists.postgresql.org; Tue, 11 Jun 2019 14:06:11 +0000 Received: from mail-qt1-x829.google.com ([2607:f8b0:4864:20::829]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hahPb-0002aF-7O for pgadmin-hackers@postgresql.org; Tue, 11 Jun 2019 14:06:09 +0000 Received: by mail-qt1-x829.google.com with SMTP id n11so12510521qtl.5 for ; Tue, 11 Jun 2019 07:06:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=dNPVUSc8OOZULd4J7R1HxBNzPjTuORPsBzYSfOXKQu8=; b=msKsf78wYDriAlw08/IkAMKR9mZoDTsUq4mmxWiuobi9mAWBbV/+bjQVOyH7coEd+W q60L6/0XXDWV9W25RILJqP6gzeRDBIv6cM7HqgOy0CC1Y8wQ3KEZHFNXw97iossRxV7h /bvP/lRa1A+mTYHl2u1UrLF4BQb6uQJwAUMA2auM/FR6KyA4cDEl0vAVIi5pJdH4ychf YTvJNQVgXtXRhxipld4shMIays7lVCoB77EC2QuXVWOatFhYOEiNLFWCy6fYswkCSS2x fsoasTBzMOVBKGg2EM/ccOn4tOJdHb0aU3znqTX5qIKwiYrJyNMUDO5tVlCRjj8hORSr sDWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=dNPVUSc8OOZULd4J7R1HxBNzPjTuORPsBzYSfOXKQu8=; b=Z7kXxYXGT8LJ9a+ZCUxMjaGeikDsWpRx3mWo3r+Tjj3eRCkx08YgPR5Jb3xOM7fWm1 /OOlkE9miNglZnaw0QA8AFiTnBQ2iEJ2KZspEq+z3gdhYmGoo4oEvuyBwfGYuXQ8HUiS R9eukzEAIsdN7mlFY8xWdMw1mF0gfyU/ezP5Is5QwitIqGkzv48QT0WW2LueNBOrlxmW GN6i7A+bLjoIIWEiuuD6mjaiM+M8rpgyzKh18S/NR9TUmJixpIgoYLDIysX3TqXf5E31 /oSH4kP0QIo+v8EcmOvTp+loMERUkQsBI4/KpUv8q5JqggKPcbVgk62Y2h3ZF0VF63RK +5fQ== X-Gm-Message-State: APjAAAXWTsDSq/ClErDfb2yJR8YfdOsbT0gaX6MYaZE3dEn7gZZ7AdZD FGdlQJag6QB7+a4+Z+9zz/H5KkBgTUmCjgSA7rU= X-Google-Smtp-Source: APXvYqxtUtRHUFWro8Y8HVRFH7M8IM8QnHYccGjveOGyEAETrr1B9HtaqchClWSOJ76neAu93zRKq4WqB2ra3bm6POA= X-Received: by 2002:ac8:373b:: with SMTP id o56mr66499636qtb.133.1560261966199; Tue, 11 Jun 2019 07:06:06 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Yosry Muhammad Date: Tue, 11 Jun 2019 16:05:54 +0200 Message-ID: Subject: Re: [GSoC] Check if a query resultset is update-able To: Dave Page , pgadmin-hackers@postgresql.org Content-Type: multipart/alternative; boundary="00000000000040907b058b0ccc9a" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000040907b058b0ccc9a Content-Type: text/plain; charset="UTF-8" Hi, On Tue, Jun 11, 2019 at 10:21 AM Dave Page wrote: > Hi > > On Mon, Jun 10, 2019 at 10:51 PM Yosry Muhammad > 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/ --00000000000040907b058b0ccc9a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

On Tue, Jun 11, 2019 at 10:21 AM Dave Page <dpage@pgadmin.org&g= t; wrote:
Hi

On Mon, Jun 10, 2019 at 10:51 PM Yosry Muhammad <yosrym93@gmail.com&g= t; 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 aggregat= ions).

What are all the conditi= ons that prevent a query being updateable? Joins, aggregates, lack of pkey,= computed columns, function calls....?
=C2=A0
- When the query = results is ready (polling is successful) I can check the results in the bac= k-end using the transaction connection cursor.
- The transaction = cursor description attribute includes a list of Column objects, each of whi= ch has attributes pointing to its original table in the system catalog tabl= e pg_class (if available) and its attribute number within the table.= [1]

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

- Is Column.table_oid the sam= e for all columns?
- Is Column.table_column present for all colum= ns?

then:

- Do we have al= l the primary key columns in the resultset?

In fac= t, we could even support queries such as:

SELECT c= ol1 || col2, col1, col2 FROM table;

because (assum= ing col1 or col2 is the pkey) we can just make the first column read-only i= n the grid, and only allow editing of the second and third. That requires t= hat the table_oid matches of course.
According to my understanding, Column.table_oid=20 is the same for all columns belonging to the same table and is None if=20 the column does not directly reference an original column in a table (an aggregation or a result of a mathematical operation or concatenation=20 for example). It follows that Column.table_column is only present for=20 the columns that have a table_oid attribute that isn't None.
=
For the first version, the query resultset can be update-abl= e if (and only if):
- All the columns either belong to the same t= able or no tables (such as the concatenation case you pointed out).
- All the primary keys of that column are available.
columns t= hat do not belong to the table can be made read-only.

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

<= /div>

=C2=A0
=C2=A0
- From thi= s 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]
<= /div>
- This can be used as an indicator to whether the resultset is up= datable (similar to the View Table mode, where tables are only editable if = they have primary keys).

I will modify the fol= lowing parts in the code:
1- web/tools/sqleditor/command.py
QueryToolCommand class will be modified to contain an attri= bute indicating whether the query results are update-able for the last succ= essful query.

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

3- web/tools/sq= leditor/__init__.py
The poll endpoint will be modified t= o check if the results are update-able (in case the results are ready), the= n the session object primary keys and the transaction object can_edit attri= bute will be updated (the primary keys are checked in the frontend, if they= exist table modifications are allowed).

<= /div>
You mean the endpoint that is polled for the results, or the tran= saction status poll endpoint? I'm assuming the former.

I did mean the endpoint that is poll= ed for the results indeed.
=C2=A0
=C2=A0<= /div>

This is the first step, to check if a query resultset is u= pdate-able. The upcoming steps will include switching the mode in the front= end to allow for editing the results and checking what options should be en= abled or disabled and any needed modifications (I think allowing for only e= diting 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 th= e query, but the user can fix that (we should probably not just make the gr= id read-only if we detect that, unless we can come up with a nice way to te= ll the user why they can't edit anything).

Inserting the rows can be supported I didn't= =20 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=20 implementation. It can be extended later according to what you see=20 suitable, for example allow editing result-sets containing columns from=20 different tables).

Looking forward for your feedba= ck to start working on that at once.

=C2=A0
=C2=A0

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

[3] https://www.postgresql.org/docs/current/catalog-pg-attribute.html

--
= Yosry Muhammad Yosry

=
Computer Engineering student,<= /div>
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.


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

EnterpriseDB = UK: http://www.en= terprisedb.com
The Enterprise PostgreSQL Company


--
Yosry Muhammad Yosry

Computer Engin= eering student,
The Faculty of Enginee= ring,
Cairo University (2021).
Class representative of CMP 2021.
https://www.linkedin.com/in/yosrym93/
--00000000000040907b058b0ccc9a--