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 1hlrjz-0005xE-77 for pgadmin-hackers@arkaria.postgresql.org; Fri, 12 Jul 2019 09:21:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hlrj0-0004EB-3B for pgadmin-hackers@arkaria.postgresql.org; Fri, 12 Jul 2019 09:20:18 +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 1hlriz-0004E4-Ro for pgadmin-hackers@lists.postgresql.org; Fri, 12 Jul 2019 09:20:17 +0000 Received: from mail-wr1-x442.google.com ([2a00:1450:4864:20::442]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hlrix-0001ss-5X for pgadmin-hackers@postgresql.org; Fri, 12 Jul 2019 09:20:17 +0000 Received: by mail-wr1-x442.google.com with SMTP id x4so9175396wrt.6 for ; Fri, 12 Jul 2019 02:20:14 -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=pN5PCKlpePGU/RTLfTcR2eYBaagrwGP9r8aJjeTj9Hc=; b=pORqXmGqNUNKUjYh4eUh4CuqanjoymxhNsutrA+uxBN5ain+F6WliWvXJPbjTUFine Ct+zGshA/NQvzKOOprnwY+wRbtovbCOUAGRAtDXdA5Ezevgp7TeCfJEbus3V6JB5MXP5 FnIa0giNQiyCoUc7h2pGAR91KTXyl6OKSVr42wsG6mR/i5ni3i7rUV1IcVREhXHb6P43 0KjAa+ld8XIUGrCVOD5NNk8sZ1Ya9u/z9vE0QXWjCtWCrGygYhP+PLJ51F0zERNuUp9Q 4zboilQ7Pq0B9k++YN+rESzR5rpd0p+p1HdX8Ho4lh6D22oXLj3FVt6O9GQHdvXCsE5w KbRQ== 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=pN5PCKlpePGU/RTLfTcR2eYBaagrwGP9r8aJjeTj9Hc=; b=MzI2BoysvC6BltO1xJLLs2lM46aqXRGsBowEL9h1uviiihdhRMRV8eD3gjJaC7uAY0 z8EXd9feiu+XxJck+Z4uWlx47+kOIU+ySp4HdV7ao7CSb90KZYhw9MsaXiNOSTftS/m9 ykEP70I/WXz0XiChF8eDFKzT6zYq+6u64NQxL5pIq2AWcFG55l0QBwCqvQyHkygjymwu ck3DOj+dP1CvSDHFj+Z7EsZHCeUpjewsUz2RjYQPTiVxtgoqmEtuL3zBep1to3sbljO2 v/zYg4BbAlxIHf0uW6MEyR7ohd/UPxaZQG0OErkhgdWtNc0S3/FwHHtkFeKKlayUABH5 7GMA== X-Gm-Message-State: APjAAAVUeNwDw+UvbPP68+5h6PP3+GCDQxzxGZeaIue2j4Py84new4TV b+56iFznb21qwUyfKHb1BSKwZcEbPPJXZsa6tnzhiA== X-Google-Smtp-Source: APXvYqxC0EkXDIB1kKgaAl3r/usQ7LZ2LWX5OlmjBNp0spdl2Q/rmcG+U2HTp5lWtImP9Ew2zkfC/7Ow5hh7jAHLwVA= X-Received: by 2002:adf:f104:: with SMTP id r4mr11076811wro.140.1562923213925; Fri, 12 Jul 2019 02:20:13 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Fri, 12 Jul 2019 10:20:02 +0100 Message-ID: Subject: Re: [GSoC] Finalized First Patch To: Khushboo Vashi Cc: Yosry Muhammad , pgadmin-hackers , Akshay Joshi Content-Type: multipart/alternative; boundary="000000000000fa4737058d786a2c" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000fa4737058d786a2c Content-Type: text/plain; charset="UTF-8" On Fri, Jul 12, 2019 at 5:57 AM Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > Hi Yosry, > > On Thu, Jul 11, 2019 at 6:50 PM Yosry Muhammad wrote: > >> Hi Khushboo, >> Please find an updated patch attached with the mentioned import line >> removed. >> >> Looks good to me. > >> 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. >> >> Exactly, my point is. It should not prompt if auto-commit & auto-rollback > both are enabled, but it is coming. Please see the attached video. > Agreed - this should be fixed. If auto-commit is turned off, it should also prompt to commit if the user hit Save in the prior step I think. Maybe reversing that prompt makes more sense in general - prompting to save rather than discard is quite normal; think about text editors etc. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --000000000000fa4737058d786a2c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Fri, Jul 12, 2019 at 5:57 AM Khush= boo Vashi <khushboo.v= ashi@enterprisedb.com> wrote:
Hi Yosry,

<= div class=3D"gmail_quote">
On Thu, Jul= 11, 2019 at 6:50 PM Yosry Muhammad <yosrym93@gmail.com> wrote:
Hi Khushboo,
Please find an updated patch attached with th= e mentioned import line removed.

Loo= ks good to me.=C2=A0=C2=A0
On Thu, Jul 11, 2019 at 6:45 AM Khushboo Vashi <khushboo.vash= i@enterprisedb.com> wrote:
Hi,

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

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

This is not implemented yet. I will work on that in a fo= llowing patch soon enough.

Okay.=C2=A0
2. As we are already showing the changed Data prompt on closing t= he Query Tool, do we really need the Uncommitted Transaction prompt?=C2=A0<= /div>

This is needed when auto-commit is off. Saving changes in the data g= rid is performed as part of the ongoing transaction (or a new one if none i= s ongoing). After saving the data changes the user should still commit the = current transaction for the changes to be commited to the database. This fe= ature 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 uncommi= tted prompt is coming.
In my opinion, it should not come, what do= you think?

We should disable the prompt if auto-c= ommit 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 c= urrent 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 (probab= ly 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.

<= /blockquote>
Exactly, my point is. It should not prompt if auto-commit = & auto-rollback both are enabled, but it is coming. Please see the atta= ched video.

Agreed - this= should be fixed.=C2=A0

If auto-commit is turned o= ff, it should also prompt to commit if the user hit Save in the prior step = I think. Maybe reversing that prompt makes more sense in general - promptin= g to save rather than discard is quite normal; think about text editors etc= .
=C2=A0
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Enterpri= seDB UK: http://w= ww.enterprisedb.com
The Enterprise PostgreSQL Company
--000000000000fa4737058d786a2c--