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 1hac2L-0000r2-Om for pgadmin-hackers@arkaria.postgresql.org; Tue, 11 Jun 2019 08:21:46 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hac2K-0003UC-Bi for pgadmin-hackers@arkaria.postgresql.org; Tue, 11 Jun 2019 08:21:44 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hac2J-0003Qt-Qy for pgadmin-hackers@lists.postgresql.org; Tue, 11 Jun 2019 08:21:44 +0000 Received: from mail-it1-x144.google.com ([2607:f8b0:4864:20::144]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hac2G-0002cM-PL for pgadmin-hackers@postgresql.org; Tue, 11 Jun 2019 08:21:43 +0000 Received: by mail-it1-x144.google.com with SMTP id e25so3368120itk.3 for ; Tue, 11 Jun 2019 01:21:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4bXS2QQhy4wD1pzzo6kOy+zp8JhCV0dcwq1tMvdzlt8=; b=TSgu6LYBlU+K35m31YEpygypGCqft9JW0n6Yz4ry1+fM1vgyoVB1h44V+FA9R/gOdG 5EM0thI3qpmIF586aLOQ9l8nGZ15Ni52VOypz0HWI/995j/JZ09NYgMJjI9ZOPKJO6+p qPauQdL6FK75KZBkmtdnEYopa3kZwPyM6q3921kJ+uK/Xk4Xn+LNaOl1Ikyqw8JDAUQ7 o5MET5ZOENSg6Jk+NTq7RDRlY6R2Lur2e5ASeXlgdKVeiiRTCADcqtPWOyByd1xrejYa 7dJ2xHL9I1zzSMCNhNZq6Fw6DdkhRTtjSTCEP1re+0qV8Rd7P4foNoTX7sJN/nq8Btr6 j8GA== 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:cc; bh=4bXS2QQhy4wD1pzzo6kOy+zp8JhCV0dcwq1tMvdzlt8=; b=XVQ1JXtyDYdFLI5eELd17pY5AkhUTiSjAE7DYPGAh+p7iaOrsjGW4/BMjdT9Bo6wUK tvmw6X70El8avQ7BM/coDEn1Jei8pg8C7BYOlsGXaWRMiYca+P50ptcRCL0wE+p9GTIw chujlN4j8XaSMRr2AIZ7+OABgTNzgxrTnGC0+DMoHOsA7zK9zNKa6e6LXFXVa7ljtKeR AZtMQ50LRDX3z26vwZVwznng0T5arDLpstAsJZBO1IaGKn1pxNpM5vhEhvcd4x7T5+KH sG1ij57P3Sdp4jUeHARHElzXaQpHiNzhojk1fLAExYs6fK+90pz1M9ro17bmf6lEcvPY BcPg== X-Gm-Message-State: APjAAAXBMWxYR8dpk9kiYtjrLh1RPo2ZTj863L7apgqkt66BuGW26GeI lGWBrVyNB2SAVmaPugaKPPFPggn4osfH8W8wcZhx/g== X-Google-Smtp-Source: APXvYqzksEcQfEaNOc06JYFB4rJs6jbnospVwYiAeSuN55yre+DHWPH23Nu8gDxcD05Tj1mF8FC45TXLKShOpHdd4Q0= X-Received: by 2002:a24:3a42:: with SMTP id m63mr18747154itm.29.1560241298754; Tue, 11 Jun 2019 01:21:38 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Tue, 11 Jun 2019 09:21:27 +0100 Message-ID: Subject: Re: [GSoC] Check if a query resultset is update-able To: Yosry Muhammad Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000606e4b058b07fc05" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000606e4b058b07fc05 Content-Type: text/plain; charset="UTF-8" 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. > - 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. > > 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). > > 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 --000000000000606e4b058b07fc05 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Jun 10, 2019 at 10:51 PM Yosry Muhammad <= ;yosrym93@gmail.com> wrote:
<= div>Dear all,

After some research and review of th= e 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 implementin= g this technique:

- Query results are not be updat= e-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 pre= vent a query being updateable? Joins, aggregates, lack of pkey, computed co= lumns, function calls....?
=C2=A0
- When the query results is r= eady (polling is successful) I can check the results in the back-end using = the transaction connection cursor.
- The transaction cursor descr= iption attribute includes a list of Column objects, each of which has attri= butes pointing to its original table in the system catalog table pg_clas= s (if available) and its attribute number within the table. [1]

Hmm, at first glance it seems to me th= at psycopg2.extensions.Column might make this much easier than previously e= xpected:

- Is Column.table_oid the same for all co= lumns?
- Is Column.table_column present for all columns?

then:

- Do we have all the primar= y 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 tabl= e_oid matches of course.
=C2=A0
- From this information, the sy= stem catalog tables pg_class, pg_attribute and pg_constraint = can be queried to check that all the columns belong to a single table and t= hat all the primary keys are available. [2][3][4]
- This can = be used as an indicator to whether the resultset is updatable (similar to t= he View Table mode, where tables are only editable if they have primary key= s).

I will modify the following parts in the c= ode:
1- web/tools/sqleditor/command.py
Query= ToolCommand class will be modified to contain an attribute indicating wheth= er the query results are update-able for the last successful query.

2- A new file will be added in web/tools/sqleditor/uti= ls/ containing the function that will check if the query results are up= date-able.

3- web/tools/sqleditor/__init__.py <= br>
The poll endpoint will be modified to check if the result= s 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 modifica= tions are allowed).

You mean th= e endpoint that is polled for the results, or the transaction status poll e= ndpoint? I'm assuming the former.
=C2=A0

Thi= s is the first step, to check if a query resultset is update-able. The upco= ming steps will include switching the mode in the frontend to allow for edi= ting the results and checking what options should be enabled or disabled an= d 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, the= y may fail if a not null column isn't present in the query, but the use= r can fix that (we should probably not just make the grid read-only if we d= etect that, unless we can come up with a nice way to tell the user why they= can't edit anything).
=C2=A0

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

[3] <= a href=3D"https://www.postgresql.org/docs/current/catalog-pg-attribute.html= " target=3D"_blank">https://www.postgresql.org/docs/current/catalog-pg-attr= ibute.html

--
<= div dir=3D"ltr" class=3D"gmail-m_97502454872681207gmail_signature">
Yosry Muhammad Yosry<= /div>

Computer Engineering student,
The Faculty of Engineering,
Cai= ro University (2021).
Class representat= ive of CMP 2021.


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL = Company
--000000000000606e4b058b07fc05--