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 1hlsFA-0007FJ-Ja for pgadmin-hackers@arkaria.postgresql.org; Fri, 12 Jul 2019 09:53:32 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hlsF9-0005Wm-8h for pgadmin-hackers@arkaria.postgresql.org; Fri, 12 Jul 2019 09:53:31 +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 1hlsF8-0005Wf-OE for pgadmin-hackers@lists.postgresql.org; Fri, 12 Jul 2019 09:53:31 +0000 Received: from mail-qt1-x843.google.com ([2607:f8b0:4864:20::843]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hlsF6-0004Aw-BR for pgadmin-hackers@postgresql.org; Fri, 12 Jul 2019 09:53:29 +0000 Received: by mail-qt1-x843.google.com with SMTP id h18so7423688qtm.9 for ; Fri, 12 Jul 2019 02:53:28 -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=tAblWs4eFvbWcc/H9r+kaV4tt9GxF3B6KFEF5zPPbMY=; b=htlPfP65bT420XJwgSOJr6xTDwFsRnCAn9/nQrRxIpEvEU7tVbdNesNyeueVyvnHgI +ys2KOWllsS6j0EqwW69FO1LpVwZKF5li6pePbC4AvzJauIE+l+jvDK1tRPUMV/8V07v RsSRg/fon4prsP8yHSMyhkTLISqee0oLB4sDenzu9fJRKYMwXQKUdcEgdY4cYiEX4lPc jsI0wL34fBt/mccYkQAm7f+O1rYx8uJiPTrkS5Yt5AXACfED8+BE++vxH9NZPZCiVCAQ kStEHVGfO29gUZ+GM5q1GQWw6bdSiy6Xtd30jPK5EQHd9DlXogXSCgrcl3W8h2ULSzB+ wfUA== 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=tAblWs4eFvbWcc/H9r+kaV4tt9GxF3B6KFEF5zPPbMY=; b=CeMb4sa1BcCnkwc45JsKGH0lh6vjFnMiSGQkwt2pXwlUoGtXqNFl7djC8AKgt01V2W wfl4TG3UUyFIWScofMzR+nrPQFYW/WtjR5qU2nH4zo3YLC/L5Me6YVYNiSBSEkjhYV0d sOn5K55pKwIXJN+TE2JtRI1vJgI+vBQNlsBHFRQq2poQHzduFa9r06LPVB4j3sMF02Kv BDKCobILR1P3+rm8pxdekObZLALmAaeNMDf4ptm6EbFilxO/pDQOJxxN5a9LztnOSLU4 Nd8Z8EOW9a8TMhyI5onkKBroGL80KwcfxFevJKWmBv24Mi9m6ia9FZFwiPmdHCGYHfxQ UCBQ== X-Gm-Message-State: APjAAAXWwiPuipZ6P6J2UTAiZowT1SHy+LB5HM0fwYEj1IFb5rPuD2qG YTReGwvZTCTCKwkptXnTFCB/FbtgMNGNddshsJg= X-Google-Smtp-Source: APXvYqxe7bWSMonhp6RUiG1dvAm+YHvr6JsPtOmvVaRWnGCab5p8nbYhG22WR78aoxqJ9QBEhLZdgfJdazX2zkissXo= X-Received: by 2002:a0c:d238:: with SMTP id m53mr5481462qvh.161.1562925207203; Fri, 12 Jul 2019 02:53:27 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Yosry Muhammad Date: Fri, 12 Jul 2019 11:53:15 +0200 Message-ID: Subject: Re: [GSoC] Finalized First Patch To: Dave Page Cc: Khushboo Vashi , pgadmin-hackers@postgresql.org, Akshay Joshi Content-Type: multipart/alternative; boundary="000000000000c932f8058d78e1cc" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000c932f8058d78e1cc Content-Type: text/plain; charset="UTF-8" I will look into it and get back to you. Thanks ! On Fri, Jul 12, 2019, 11:20 AM Dave Page wrote: > > > 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 > --000000000000c932f8058d78e1cc Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I will look=C2=A0into it and get=C2=A0back to you.
Thanks !

On Fri, Jul 12, 2019, 11:20 AM Dave Page <dpage@pgadmin.org> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">

<= br>
On Fri,= Jul 12, 2019 at 5:57 AM Khushboo Vashi <khushboo.vashi@ent= erprisedb.com> wrote:
Hi Yosry,

On Thu, Jul 11, 201= 9 at 6:50 PM Yosry Muhammad <yosrym93@gmail.com> wrote:
=
Hi Khushboo,
Please find an updated patch attac= hed with the mentioned import line removed.

Looks good to me.=C2=A0=C2=A0
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 <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 o= f OIDs=3D True (without primary key) has not considered as that is not work= ing.

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

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

= 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 ongoi= ng). After saving the data changes the user should still commit the current= transaction for the changes to be commited to the database. This feature i= s also useful in general when auto-commit is off as users may forget to com= mit ongoing transactions.

One thing I have noticed, when I add a new row and delete it immedi= ately without saving it and try to close the query tool, the uncommitted pr= ompt is coming.
In my opinion, it should not come, what do you th= ink?

We should disable the prompt if auto-commit a= nd auto-rollback both are enabled.

<= /div>
The uncommited prompt does not keep track of what the user has do= ne 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 de= leted it without saving, the transaction status is not affected, you must h= ave done a previous operation and had auto-commit turned off (probably the = select statement).
if auto-commit & auto-rollback are both en= abled then there won't be any ongoing transaction at any point, thus, t= he prompt will never come up.

Exactly, my point is. It should not prompt if auto-commit & a= uto-rollback both are enabled, but it is coming. Please see the attached vi= deo.

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

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 sa= ve rather than discard is quite normal; think about text editors etc.
=
=C2=A0
--
Dave Page
Blog: http://pgsnake.blogspot.com
= Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com=
The Enterprise PostgreSQL Company
--000000000000c932f8058d78e1cc--