public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: RM #1250 Collection node counts
Date: Mon, 8 Aug 2016 13:20:56 +0100
Message-ID: <CA+OCxowLXixHXAfJx_2MOLDDUjvZTV7h8DZOQuTXbDcKHUdxQw@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDd=7_1hv6kujs6AwiL3fO_DRXydWZeNxTQR6cKxn5kDbA@mail.gmail.com>
References: <CANxoLDd4-bmH9jtT1Ki4MD3kBmt15MF2yOWsa7LyGWcgo-kGzQ@mail.gmail.com>
<CA+OCxowgJNaBc2bAGamw6zB=v1sbpzU7cqF8RP_YbWzfA6PmHA@mail.gmail.com>
<CANxoLDd=7_1hv6kujs6AwiL3fO_DRXydWZeNxTQR6cKxn5kDbA@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
On Mon, Aug 8, 2016 at 1:18 PM, Akshay Joshi <[email protected]>
wrote:
>
>
> On Mon, Aug 8, 2016 at 5:33 PM, Dave Page <[email protected]> wrote:
>
>> Hi
>>
>> On Mon, Aug 8, 2016 at 11:39 AM, Akshay Joshi <
>> [email protected]> 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 ([email protected])
>>> 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
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: RM #1250 Collection node counts
In-Reply-To: <CA+OCxowLXixHXAfJx_2MOLDDUjvZTV7h8DZOQuTXbDcKHUdxQw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox