Re: [GSoC] Finalized First Patch

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Yosry Muhammad <yosrym93(at)gmail(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Subject: Re: [GSoC] Finalized First Patch
Date: 2019-07-12 09:20:02
Message-ID: CA+OCxoyOOwO1SLqgBW3Sx+nYEx2pYqgp==cU2be-G_EtVv4HZQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Fri, Jul 12, 2019 at 5:57 AM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi Yosry,
>
> On Thu, Jul 11, 2019 at 6:50 PM Yosry Muhammad <yosrym93(at)gmail(dot)com> 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(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> On Wed, Jul 10, 2019 at 3:11 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
>>> wrote:
>>>
>>>> Hi,
>>>>
>>>> On Wed, Jul 10, 2019, 9:14 AM Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2019-07-12 09:37:44 pgAdmin 4 commit: Fix dropping of pgAgent schedules through the Job pro
Previous Message Dave Page 2019-07-12 09:16:20 pgAdmin 4 commit: Fix an error that could be seen when editing column p