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

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
>