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

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*