Re: RM #1250 Collection node counts

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: RM #1250 Collection node counts
Date: 2016-08-08 12:18:28
Message-ID: CANxoLDd=7_1hv6kujs6AwiL3fO_DRXydWZeNxTQR6cKxn5kDbA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Mon, Aug 8, 2016 at 5:33 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Mon, Aug 8, 2016 at 11:39 AM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi All
>>
>> I have fixed the RM#1250 "Collection node counts". To fix this RM I need
>> to do following changes
>>
>> - Move "check_precondition" function from module's view class to
>> global level within that python file itself, so that module class will use
>> it.
>> - Modified "get_nodes" function of each module's class, run the sql
>> query to count the number of objects and pass the count to
>> "generate_browser_collection_node" function to display the collection
>> count.
>> - Reuse SQL queries which is used to fetch nodes. Make that query as
>> inner query like "SELECT count(*) FROM( <node's> query ) AS
>> collection_count". For that I'll have to remove semicolon's from some of
>> the SQL queries.
>>
>> One case is not handled with this patch and that is on "Refresh" of
>> collection node, count is not updated. If user refresh the parent node then
>> it will be updated. I'll create a separate RM for that.
>>
>
> Sorry Akshay, but I really don't like the way you've done this. It seems
> like an unnecessarily large patch, and if I'm reading the patch correctly,
> it doubles the amount of SQL queries run against the database when
> navigating the tree, and introduces race conditions where the count
> displayed could be different from the actual number of nodes.
>
> I was expecting to see this implemented by watching for tree events (e.g.
> 'added' and 'removed') and using those events to update the label on the
> parent node, if that node is a collection. That should just be a few lines,
> and should be correct at all times right?
>

With current implementation children's of any collection node will be
fetched/added when user will expand that collection node, in that case we
will update the label once the node gets expanded. For example initially we
will show "Databases" and when it gets expanded then we will update it to
"Databases (5)".

>
>
>>
>> Attached is the patch file. Please review it and let me know the review
>> comments.
>>
>> --
>> *Akshay Joshi*
>> *Principal Software Engineer *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-08-08 12:20:56 Re: RM #1250 Collection node counts
Previous Message Surinder Kumar 2016-08-08 12:15:22 Re: [pgAdmin4][Patch]: RM#1478 - Make code mirror text editor keyboard shortcuts consistent irrespective of platform type