Re: RM #1250 Collection node counts

Awesome - just what I was expecting, and it works nicely :-).

Thanks, committed.

On Tue, Aug 9, 2016 at 12:00 PM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

> Hi Dave
>
> I have implemented the logic as per your suggestion. When user will expand
> the collection node label will get updated with collection count. Attached
> is the new patch file, please review it and let me know the review
> comments.
>
> On Mon, Aug 8, 2016 at 5:50 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Mon, Aug 8, 2016 at 1:18 PM, Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> 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)".
>>>
>>
>> Right, but you also need to allow for removal and addition of children
>> when the node is already expanded, and refreshes. Plus my other comments
>> are still valid I believe - race condition, double the SQL and a very large
>> change late in the beta cycle which isn't ideal.
>>
>>
>>>
>>>>
>>>>>
>>>>> 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-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>>
>>
>>
>>
>> --
>> 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-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>

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

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