Re: [pgAdmin][RM5157] Default sort order at start in view table data by primary key by default

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: navnath gadakh <navnath(dot)gadakh(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM5157] Default sort order at start in view table data by primary key by default
Date: 2020-04-22 08:56:42
Message-ID: CANxoLDdSBQzHJWPfLY5iWH4n=Y24x3Ph9n3z3KizONv=cNo7pw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Navnath

Please update the screenshot and documentation for the new parameter you
have added in Preferences.

On Tue, Apr 21, 2020 at 6:32 PM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> The patch looks good to me.
>
> On Tue, Apr 21, 2020 at 6:18 PM navnath gadakh <
> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>
>> Hello Khushboo,
>>
>> Please find an updated patch with the following changes:
>>
>> 1. View table data sorting(by Primary Key columns) is configurable now
>> (Preferences > Query Tools > Options > Sort View Data results by primary
>> key columns? ).
>> 2. This feature will work with only View/Edit Data - All Rows option of
>> the table's context menu as for other options data is always sorted.
>> 3. Test cases added.
>>
>> Thanks!
>>
>>
>> On Tue, Apr 21, 2020 at 5:31 PM navnath gadakh <
>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>
>>> Thanks, Dave for all clarifications. Will send updated patch.
>>>
>>> On Tue, Apr 21, 2020 at 5:04 PM Dave Page <dave(dot)page(at)enterprisedb(dot)com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Apr 21, 2020 at 12:21 PM Khushboo Vashi <
>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Apr 21, 2020 at 4:50 PM Dave Page <dave(dot)page(at)enterprisedb(dot)com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Apr 21, 2020 at 12:05 PM navnath gadakh <
>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Apr 21, 2020 at 4:17 PM Dave Page <
>>>>>>> dave(dot)page(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Apr 21, 2020 at 11:16 AM Ashesh Vashi <
>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Apr 21, 2020 at 3:38 PM navnath gadakh <
>>>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Dave/Team,
>>>>>>>>>>
>>>>>>>>>> I have added an option under preferences menu to table data
>>>>>>>>>> sorting by primary key.
>>>>>>>>>>
>>>>>>>>>> [image: image.png]
>>>>>>>>>>
>>>>>>>>>> Are you okay with text/labels?
>>>>>>>>>>
>>>>>>>>> "Sort table/view data by primary key(s)?"
>>>>>>>>> Question mark (?) is missing your statement, which is must for a
>>>>>>>>> boolean flag.
>>>>>>>>>
>>>>>>>>
>>>>>>>> "Sort View Data results by primary key columns?"
>>>>>>>>
>>>>>>>> "If set to True, data returned when using the View Data option will
>>>>>>>> be sorted by the Primary Key columns by default."
>>>>>>>>
>>>>>>> Ok. Will add this label and description.
>>>>>>>
>>>>>>>>
>>>>>>>> Why does it only apply if "All Rows" is used? I don't see any
>>>>>>>> reason not to do it at all times.
>>>>>>>>
>>>>>>> As per discussion with Khushboo and Akshay will implement for "All
>>>>>>> Rows" as primary key columns ordering is already present for other
>>>>>>> options.
>>>>>>>
>>>>>>
>>>>>> OK. Let's make it consistent then, and have the preference affect all
>>>>>> options.
>>>>>>
>>>>>>
>>>>> In case of the Last 100 rows, how will we get the result without
>>>>> sorting?
>>>>>
>>>>
>>>> SELECT * FROM table ORDER BY xmin DESC limit 100 :-)
>>>>
>>>> That's a good point though. Let's just modify the description:
>>>>
>>>> "If set to True, data returned when using the View/Edit Data - All Rows
>>>> option will be sorted by the Primary Key columns by default. When using the
>>>> First/Last 100 Rows options, data is always sorted."
>>>>
>>>>
>>>>
>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> -- Ashesh
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>>
>>>>>>>>>> On Tue, Apr 21, 2020 at 12:35 PM Akshay Joshi <
>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Navnath
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Apr 21, 2020 at 12:21 PM navnath gadakh <
>>>>>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hello Hackers,
>>>>>>>>>>>> It's related to applying data sorting on table data by
>>>>>>>>>>>> primary key.
>>>>>>>>>>>> With the existing implementation, we can view the table's data
>>>>>>>>>>>> using 4 options with the different orders by default
>>>>>>>>>>>> 1 - All Rows (No order)
>>>>>>>>>>>> 2 - First 100 rows (ASC order)
>>>>>>>>>>>> 3 - Last 100 rows (DESC order)
>>>>>>>>>>>> 4 - Filtered rows (No order)
>>>>>>>>>>>>
>>>>>>>>>>>> In the https://redmine.postgresql.org/issues/5157 it's not
>>>>>>>>>>>> clearly mentioned on which option to apply sorting by PK? I'm
>>>>>>>>>>>> assuming that should be on ALL Rows option.
>>>>>>>>>>>>
>>>>>>>>>>>> Please suggest.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Yes, for all those options. Sorting by Primary Key is all
>>>>>>>>>>> depends on the value set by the user in the Preferences dialog.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Apr 21, 2020 at 10:12 AM navnath gadakh <
>>>>>>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Khushboo,
>>>>>>>>>>>>> Please hold this patch for review I'm still optimizing the
>>>>>>>>>>>>> code in the patch.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Mon, Apr 20, 2020 at 9:16 PM navnath gadakh <
>>>>>>>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Khushboo,
>>>>>>>>>>>>>> I have modified the code as per review comments. Please
>>>>>>>>>>>>>> review the attached patch file.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Mon, Apr 20, 2020 at 10:56 AM Khushboo Vashi <
>>>>>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Navnath,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Review comments:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 1. If we have multiple Primary keys, then we should include
>>>>>>>>>>>>>>> all the keys into the Order by clause.
>>>>>>>>>>>>>>> 2. In the Preferences dialog, please put this option in the
>>>>>>>>>>>>>>> Query Tool > Options instead of Result Grid and also change the Label.
>>>>>>>>>>>>>>> 3. Please optimize the code, as I can see objectname.sql
>>>>>>>>>>>>>>> file is being used in else condition also, which is not required. Based on
>>>>>>>>>>>>>>> the parameter setting, Just one call of that sql is enough.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Khushboo
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Fri, Apr 17, 2020 at 6:43 PM navnath gadakh <
>>>>>>>>>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hello Hackers,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Please find the modified patch with an option in
>>>>>>>>>>>>>>>> Preferences for data sorting by the primary key. Also, the previous patch
>>>>>>>>>>>>>>>> was not working with table has no primary key.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Thu, Apr 16, 2020 at 5:01 PM Dave Page <
>>>>>>>>>>>>>>>> dave(dot)page(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Thu, Apr 16, 2020 at 12:08 PM navnath gadakh <
>>>>>>>>>>>>>>>>> navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi Dave/Team,
>>>>>>>>>>>>>>>>>> This patch is related to the default sort order
>>>>>>>>>>>>>>>>>> for the view table data. In pgAdminIII default ordering is by primary key
>>>>>>>>>>>>>>>>>> and this is not working in pgAdminIV.
>>>>>>>>>>>>>>>>>> I have attached the patch with the back end code.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Please review it.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> *Question*: There is one suggestion on
>>>>>>>>>>>>>>>>>> https://redmine.postgresql.org/issues/5157 about to put
>>>>>>>>>>>>>>>>>> a checkbox in the configuration for this behavior.
>>>>>>>>>>>>>>>>>> Do I need to implement that really? I
>>>>>>>>>>>>>>>>>> yes, Is preferences a good place for that? / Suggestions?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I think we should make this optional, and yes, Preferences
>>>>>>>>>>>>>>>>> is a good place. The reason is that sorting data is not without cost - at
>>>>>>>>>>>>>>>>> the very least it will require use of an index to access what may be the
>>>>>>>>>>>>>>>>> whole table.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>>>>>> VP & Chief Architect, Database Infrastructure
>>>>>>>>>>>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>> Navnath Gadakh
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>> Navnath Gadakh
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>> Navnath Gadakh
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Navnath Gadakh
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> *Thanks & Regards*
>>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>>>
>>>>>>>>>>> *Sr. Software Architect*
>>>>>>>>>>> *EnterpriseDB Software India Private Limited*
>>>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Regards,
>>>>>>>>>> Navnath Gadakh
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Dave Page
>>>>>>>> VP & Chief Architect, Database Infrastructure
>>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>
>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>> Twitter: @pgsnake
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Regards,
>>>>>>> Navnath Gadakh
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> VP & Chief Architect, Database Infrastructure
>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> VP & Chief Architect, Database Infrastructure
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>
>>>
>>> --
>>> Regards,
>>> Navnath Gadakh
>>>
>>
>>
>> --
>> Regards,
>> Navnath Gadakh
>>
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message navnath gadakh 2020-04-22 09:58:03 Re: [pgAdmin4][Patch] - RM 3900 - Delete/Drop option is disabled for the constraints
Previous Message Akshay Joshi 2020-04-22 08:29:19 Re: Exclude RESQL test cases