public inbox for [email protected]  

help / color / mirror / Atom feed
From: Joao Pedro De Almeida Pereira <[email protected]>
To: Dave Page <[email protected]>
Cc: Khushboo Vashi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Ashesh Vashi <[email protected]>
Subject: Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken
Date: Tue, 25 Apr 2017 11:04:19 -0400
Message-ID: <CAE+jja=Q0bQXLz4Dd4i=Gnx_P=y7nL+bS+j1JDpxhoAmh=ZV8A@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxozrW_YNjrHRUC4qrzBkxLocVqmoBXnMH=NKxCrGn+0H2g@mail.gmail.com>
References: <CAFOhELcc_ctZkKJYH1c+7pd3zGM3uy=FQh+nEya+yczZJ4d+BA@mail.gmail.com>
	<CA+OCxozrW_YNjrHRUC4qrzBkxLocVqmoBXnMH=NKxCrGn+0H2g@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hello Khushboo,

We reviewed the this patch and have some suggestions:

*Python:*
​
The functionality for adding the "can_prettify" is repeated in multiple
places. Maybe this could be extracted into a function.

*Javascript:*
​

   - 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 = findDataUnitIndex(rawData);
   if (unitIdx == 0) {
      return rawData + ' ' + this.dataUnits[i];
   }
   return formatOutput(rawData, unitIdx);
},

​


   - 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:*
​

   - 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 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 <[email protected]> wrote:

> Ashesh, can you review/commit this please?
>
> Thanks.
>
> On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi <
> [email protected]> 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 actual
>> value of size instead of formatted value.
>>
>>
>> Thanks,
>> Khushboo
>>
>>
>>
>>
>> --
>> Sent via pgadmin-hackers mailing list ([email protected])
>> 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
>


view thread (15+ messages)  latest in thread

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken
  In-Reply-To: <CAE+jja=Q0bQXLz4Dd4i=Gnx_P=y7nL+bS+j1JDpxhoAmh=ZV8A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox