Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d32n1-0005XT-ED for pgadmin-hackers@arkaria.postgresql.org; Tue, 25 Apr 2017 15:54:07 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d32n0-0003uV-Td for pgadmin-hackers@arkaria.postgresql.org; Tue, 25 Apr 2017 15:54:06 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1d32ml-0003VF-KG for pgadmin-hackers@postgresql.org; Tue, 25 Apr 2017 15:53:51 +0000 Received: from mail-io0-x22c.google.com ([2607:f8b0:4001:c06::22c]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d32mh-0003HZ-Jl for pgadmin-hackers@postgresql.org; Tue, 25 Apr 2017 15:53:50 +0000 Received: by mail-io0-x22c.google.com with SMTP id a103so217296134ioj.1 for ; Tue, 25 Apr 2017 08:53:46 -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=elxQy/FG45AkOlP4wbj54HLmX0QS0cwU7R1oCk8IWPA=; b=j/9YWKHu5UmGn+Lfxli5UdCIVFGWElBtWt9C8T9OdC0DUeaYoASK8tHUytBPA/o4Wn vp9Bs+mj84+U+yRulTeUCTFHVZ/7Sba54YD014C/5Kx5I/NpIhj/NW9GGtzM2Mn34MKT aei/yY/ll4h1vnGTkUI/V19dne6cpnu19kt9qoenYBPEFk5GWfQsHVAbLxNu2d7xKhbe NBRHS/EKEtavoLer2hJL1/SA9lY90xrrG3rKLSbOqySfOEcMi6SwTfbY1CXOuyB7shwn vha4jFonIWNEG9+yKEKwnIoSDXrVZbPfCDzLLWJAb5Ec3OFjxnpeDv0W9ue3zxdqW9Sh dfeg== 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=elxQy/FG45AkOlP4wbj54HLmX0QS0cwU7R1oCk8IWPA=; b=mHCWRpWUq8v/pMOi9yVTPT6g9ApSNT3nrHNZuh3eXPIUWNpjS3PK3gCspRT9a3m30P 0w3O9MYeZIQDP4pPDQqhyenSlxUR15s+d13na1/BB1doTXb9Ik4DEo7PBY1dR4giQZ6c 3j3tlCZKTxskm/eD/T4s0uGqJhtahQAZy2nKmxku2EO8DSSi4fIcDz2wJSeNbT8YDVL0 fjrLpNdlHb8jRHpwOuNxzHlS6VLmN3P0VJV3IlD7D2t3VI+wrtL5CsCPFf4x++EqWNG3 DE+wcbt8GPrjVarPKvnVcLqLnU1HEfvVd/GZCxsPBU5sOmJqF/4wCmsr59eaDwB5A6y+ 1rjQ== X-Gm-Message-State: AN3rC/7vAfjW7xppmdh/TQDLebrXJSZrCC58J0raHHq+WHrIRdnsZX/F gBNqZ+nEuGT+zFrT41yBKWzqvnWycA== X-Received: by 10.107.13.16 with SMTP id 16mr16634624ion.144.1493135625720; Tue, 25 Apr 2017 08:53:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.174.167 with HTTP; Tue, 25 Apr 2017 08:53:44 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Tue, 25 Apr 2017 16:53:44 +0100 Message-ID: Subject: Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken To: Joao Pedro De Almeida Pereira Cc: Khushboo Vashi , pgadmin-hackers , Ashesh Vashi Content-Type: multipart/alternative; boundary=001a1140a35892c107054dffbaa9 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 --001a1140a35892c107054dffbaa9 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Tue, Apr 25, 2017 at 4:04 PM, Joao Pedro De Almeida Pereira < jdealmeidapereira@pivotal.io> wrote: > 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? > > Good idea. I'd also like to see a unit test added please Khushboo. > > - > - 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 > sizes > > > > Visually we saw a difference between "Databases" statistics and a specifi= c > 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 ac= tual >>> 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 >> > > --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --001a1140a35892c107054dffbaa9 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Tue, Apr 25, 2017 at 4:04 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Khushboo,

We revie= wed the this patch 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?
Good idea. I'd also like to see a u= nit test added please Khushboo.=C2=A0
  • The function is a little bit hard= to read, is it possible to refactor using private functions like:
    • fromRaw: fun=
ction (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@pgadmin.org> wrote:
Ashesh, can you review/commit= this please?

Thanks.

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

Fixed RM #2315 : Sorting by size is brok= en.

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 algorith= m will get the actual value of size instead of formatted value.=C2=A0
=
=C2=A0

Thanks,
Khushboo




--
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
Twit= ter: @pgsnake

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




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

EnterpriseDB UK: http://www.enterprised= b.com
The Enterprise PostgreSQL Company
--001a1140a35892c107054dffbaa9--