Re: [pgAdmin4][Patch]: RM #3607 Edit Data: Not able to remove filter SQL condition

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM #3607 Edit Data: Not able to remove filter SQL condition
Date: 2018-09-14 13:18:55
Message-ID: CA+OCxozMXQvCLKDc1uEC4hunGVBadkt61pQL-pDbcrPpTxYtNw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, patch applied!

On Thu, Sep 13, 2018 at 7:10 AM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

>
>
> On Wed, Sep 12, 2018 at 10:38 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On 12 Sep 2018, at 17:30, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
>> wrote:
>>
>>
>>
>> On Wed, 12 Sep 2018, 20:34 Dave Page, <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Wed, Sep 12, 2018 at 3:42 PM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>>
>>>> On Wed, 12 Sep 2018, 19:36 Dave Page, <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Wed, Sep 12, 2018 at 2:48 PM, Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, 12 Sep 2018, 18:31 Dave Page, <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> On Wed, Sep 12, 2018 at 10:03 AM, Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Sep 11, 2018 at 7:10 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Tue, Sep 11, 2018 at 10:12 AM, Akshay Joshi <
>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Hackers,
>>>>>>>>>>
>>>>>>>>>> Attached is the patch to fix the following for filter dialog:
>>>>>>>>>>
>>>>>>>>>> - OK button should be disabled for Filter dialog when filter
>>>>>>>>>> string is empty. Filter dialog should be opened using context
>>>>>>>>>> menu View Data -> Filtered Rows or shortcut toolbar.
>>>>>>>>>> - On validation error dialog should not be closed.
>>>>>>>>>> - User should be able to clear the filter string from the
>>>>>>>>>> Query Tool.
>>>>>>>>>> - Validation error should be thrown when filter dialog opened
>>>>>>>>>> from Query Tool and provide some wrong filter string.
>>>>>>>>>>
>>>>>>>>>> Please review it.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This works better now, but I think there are still some related
>>>>>>>>> issues. If I clear the filter, then the filter/sort button stays blue in
>>>>>>>>> this mode; and that is because we sort by default.
>>>>>>>>>
>>>>>>>>> However; when we open in first/last/view modes, we also sort by
>>>>>>>>> default, but *don't* colour the button.
>>>>>>>>>
>>>>>>>>> I think that the button should always be blue if there is sorting
>>>>>>>>> and/or filtering in place, and that the standard "View" option should *not*
>>>>>>>>> sort at all by default.
>>>>>>>>>
>>>>>>>>> Make sense?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Attached is the modified patch where I have added following:
>>>>>>>>
>>>>>>>> - When user click on View Data -> All Row, data will not sorted
>>>>>>>> by default, when filter dialog opens from query tool it won't show anything
>>>>>>>> (empty dialog).
>>>>>>>> - When user click on View Data -> All Row, and later change the
>>>>>>>> limit then data will be sorted based on primary keys.
>>>>>>>> - When user click on View Data -> First/Last 100 rows then it
>>>>>>>> will be sorted on primary keys and when filter dialog opens from query tool
>>>>>>>> then it shows the sorting columns.
>>>>>>>>
>>>>>>>> Please review it.
>>>>>>>>
>>>>>>>
>>>>>>> Just one issue I found; if I open the tool via "View first 100 rows"
>>>>>>> or "View last 100 rows", and then click the Sort/Filter button and remove
>>>>>>> the sort field and hit OK, the button stays blue and if the dialogue is
>>>>>>> re-opened, the sort field is still present.
>>>>>>>
>>>>>>
>>>>>> I think that should be the expected behaviour, because you have
>>>>>> run the query for first/last 100 rows so it will sort the data based on
>>>>>> primary key and limit is also set to 100.
>>>>>>
>>>>>
>>>>> I disagree - I think that should be the *initial* state, but one that
>>>>> you can adjust. We can already do this with the number of rows - you can
>>>>> change that to 500 for example; it's just that it shows 100 initially.
>>>>>
>>>>
>>>> If you remove the sorting then we will have to reset the limit as
>>>> well and it is as good as view all rows. Is that behaviour you want.
>>>>
>>>
>>> Why do we have to reset the limit? I don't see any need to change that
>>> if you remove the sorting.
>>>
>>
>> Limit works with order by ASC/ DESC. If we don't reset the limit how
>> would user knows records are first 100 or last 100. This is my
>> understanding it may be wrong. If user remove sorting then we will have to
>> execute query like "select * from <table> limit 100"
>>
>>
>> Sure, and that’s fine - the user may want a random-ish sample of the data.
>>
>
> Attached is the modified patch.
>
>>
>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2018-09-14 13:41:56 pgAdmin 4 commit: Add a startup logger to the runtime to aid with debug
Previous Message Dave Page 2018-09-14 13:18:47 pgAdmin 4 commit: Fix logic around validation and highlighting of Sort/