Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d7fvI-0001QR-Jg for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 May 2017 10:29:48 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d7fvI-0002ho-68 for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 May 2017 10:29:48 +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 1d7fv2-0001xS-8T for pgadmin-hackers@postgresql.org; Mon, 08 May 2017 10:29:32 +0000 Received: from mail-it0-x233.google.com ([2607:f8b0:4001:c0b::233]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d7fux-0000LT-20 for pgadmin-hackers@postgresql.org; Mon, 08 May 2017 10:29:31 +0000 Received: by mail-it0-x233.google.com with SMTP id e65so36264913ita.1 for ; Mon, 08 May 2017 03:29:26 -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=QcYmXpzbe8d2qiT3sL9JfzD/lOEgKB3tD9cYIikADbQ=; b=lP475KgV3WGV9j2zksfQj/s+ok9BtLY4TFxXlZ6/CJcunPS+87oeJWNZP2Lt6ihWVu 3gi0QXpxdFYovOmZZbEbYxZSE8718GS1Iz9ohAbP7aIevSXMifTTmiQbAPIeCWdir7st xjB5Fg2YhpsMQn1ecuxb1/F0ocbxJSaqn9UYw2DHn3HQFYXZIqnzJGIhkdO+9qDKtJ5f ur5miiB+BJOfKuTiSK9fHiG/zCZadluc05g46kQSk/56Kg/Ly5FjIYZtYxg98pEG60k+ uUqr6gD+Btws0LYsQiTsh7/F75GDOnmFUv2ng77lnabL3chUQpPRtrmR2c6/rhso67cF PfSg== 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=QcYmXpzbe8d2qiT3sL9JfzD/lOEgKB3tD9cYIikADbQ=; b=hvqdXvX86acCp1rOup51L2dKdPxtN2yba1STwYLN3ZroEhX73D/elJlxlXCyaZCu10 gv5gOzcoa5fXggWgtgBG/uhQM+TDXym2EYqFwBK3YmsQaKTfqFyxuJMo5uuvZ4hxnfd/ 1UgNh3K86/W4pUuaF27r1LxdBa7XWFfJwAt7snCYO7LH2K7DHtfvFsTqo9Do+0dH1IpZ O6xUdSLwrM0Io0m1O23b7O+zikUVH5cQm2Tph9sVLUbGI1VNiaiNucrBX/Pr+CmfllI7 Ft95y9yBmnc6qUG0F+V+AmaAqKW2bnvT9WzjLJGdNSzl0oMhqB/yKntlXYYDnxmn3WCq LQrA== X-Gm-Message-State: AN3rC/7dOioIcxJ2800NzBPDTuI4yEC3tFh5jYjslGbOzMN/DMVGur2M 2GsY28LzZ8JOFeznVpnUvIBJPqLUJg== X-Received: by 10.36.17.197 with SMTP id 188mr20572617itf.28.1494239365258; Mon, 08 May 2017 03:29:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.174.167 with HTTP; Mon, 8 May 2017 03:29:24 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Mon, 8 May 2017 11:29:24 +0100 Message-ID: Subject: Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken To: Khushboo Vashi Cc: Joao Pedro De Almeida Pereira , pgadmin-hackers , Ashesh Vashi , George Gelashvili Content-Type: multipart/alternative; boundary=001a11437efe936d50054f00b698 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 --001a11437efe936d50054f00b698 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Thanks - patch applied. On Mon, May 8, 2017 at 8:08 AM, Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > Hi, > > Please find the attached updated patch. > > Thanks, > Khushboo > > On Fri, May 5, 2017 at 9:00 PM, Joao Pedro De Almeida Pereira < > jdealmeidapereira@pivotal.io> wrote: > >> Hi Khushboo, >> >> We looked at your updated patch: >> - the tests look good! >> - there's a small comment header change needed in size_prettify_spec >> > Fixed > >> - it looks like the previous and new functions have different behaviors >> (where the new behavior changes units on 10000 of the lower unit, a 9999= GB) >> - We should clarify that in size_prettify, we were mostly talking about >> name readability in your first patch, and that the original structure wa= s >> better (especially the sizes array) >> At first glance, the new sizePrettify appears to behave like a for loop, >> so that might be the simplest refactor. >> >> Added loop. > > >> Thanks, >> Joao and George >> >> >> >> On Thu, May 4, 2017 at 5:02 AM, Khushboo Vashi < >> khushboo.vashi@enterprisedb.com> wrote: >> >>> Hi, >>> >>> Please find the attached updated patch. >>> >>> Thanks, >>> Khushboo >>> >>> On Fri, Apr 28, 2017 at 7:58 PM, Matthew Kleiman >>> wrote: >>> >>>> Hi Khushboo, >>>> >>>> That sounds good. Sorry if we weren't clear at first. >>>> >>>> Have a good holiday weekend! >>>> >>>> Sarah & Matt >>>> >>>> >>>> On Fri, Apr 28, 2017 at 4:35 AM, Khushboo Vashi < >>>> khushboo.vashi@enterprisedb.com> wrote: >>>> >>>>> Hi Sarah, >>>>> >>>>> On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear >>>>> wrote: >>>>> >>>>>> Hi Kushboo! >>>>>> >>>>>> We understand your point, but we believe that relying on 2 >>>>>> independent functions to deliver the same formatting can become a pr= oblem >>>>>> if the PG function changes. Our suggestion is to use a single functi= on in >>>>>> our javascript code to do this formatting. >>>>>> >>>>>> It seems reasonable to me and I am going to use a single javascript >>>>> function which will support PB also (as per Dave we should add suppor= t till >>>>> PB) . >>>>> >>>>>> If the community believes we can live with this risk, let's move >>>>>> forward. >>>>>> >>>>>> Thanks! >>>>>> Sarah & Joao >>>>>> >>>>>> On Thu, Apr 27, 2017 at 1:50 AM, Khushboo Vashi < >>>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Joao & Sarah, >>>>>>> >>>>>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >>>>>>> jdealmeidapereira@pivotal.io> wrote: >>>>>>> >>>>>>>> Hi Khushboo! >>>>>>>> >>>>>>>> Thanks for your reply! >>>>>>>> >>>>>>>> >>>>>>>>> *SQL Files:* >>>>>>>>>> >>>>>>>>>> - Is there a way to avoid conditionals here? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> - Maybe we can use the same javascript function to prettify >>>>>>>>>> all the sizes >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> In case of collection node (ex: Databases), I have implemented >>>>>>>>>> this functionality via putting a formatter for a back-grid colum= n. So, it >>>>>>>>>> is applicable for the entire column not for the individual cell.= To apply >>>>>>>>>> the javascript function on a single cell for the column (string = column), >>>>>>>>>> first we need to identify that cell and then apply the function,= I think >>>>>>>>>> this is overhead. Just to avoid conditional sql template I would= not prefer >>>>>>>>>> this approach. >>>>>>>>> >>>>>>>>> >>>>>>>> We are not totally sure we understood what you meant on the >>>>>>>> previous statement. Are you saying that the conditionals in SQL ar= e used to >>>>>>>> ensure that we can apply a javascript function at column level ins= tead of >>>>>>>> cell level? >>>>>>>> >>>>>>>> Correct. >>>>>>> >>>>>>>> Our concern is that the templates are being made more complex and >>>>>>>> inconsistencies are introduced in the code and the UI. >>>>>>>> >>>>>>> >>>>>>> Inconsistencies in the UI can be avoided through making the >>>>>>> size_formatter same as pg_size_pretty function which I will impleme= nt. >>>>>>> I have checked the pg_size_pretty function code and it supports til= l >>>>>>> TB format, so I think we should keep till TB only. >>>>>>> >>>>>>> In this particular example, we are allowing the backend to respond >>>>>>>> sometimes with prettified data and sometimes without it, so at UI = level we >>>>>>>> will have inconsistencies between screens or more complex Javascri= pt code >>>>>>>> to support sometimes prettifying and sometimes not prettify the sa= me >>>>>>>> fields. >>>>>>>> >>>>>>>> We have separate logic for collection and single node in >>>>>>> statistics.js and we are using javascript code for prettifying only= for >>>>>>> collection node. >>>>>>> >>>>>>> >>>>>>>> What we were thinking was to maybe not specify on the SQL level an= d >>>>>>>> have the same format for "Size" everywhere in the UI. >>>>>>>> >>>>>>>> >>>>>>> Here our main concern is inconsistency in "Size" format in the UI >>>>>>> that can be avoided as I said earlier. >>>>>>> We are using pg_size_pretty function for different fields like Size= , >>>>>>> Index Size, Table space size, Tuple length, Size of Temporary files= in >>>>>>> different modules and some of them are cell level and we don't requ= ire to >>>>>>> put overhead on cell level fields as sorting is not required for in= dividual >>>>>>> node statistics. >>>>>>> >>>>>>> >>>>>>> >>>>>>>> Thanks >>>>>>>> Joao & Sarah >>>>>>>> >>>>>>>> On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi < >>>>>>>> khushboo.vashi@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> Hi Joao & Sarah, >>>>>>>>> >>>>>>>>> Thanks for reviewing the patch. >>>>>>>>> >>>>>>>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>>>>>>> >>>>>>>>>> When I have implemented this, my first thought is exactly same a= s >>>>>>>>> you suggested but while looking at the code I felt its not a goo= d idea to >>>>>>>>> have a function. In case of a function, we need to pass the whole >>>>>>>>> result-set as well as the list of fields which we need to be pret= tified. >>>>>>>>> So, only for 2 lines, I didn't find any reason to make a function= . >>>>>>>>> >>>>>>>>>> *Javascript:* >>>>>>>>>> =E2=80=8B >>>>>>>>>> >>>>>>>>>> - The class Backgrid.SizeFormatter doesn't seem to have any >>>>>>>>>> tests. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Sure, will do. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> - The function pg_size_pretty displays bytes and Kilobytes >>>>>>>>>> differently. >>>>>>>>>> - Is it possible to add PB as well? >>>>>>>>>> >>>>>>>>>> Will check and add PB. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> - >>>>>>>>>> - The function is a little bit hard to read, is it possible >>>>>>>>>> to refactor using private functions like: >>>>>>>>>> >>>>>>>>>> Will make it more readable. >>>>>>>>> >>>>>>>>>> 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 "co= l" to "column". >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> I will change the variable name from "c" to "rawColumn" but I >>>>>>>>> think "col" is appropriate as we already have columns variable an= d anyone >>>>>>>>> can confuse while reading the code (for columns and 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 >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> In case of collection node (ex: Databases), I have implemented >>>>>>>>> this functionality via putting a formatter for a back-grid column= . So, it >>>>>>>>> is applicable for the entire column not for the individual cell. = To apply >>>>>>>>> the javascript function on a single cell for the column (string c= olumn), >>>>>>>>> first we need to identify that cell and then apply the function, = I think >>>>>>>>> this is overhead. Just to avoid conditional sql template I would = not prefer >>>>>>>>> this approach. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Visually we saw a difference between "Databases" statistics and = a >>>>>>>>>> specific database statistics. In "Databases" statistics the "Siz= e" is "7.4 >>>>>>>>>> MB" but when you are in the specific database the "Size" is "742= 0 kB". >>>>>>>>>> Is this the intended behavior? >>>>>>>>>> >>>>>>>>>> Only for the Databases (collection node), the client side >>>>>>>>> functionality is implemented not for individual node , so this be= haviour is >>>>>>>>> different. For the individual node still, we are using pg_size_pr= etty >>>>>>>>> function >>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>>> 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 a= lgorithm will >>>>>>>>>>>> get the actual 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 >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Khushboo >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> Thanks, >>>>>>> Khushboo >>>>>>> >>>>>> >>>>>> >>>>> 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 >>> >>> >> > --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --001a11437efe936d50054f00b698 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Thanks - patch applied.
On Mon, May 8, 2017 at 8:08 AM, Khushboo Vashi= <khushboo.vashi@enterprisedb.com> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">
Hi,
Please find the attached updated patch.

Thanks,
Khushboo

On Fri, May 5, 2017 at 9:00 PM, Joao= Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io>= wrote:
Hi Khushb= oo,

We looked at your updated patch:
- the tes= ts look good!
- there's a small comment header change nee= ded in size_prettify_spec
Fixed=C2=A0
-= it looks like the previous and new functions have different behaviors (whe= re the new behavior changes units on 10000 of the lower unit, a 9999GB)=C2= =A0
- We should clarify that in size_prettify, we were mostly tal= king about name readability in your first patch, and that the original stru= cture was better (especially the sizes array)
At first glance, the new s= izePrettify appears to behave like a for loop, so that might be the simples= t refactor.

Added loop.<= /div>
=C2=A0
Thanks,
Joao and George



On Thu, May 4, 2017 at 5:02 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">
Hi,

Pleas= e find the attached updated patch.

Thanks,
Khushboo
<= div class=3D"gmail_extra">
On Fri, Apr 28, 20= 17 at 7:58 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote= :
Hi Khushboo,

<= /div>
That sounds good. Sorry if we weren't clear at first.=C2=A0

Have a good holiday weekend!

Sarah & Matt


On Fri, Apr 28, 20= 17 at 4:35 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.c= om> wrote:
Hi Sarah,

On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear &l= t;smcalear@pivotal= .io> wrote:
Hi Kushboo!

We understand your point, but we believe t= hat relying on 2 independent functions to deliver the same formatting can b= ecome a problem if the PG function changes. Our suggestion is to use a sing= le function in our javascript code to do this formatting.=C2=A0
<= br>
It seems reasonable to me and I am = going to use a single javascript function which will support PB also (as pe= r Dave we should add support till PB) .
If the = community believes we can live with this risk, let's move forward.

Thanks!
Sarah & Joao

On Thu, Apr 27, 2017 at 1:50 AM, Khushboo Vashi &= lt;khu= shboo.vashi@enterprisedb.com> wrote:
Hi Joao & Sa= rah,

O= n Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <= jdealmeidapereira@pivotal.io> wrote:
Hi Khushboo!
Thanks for your reply!
=C2=A0
SQL Files:
  • I= s there a way to avoid conditionals here?=C2=A0
  • Maybe we can use the same javascript function to prettify = all the sizes

In case of collection node (ex: Databases), I ha= ve implemented this functionality via putting a formatter for a back-grid c= olumn. So, it is applicable for the entire column not for the individual ce= ll. To apply the javascript function on a single cell for the column (strin= g column), first we need to identify that cell and then apply the function,= I think this is overhead. Just to avoid conditional sql template I would n= ot prefer this approach.
We are not totally sure we understood what you meant on= the previous statement. Are you saying that the conditionals in SQL are us= ed to ensure that we can apply a javascript function at column level instea= d of cell level?=C2=A0

Correct.=C2=A0
Our concern is that the templa= tes are being made more complex and inconsistencies are introduced in the c= ode and the UI.
=C2=A0
Inconsistencies in the UI can be avoided through making the size_formatter= same as pg_size_pretty function which I will implement.
I ha= ve checked the pg_size_pretty function code and it supports till TB format,= so I think we should keep till TB only.

In thi= s particular example, we are allowing the backend to respond sometimes with= prettified data and sometimes without it, so at UI level we will have inco= nsistencies between screens or more complex Javascript code to support some= times prettifying and sometimes not prettify the same fields.=C2=A0

We have separate logic f= or collection and single node in statistics.js and we are using javascript = code for prettifying only for collection node.
=C2=A0
=
What we were thinking was to maybe not specify on the SQL leve= l and have the same format for "Size" everywhere in the UI.=C2=A0=
=C2=A0
Here our main = concern is inconsistency in "Size" format in the UI that can be a= voided as I said earlier.
We are using pg_size_pretty function fo= r different fields like Size, Index Size, Table space size, Tuple length, S= ize of Temporary files in different modules and some of them are cell level= and we don't require to put overhead on cell level fields as sorting i= s not required for individual node statistics.
=C2=A0
=C2=A0
Thanks
Joao & Sarah
On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vas= hi <khushboo.vashi@enterprisedb.com> wrot= e:
Hi= Joao & Sarah,

Thanks for reviewing the patch.
=

On Tue, Apr= 25, 2017 at 8:34 PM, Joao Pedro De Almeida Pereira <<= a href=3D"mailto:jdealmeidapereira@pivotal.io" target=3D"_blank">jdealmeida= pereira@pivotal.io> wrote:
Hello Khushboo,

We r= eviewed 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 extract= ed into a function.=C2=A0

When I have implemented this, my first thought is exactly same as you sug= gested but =C2=A0while looking at the code I felt its not a good idea to ha= ve a function. In case of a function, we need to pass the whole result-set = as well as the list of fields which we need to be prettified. So, only for = 2 lines, I didn't find any reason to make a function.

= Javascript:

=E2=80=8B
  • The class=C2=A0Backgrid.SizeForm= atter doesn't seem to have any tests.=C2=A0
    =

Sure, will do= .=C2=A0
  • The function pg_size_pretty displays byt= es and Kilobytes differently.=C2=A0
  • Is it possible to add PB as= well?
Will check and add= PB.=C2=A0
  • The function is a little bit hard to re= ad, is it possible to refactor using private functions like:
Will make it more readable.
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 &qu= ot;rawColumn" and "col" to "column".
=

I will change the varia= ble name from =C2=A0"c" to =C2=A0"rawColumn" but I thin= k "col" is appropriate as we already have columns variable and an= yone can confuse while reading the code (for columns and column).
<= /div>

SQL Files:

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

In case of collection node (ex: Databases), I have implemented th= is functionality via putting a formatter for a back-grid column. So, it is = applicable for the entire column not for the individual cell. To apply the = javascript function on a single cell for the column (string column), first = we need to identify that cell and then apply the function, I think this is = overhead. Just to avoid conditional sql template I would not prefer this ap= proach.

Visually we saw a difference= between "Databases" statistics and a specific database statistic= s. In "Databases" statistics the "Size" is "7.4 MB= " but when you are in the specific database the "Size" is &q= uot;7420 kB".
Is this the intended behavior?

Only for the Databases (collectio= n node), the client side functionality is implemented not for individual no= de , so this behaviour is different. For the individual node still, we are = using pg_size_pretty function
=C2=A0

Thanks
Joao & Sarah

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

Thanks.

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

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

Removed the pg_size_pretty function from query f= or the collection and introduced the client side function to convert size i= nto human readable format. So, the sorting issue is fixed as the algorithm = 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
Tw= itter: @pgsnake

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


= Thanks,
Khushboo


Thanks,
Khushboo


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.bl= ogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com<= br>The Enterprise PostgreSQL Company
--001a11437efe936d50054f00b698--