Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d320y-0000D9-MS for pgadmin-hackers@arkaria.postgresql.org; Tue, 25 Apr 2017 15:04:28 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d320y-0002tA-8z for pgadmin-hackers@arkaria.postgresql.org; Tue, 25 Apr 2017 15:04:28 +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 1d320x-0002qb-Ay for pgadmin-hackers@postgresql.org; Tue, 25 Apr 2017 15:04:27 +0000 Received: from mail-vk0-x22d.google.com ([2607:f8b0:400c:c05::22d]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d320s-0007e7-EL for pgadmin-hackers@postgresql.org; Tue, 25 Apr 2017 15:04:26 +0000 Received: by mail-vk0-x22d.google.com with SMTP id k4so54008555vki.1 for ; Tue, 25 Apr 2017 08:04:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pivotal-io.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=uPC+v74bj5Ah8G8GuasyKJYua/gi4ZV7d4+3eq+FNEU=; b=mumlLnMGUJZ+gWbLEYGnMyXjnAkrAic89JGAvnDhK5xAds5Z4XyRJE+exx4KrL2QKQ 05ZzY/h1NmUAv8rbWv03fZIXuB+ObJfnl0BjW2z54UEdtmvNbQIs0Nw63PYz9+dVvxc1 tmLNIVgWDYnYPPawaWGL9RabgqUPxq3/wR/C4tV7k28wBc77pMYlZOeycT+Fj3VruhCz sVPGjus3pkTWTvEe/83Mg/yiZ8njgUolPb1kAuocP/eSPPKaWsHBO+DnFl8iwCKH9FBH gRL+Qym5sF1Q/XxHs/tiXIfaSFsJhfWERI0lvEdCz7y7Wmffaa2uIoKDBMRUPh9dRxBc /VIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=uPC+v74bj5Ah8G8GuasyKJYua/gi4ZV7d4+3eq+FNEU=; b=QuDjJxStks+QQn4mv0EuR1hMq3kxFwuDylnxD5W3oUQPQDH9ZYrsUD3/NABSGrAHZV EA/xDW93pnaawazZtrbkG37xS3+gOFVcVDLCwXb2iys4I7QJ2Mhw9awMVXtKE/pjWEJY /4lGpr7kFL9K10tHuEFryqc5+Vo/2n/Rm+iQYBmr4DgDSnowMU+DGWGBJFCO3Q6vh3jn dYsGBopXhBq+T2z3akZYergYbIwZFY3Y8J9YhqPhjH7cl1RluzzzhnShZ08J3QZWvsMw +TVaDhbTUbtCon9WCTdVKRpEXfd07D1AuWEOsx/9DqB3ycoCwjd/suTnf7XyuPIoqHI+ Az/Q== X-Gm-Message-State: AN3rC/6Cd+Cdu/q2XQempZXwEL3Rv85vi0zG+gRC0yz5pUI5dyA0+0aM sZ5NyiU3sFQyIaRbed1lfBWmhQRfNx7XdE8= X-Received: by 10.31.50.149 with SMTP id y143mr379203vky.63.1493132660098; Tue, 25 Apr 2017 08:04:20 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.10.199 with HTTP; Tue, 25 Apr 2017 08:04:19 -0700 (PDT) In-Reply-To: References: From: Joao Pedro De Almeida Pereira Date: Tue, 25 Apr 2017 11:04:19 -0400 Message-ID: Subject: Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken To: Dave Page Cc: Khushboo Vashi , pgadmin-hackers , Ashesh Vashi Content-Type: multipart/alternative; boundary=001a1143fc1ccee233054dff09ec 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 --001a1143fc1ccee233054dff09ec Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hello Khushboo, We reviewed the this patch and have some suggestions: *Python:* =E2=80=8B The functionality for adding the "can_prettify" is repeated in multiple places. Maybe this could be extracted into a function. *Javascript:* =E2=80=8B - The class Backgrid.SizeFormatter doesn't seem to have any tests. - The function pg_size_pretty displays bytes and Kilobytes differently. - Is it possible to add PB as well? - The function is a little bit hard to read, is it possible to refactor using private functions like: fromRaw: function (rawData, model) { var unitIdx =3D findDataUnitIndex(rawData); if (unitIdx =3D=3D 0) { return rawData + ' ' + this.dataUnits[i]; } return formatOutput(rawData, unitIdx); }, =E2=80=8B - In statistics.js:326 we believe it would make the code more readable if we change the variable "c" to "rawColumn" and "col" to "column". *SQL Files:* =E2=80=8B - Is there a way to avoid conditionals here? - Maybe we can use the same javascript function to prettify all the size= s Visually we saw a difference between "Databases" statistics and a specific database statistics. In "Databases" statistics the "Size" is "7.4 MB" but when you are in the specific database the "Size" is "7420 kB". Is this the intended behavior? Thanks Joao & Sarah On Tue, Apr 25, 2017 at 7:58 AM, Dave Page wrote: > Ashesh, can you review/commit this please? > > Thanks. > > On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi < > khushboo.vashi@enterprisedb.com> wrote: > >> Hi, >> >> Fixed RM #2315 : Sorting by size is broken. >> >> Removed the pg_size_pretty function from query for the collection and >> introduced the client side function to convert size into human readable >> format. So, the sorting issue is fixed as the algorithm will get the act= ual >> value of size instead of formatted value. >> >> >> Thanks, >> Khushboo >> >> >> >> >> -- >> 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 > --001a1143fc1ccee233054dff09ec Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hello Khushboo,

We reviewed the this pa= tch and have some suggestions:

Python:

=E2=80=8B
The functionality for adding the "can_prettify&qu= ot; is repeated in multiple places. Maybe this could be extracted into a fu= nction.=C2=A0

Javascript:=

=E2=80=8B=
  • The class=C2=A0Backgrid.SizeFormatter doesn&= #39;t seem to have any tests.=C2=A0

  • The function pg_size_pretty displays bytes and Kilobyt= es differently.=C2=A0
  • Is it possible to add PB as well?
  • The function is a little bit hard to read, is it possible to refactor = using private functions like:
f=
romRaw: function (rawData, model) {
   var unitIdx =3D findDataUnitIndex(rawData);
   if (unitIdx =3D=3D 0) {
      return rawData + ' ' + this.dataUnits[i];
   }
   return formatOutput(rawData, unitIdx);
},
=E2=80=8B=

  • In statistics.js:326 we believe it would= make the code more readable if we change the variable "c" to &qu= ot;rawColumn" and "col" to "column".
=


SQL Files= :

=E2=80= =8B
  • Is there a way to avoid conditionals here= ?=C2=A0
  • Maybe we can use the same javascript function to prettify a= ll the sizes


Visuall= y we saw a difference between "Databases" statistics and a specif= ic database statistics. In "Databases" statistics the "Size&= quot; is "7.4 MB" but when you are in the specific database the &= quot;Size" is "7420 kB".
Is this the intended beha= vior?



Thanks
Joao & Sarah

On Tue, Apr 25, 2017 at 7:58 AM, Dave Page <dpage@pgad= min.org> wrote:
Ashesh, can you review/commit this please?

Thanks.=

=
On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,

Fixed RM #2315 : Sorting by size is broken.
=

Removed the pg_size_pretty function from query for the = collection and introduced the client side function to convert size into hum= an readable format. So, the sorting issue is fixed as the algorithm will ge= t the actual value of size instead of formatted value.=C2=A0
=C2= =A0

Thanks,
Khushboo

<= /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<= br>Blog: http://p= gsnake.blogspot.com
Twitter: @pgsnake

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

--001a1143fc1ccee233054dff09ec--