Re: [GSoC] Finalized First Patch

From: Yosry Muhammad <yosrym93(at)gmail(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, 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:53:15
Message-ID: CAFSMqn_27bbwSrme46k7L20UCDMcJ3-x22-5xGKW6bhPofpOKg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

I will look into it and get back to you.
Thanks !

On Fri, Jul 12, 2019, 11:20 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> 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 10:00:51 Re: [pgAdmin4][Patch]: RM 3996 "Error dropping Schedule" message displayed if drop schedule through property section
Previous Message Khushboo Vashi 2019-07-12 09:46:09 [pgAdmin4][Patch] : RE-SQL tests for Collation node