public inbox for [email protected]  
help / color / mirror / Atom feed
From: Dave Page <[email protected]>
To: Joao Pedro De Almeida Pereira <[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 16:53:44 +0100
Message-ID: <CA+OCxox-5cSiObuSMdtd+OFATDaMtgCFfuqThO_Ow3V8EhC5mQ@mail.gmail.com> (raw)
In-Reply-To: <CAE+jja=Q0bQXLz4Dd4i=Gnx_P=y7nL+bS+j1JDpxhoAmh=ZV8A@mail.gmail.com>
References: <CAFOhELcc_ctZkKJYH1c+7pd3zGM3uy=FQh+nEya+yczZJ4d+BA@mail.gmail.com>
	<CA+OCxozrW_YNjrHRUC4qrzBkxLocVqmoBXnMH=NKxCrGn+0H2g@mail.gmail.com>
	<CAE+jja=Q0bQXLz4Dd4i=Gnx_P=y7nL+bS+j1JDpxhoAmh=ZV8A@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

On Tue, Apr 25, 2017 at 4:04 PM, Joao Pedro De Almeida Pereira <
[email protected]> wrote:

> 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?
>
> 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 = 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
>>
>
>


-- 
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: <CA+OCxox-5cSiObuSMdtd+OFATDaMtgCFfuqThO_Ow3V8EhC5mQ@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