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 1hlZ00-0000BE-F5 for pgadmin-hackers@arkaria.postgresql.org; Thu, 11 Jul 2019 13:20:36 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hlYzz-0007yl-0o for pgadmin-hackers@arkaria.postgresql.org; Thu, 11 Jul 2019 13:20:35 +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 1hlYzy-0007ye-NZ for pgadmin-hackers@lists.postgresql.org; Thu, 11 Jul 2019 13:20:34 +0000 Received: from mail-yb1-xb29.google.com ([2607:f8b0:4864:20::b29]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hlYzv-00010a-Rm for pgadmin-hackers@postgresql.org; Thu, 11 Jul 2019 13:20:34 +0000 Received: by mail-yb1-xb29.google.com with SMTP id a14so2467586ybm.11 for ; Thu, 11 Jul 2019 06:20:31 -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 :cc; bh=F71b0YTTxaN6uFlBZOZ4SAfEl412wpaeyBI75gm17tI=; b=lSFFOG5SKjZSGQcFVYg43aqgGduYMPy/ZxvlYZN/gbGQQ7RDC1joeM+/BOnyotixcP KztANvhBbZUNCvYNopZmcYWaIW/oJo9RJuY0uBcAG7WQ/+PDHpaXc3fn9g2ROYfjShbR GcR5feGsZMpKdlBSKcQs0ggFUVi5cNFD6IXf+pUl/zo4Gr+SAJYFnchyRn++XEHi2hpN lfg8ra5tbmpNfUoHV9iuvsIFnQMNy6j+gkuMi/ALvgs5Rz1fUZxfrXMT0rYXb+kjCcOE x8W2yvQSpRq/dJtq5p97ogzpTr3xfC3F9XFJSyENQsSSWokqNKMVDbDwqHej1ZI/FZBF ivMg== 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=F71b0YTTxaN6uFlBZOZ4SAfEl412wpaeyBI75gm17tI=; b=EXS+RAKzB9HdrmuzuXqKif2v9Fg5D2ovNti9pftgbANAkIqVmwes/JKvkBPAv1RNIT iHBEHILO91oojS82ItdiP3Ox7XGL/C8iREDjZFgF95BvGEA5U91CgbC2HR1o/PHqF85B ntbQ2Te5VQ7T0k5gTI2QoGa/hAvgoF8ptd5tAcb0piq0FugykVouwqGPmyMCS7T6e67r cGV/t4QotC3E6tjJ/ReFjF6wNK0XGJGaxjxQVLP+dFX4JcEvxTUZqCIAqxd32A9PcfJW uF/ZHsYCvSfG09XIgK2UVzHvspNYau2acqZSU+EcWqQbjvQMhG07UlXhoYyNEZOYW8mD Xmxw== X-Gm-Message-State: APjAAAWc4OkYG0+tZOGCy9YuV/ndh87pB3m8GjWGxYwHSFOAEwL0gxyQ zpzBi5sSjLsRgv9NiYN2oA+WN6LcjSzhWZ+Oyz4= X-Google-Smtp-Source: APXvYqzxg9/U0Iafnl9XZP9/vonUnxYkWRtEZS4QgzXhDTcGuHRz3AddirwBgcJ5RoNs21YFAnMtqZgsohxNdFCxMAI= X-Received: by 2002:a0c:d238:: with SMTP id m53mr1915434qvh.161.1562851229621; Thu, 11 Jul 2019 06:20:29 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Yosry Muhammad Date: Thu, 11 Jul 2019 15:20:17 +0200 Message-ID: Subject: Re: [GSoC] Finalized First Patch To: Khushboo Vashi Cc: Dave Page , pgadmin-hackers , Akshay Joshi Content-Type: multipart/alternative; boundary="00000000000060e0c8058d67a853" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --00000000000060e0c8058d67a853 Content-Type: text/plain; charset="UTF-8" Hi Khushboo, Please find an updated patch attached with the mentioned import line removed. On Thu, Jul 11, 2019 at 6:45 AM Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > Hi, > > On Wed, Jul 10, 2019 at 3:11 PM Yosry Muhammad wrote: > >> Hi, >> >> On Wed, Jul 10, 2019, 9:14 AM Khushboo Vashi < >> khushboo.vashi@enterprisedb.com> wrote: >> >>> Some points I missed: >>> 1. I assumed that in this patch modification in case of OIDs= True >>> (without primary key) has not considered as that is not working. >>> >> >> This is not implemented yet. I will work on that in a following patch >> soon enough. >> >> Okay. > >> 2. As we are already showing the changed Data prompt on closing the Query >>> Tool, do we really need the Uncommitted Transaction prompt? >>> >> >> This is needed when auto-commit is off. Saving changes in the data grid >> is performed as part of the ongoing transaction (or a new one if none is >> ongoing). After saving the data changes the user should still commit the >> current transaction for the changes to be commited to the database. This >> feature is also useful in general when auto-commit is off as users may >> forget to commit ongoing transactions. >> >> One thing I have noticed, when I add a new row and delete it immediately > without saving it and try to close the query tool, the uncommitted prompt > is coming. > In my opinion, it should not come, what do you think? > > We should disable the prompt if auto-commit and auto-rollback both are > enabled. > The uncommited prompt does not keep track of what the user has done so far, it only checks for the current transaction status. If a current transaction is ongoing, the prompt comes up. If you added a new row then deleted it without saving, the transaction status is not affected, you must have done a previous operation and had auto-commit turned off (probably the select statement). if auto-commit & auto-rollback are both enabled then there won't be any ongoing transaction at any point, thus, the prompt will never come up. Looking forward for any feedback ! Thanks ! -- *Yosry Muhammad Yosry* Computer Engineering student, The Faculty of Engineering, Cairo University (2021). Class representative of CMP 2021. https://www.linkedin.com/in/yosrym93/ --00000000000060e0c8058d67a853 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Khushboo,
Please fi= nd an updated patch attached with the mentioned import line removed.
<= /div>
O= n Thu, Jul 11, 2019 at 6:45 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
<= div dir=3D"ltr">Hi,

On Wed, Jul 10, 2019 at 3:11 PM Yosry Muhammad <yosrym93@gmail.com&g= t; wrote:
Hi,

On Wed, Jul 10, 2019, 9:14 AM Khushboo Vashi <khushboo.vash= i@enterprisedb.com> wrote:
Some points I missed:
1.=C2=A0 I ass= umed that in this patch modification in case of OIDs=3D True (without prima= ry key) has not considered as that is not working.
=

This is not imple= mented yet. I will work on that in a following patch soon enough.

Okay.=C2=A0
2. As we are already showi= ng the changed Data prompt on closing the Query Tool, do we really need the= Uncommitted Transaction prompt?=C2=A0
=

This is needed when auto-comm= it is off. Saving changes in the data grid is performed as part of the ongo= ing transaction (or a new one if none is ongoing). After saving the data ch= anges the user should still commit the current transaction for the changes = to be commited to the database. This feature is also useful in general when= auto-commit is off as users may forget to commit ongoing transactions.

One thing I have noti= ced, when I add a new row and delete it immediately without saving it and t= ry to close the query tool, the uncommitted prompt is coming.
In = my opinion, it should not come, what do you think?

We should disable the prompt if auto-commit and auto-rollback both are ena= bled.

The uncommited prom= pt does not keep track of what the user has done so far, it only checks for= the current transaction status. If a current transaction is ongoing, the p= rompt comes up. If you added a new row then deleted it without saving, the = transaction status is not affected, you must have done a previous operation= and had auto-commit turned off (probably the select statement).
= if auto-commit & auto-rollback are both enabled then there won't be= any ongoing transaction at any point, thus, the prompt will never come up.=

Looking forward for any feedback !
Thanks !

--
Yosry Muhamm= ad Yosry

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
= Class representative of CMP 2021.
<= a href=3D"https://www.linkedin.com/in/yosrym93/" target=3D"_blank">https://= www.linkedin.com/in/yosrym93/
--00000000000060e0c8058d67a853--