Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bWjHt-00088k-VH for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 Aug 2016 12:04:10 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bWjHt-0000I8-EA for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 Aug 2016 12:04:09 +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 1bWjHg-0008V4-8a for pgadmin-hackers@postgresql.org; Mon, 08 Aug 2016 12:03:56 +0000 Received: from mail-it0-x22a.google.com ([2607:f8b0:4001:c0b::22a]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bWjHc-0000IO-RY for pgadmin-hackers@postgresql.org; Mon, 08 Aug 2016 12:03:55 +0000 Received: by mail-it0-x22a.google.com with SMTP id u186so71696618ita.0 for ; Mon, 08 Aug 2016 05:03:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Bgn8F3nmDqhBSYaC10nD1fqUY/XUvTjndo0zhY/qacA=; b=HMNnqJ9w8Kgk0YvRZj6sgQZmx7qd1dHNDi/sj1x20AkytrxhQDYadZjzF1iSklfSlz k/AdNthR8yOta1hEC3m5VwTtvtF5bSlY+P+nq/8Sl4HV3x6w1PSv95f5Sfqodw/UwGtC In1FdKTtw9M+VJGOg8tx2PtblH5ubAL1rkEwg98j7aSJ91HdCQOCGq/Bvz/7c+jj0Luq BbR9lo3FC8nfbdbM84Zr1CBUtldKyyjedObkxQOZ4QDYnfn7Cil5HSWpsjHWTh7GDQ/T R4b/GJHkbgAsoKH0Sb9x9NcVQxK3NglAkrDWDF3QdGV+6EX8QWjVfYuNqqIwSBxi0ypZ UApQ== 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=Bgn8F3nmDqhBSYaC10nD1fqUY/XUvTjndo0zhY/qacA=; b=DEkbuB+Jha1feiCDsyJxe5qE/xvVwiHpBwIFp5luAnbnFx5cQpRX5gh78GAu62C1l8 ukbkZcLd7x7tNby/j0WP4pcVYlL9m3yYcAffa3LQUjwqigTJD7MgkH2dd87HkYtenRDb Zip2XfaH7i4W9yJzJZ0YDDHSnLiqmJHBwuFtT9e6fe0hYdyP6cadQbDrle07yVSlJhqq h1mPE91Ja+ntO6k8tmzl1ZmhHOFyYj0YMwUTASS7wUwx3sJezLHKMzec+TBCRhxjrL6V f8yvuSJhn1rzpNK5y/JFHC4L0d/NlAcrK8BL1tr9VIBXU+EYHQZYUlBOTYtAN7HAoDH+ hcoQ== X-Gm-Message-State: AEkoouu8E6MIa2jJcrSy23V1/Ee6CCpjyZKOReeCv/c1yXelS6mOJr/7vAc/oo20axBUN0a08mVpOkbqOUDRkw== X-Received: by 10.36.117.79 with SMTP id y76mr16778237itc.35.1470657831092; Mon, 08 Aug 2016 05:03:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.208.97 with HTTP; Mon, 8 Aug 2016 05:03:50 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Mon, 8 Aug 2016 13:03:50 +0100 Message-ID: Subject: Re: RM #1250 Collection node counts To: Akshay Joshi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=001a114a90ce9bfe9405398e3568 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 --001a114a90ce9bfe9405398e3568 Content-Type: text/plain; charset=UTF-8 Hi On Mon, Aug 8, 2016 at 11:39 AM, Akshay Joshi 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? > > 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 --001a114a90ce9bfe9405398e3568 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Aug 8, 2016 at 11:39 AM, Akshay Joshi <<= a href=3D"mailto:akshay.joshi@enterprisedb.com" target=3D"_blank">akshay.jo= shi@enterprisedb.com> wrote:
Hi All

I have fixed the RM#1250 "= ;Collection node counts". To fix this RM I need to do following change= s
  • Move "check_precondition" function from modul= e's view class to global level within that python file itself, so that = module class will use it.=C2=A0
  • Modified "get_nodes" func= tion of each module's class, run the sql query to count th= e 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 inn= er query like "SELECT count(*) FROM( <node's> query =C2=A0) = AS collection_count". For that I'll have to remove semicolon's= from some of the SQL queries.
One case is not handle= d with this patch and that is on "Refresh" of collection node, co= unt is not updated. If user refresh the parent node then it will be updated= . I'll create a separate RM for that.
=C2= =A0
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 readi= ng the patch correctly, it doubles the amount of SQL queries run against th= e database when navigating the tree, and introduces race conditions where t= he count displayed could be different from the actual number of nodes.

I was expecting to see this implemented by watching fo= r 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= ?
=C2=A0

Attached is the patch file. Please review it and let me kn= ow the review comments. =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0

--
Akshay Joshi
P= rincipal Software Engineer=C2=A0


Phone: +91 20-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-ha= ckers




--
--001a114a90ce9bfe9405398e3568--