Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bWjVs-0000DK-Ne for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 Aug 2016 12:18:36 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bWjVr-0001gr-Vj for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 Aug 2016 12:18:36 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bWjVr-0001gl-Kf for pgadmin-hackers@postgresql.org; Mon, 08 Aug 2016 12:18:35 +0000 Received: from mail-io0-x232.google.com ([2607:f8b0:4001:c06::232]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bWjVn-0000b4-A8 for pgadmin-hackers@postgresql.org; Mon, 08 Aug 2016 12:18:35 +0000 Received: by mail-io0-x232.google.com with SMTP id q83so356205982iod.1 for ; Mon, 08 Aug 2016 05:18:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=vcOmPoR7S+QAr6D/HfKTIf6qLnq9mi0mmOltmw9a53c=; b=iDxAUpru6RbPfuci90LyRKyCfNsCL77CIodmFrwKzD52H1655JQGJlqQal1+rmjXCQ WT5ewWJ56mxZLyaokzJrJZDsc35UoGSaFOM2xIlRvstDmow3Q/YYSvH6RvTypOG84n3I LYOxXkcvjtdezV744SiAyldfpKKBtGFksLHkMMESYQM8shAj/AjKvvRCGq3TAvGLMwAb KdrUgst8lGK9df8YMBRVLcEtY3sPtW+uuaJGhVbbTFLUiWouQuUuOpYXp2IptjOwMUCa n5wwCDyUfkePjgamw7Zw6XYkVwn/foWQftolzm7O/NA3pX88kOrR9XB8+WqUcSYAw60b Pjug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=vcOmPoR7S+QAr6D/HfKTIf6qLnq9mi0mmOltmw9a53c=; b=cCBXNNBXPQwpH6jnczoDe7nkWOJNluZj6H1Adh/OzlPG2Sx4MdX1ddoogq2ZUjaS6A pvGHyV3LyKs2ASCFbQzlHhDNqdgNO80DN1JLVjsInjESusROJYLPvKLUfgQ1HKEGwOpQ 0rkQFbZxkiRtJwBpiMPC8Z7f1Ot7xwUyw/m83lok49iJxw/OluDWKBNZS9Z7RwESXRZi phrTze+oZvDAj6AMLVmufUJ2RwKL1p/pXvMvzboxfguW+c6dXTwkSVhbWv9Z6bW5CHOS CW2Jjo07NqpxawMgZRWCtKgyOz+aDsajLNjZVW3ci4BQ0eNohOL7tgReIK6DCrQlPzj7 AEjg== X-Gm-Message-State: AEkooutTTmPh94JIJuCBTKuCIb3vwtu9r3HZu0Rj9mKOIg7cjTaw4nh7NZCn0BtvXj4FxiubPJ2ZVMP82JIdQEp6 X-Received: by 10.107.55.70 with SMTP id e67mr94817870ioa.51.1470658709269; Mon, 08 Aug 2016 05:18:29 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.17.70 with HTTP; Mon, 8 Aug 2016 05:18:28 -0700 (PDT) In-Reply-To: References: From: Akshay Joshi Date: Mon, 8 Aug 2016 17:48:28 +0530 Message-ID: Subject: Re: RM #1250 Collection node counts To: Dave Page Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=001a114a6d76f3c21505398e6939 X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a114a6d76f3c21505398e6939 Content-Type: text/plain; charset=UTF-8 On Mon, Aug 8, 2016 at 5:33 PM, Dave Page wrote: > Hi > > On Mon, Aug 8, 2016 at 11:39 AM, Akshay Joshi < > akshay.joshi@enterprisedb.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( 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@postgresql.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* --001a114a6d76f3c21505398e6939 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Mon, Aug 8, 2016 at 5:33 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, Aug 8, 2016 at 11:39 AM, Akshay Joshi <= span dir=3D"ltr"><akshay.joshi@enterprisedb.com> wrote:
Hi All

I h= ave fixed the RM#1250 "Collection node counts". To fix this RM I = need to do following changes
  • Move "check_preconditio= n" function from module's view class to global level within that p= ython file itself, so that module class will use it.=C2=A0
  • Modified= "get_nodes" function of each module's class, run the s= ql query to count the number of objects and pass the count to "= generate_browser_collection_node" function to display the collect= ion count.
  • Reuse SQL queries which is used to fetch no= des. Make that query as inner query like "SELECT count(*) FROM( <no= de's> query =C2=A0) AS collection_count". For that I'll hav= e to remove semicolon's from some of the SQL queries.
    • =
One case is not handled with this patch and that is on "Refresh&q= uot; of collection node, count is not updated. If user refresh the parent n= ode then it will be updated. I'll create a separate RM for that.
<= /div>
=C2=A0
Sorry Akshay, but I really d= on'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 amo= unt of SQL queries run against the database when navigating the tree, and i= ntroduces race conditions where the count displayed could be different from= the actual number of nodes.

I was expecting to se= e this implemented by watching for tree events (e.g. 'added' and &#= 39;removed') and using those events to update the label on the parent n= ode, if that node is a collection. That should just be a few lines, and sho= uld be correct at all times right?

=C2=A0 =C2=A0With current implementation children's of = any collection node will be fetched/added when user will expand that collec= tion node, in that case we will update the label once the node gets expande= d. For example initially we will show "Databases" =C2=A0and when = it gets expanded then we will update it to "Databases (5)". =C2= =A0 =C2=A0 =C2=A0
=C2=A0

Attached is the patch file. Please review it and let me know the review c= omments. =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0

--
Akshay Joshi
Principal Software Engineer=C2= =A0


=
Pho= ne: +91 20-3058-9517
Mobile: +91 976-788-8246<= /div>


 --
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-ha= ckers




--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


--
Akshay Joshi
Principal Software Engineer=C2=A0=


Phone: +91 20= -3058-9517
Mobile: +91 976-788-8246
--001a114a6d76f3c21505398e6939--