Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-05-08 Thread Dave Page
Thanks - patch applied.

On Mon, May 8, 2017 at 8:08 AM, Khushboo Vashi <
khushboo.va...@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 <
> jdealmeidapere...@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 1 of the lower unit, a GB)
>> - We should clarify that in size_prettify, we were mostly talking about
>> name readability in your first patch, and that the original structure was
>> 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.va...@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.va...@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 problem
>> if the PG function changes. Our suggestion is to use a single function 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 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 <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Joao & Sarah,
>>>
>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
>>> jdealmeidapere...@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 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 approach.
>
>
 We are not totally sure we understood what you meant on the
 previous statement. Are you saying that the conditionals in SQL are 
 used to
 ensure that we can apply a javascript function at column level instead 
 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 implement.
>>> I have checked the pg_size_pretty function code and it supports till
>>> 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 Javascript 
 code
 to support sometimes prettifying and sometimes not prettify the same
 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 and
 have the same format for "Size" everywhere in the UI.


>>> Here our main concern is inconsistency in "Size" 

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-05-08 Thread Khushboo Vashi
Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Fri, May 5, 2017 at 9:00 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@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 1 of the lower unit, a GB)
> - We should clarify that in size_prettify, we were mostly talking about
> name readability in your first patch, and that the original structure was
> 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.va...@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.va...@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 problem if the PG
> function changes. Our suggestion is to use a single function 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 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 <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Joao & Sarah,
>>
>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@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 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 approach.


>>> We are not totally sure we understood what you meant on the previous
>>> statement. Are you saying that the conditionals in SQL are used to 
>>> ensure
>>> that we can apply a javascript function at column level instead 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 implement.
>> I have checked the pg_size_pretty function code and it supports till
>> 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 Javascript 
>>> code
>>> to support sometimes prettifying and sometimes not prettify the same
>>> 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 and
>>> 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 require 

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-05-05 Thread Joao Pedro De Almeida Pereira
Hi Khushboo,

We looked at your updated patch:
- the tests look good!
- there's a small comment header change needed in size_prettify_spec
- it looks like the previous and new functions have different behaviors
(where the new behavior changes units on 1 of the lower unit, a GB)
- We should clarify that in size_prettify, we were mostly talking about
name readability in your first patch, and that the original structure was
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.

Thanks,
Joao and George



On Thu, May 4, 2017 at 5:02 AM, Khushboo Vashi <
khushboo.va...@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.va...@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 problem if the PG
 function changes. Our suggestion is to use a single function 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 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 <
 khushboo.va...@enterprisedb.com> wrote:

> Hi Joao & Sarah,
>
> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@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 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
 approach.
>>>
>>>
>> We are not totally sure we understood what you meant on the previous
>> statement. Are you saying that the conditionals in SQL are used to ensure
>> that we can apply a javascript function at column level instead 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 implement.
> I have checked the pg_size_pretty function code and it supports till
> 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 Javascript code
>> to support sometimes prettifying and sometimes not prettify the same
>> 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 and
>> 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 require to
> put overhead on cell level fields as sorting is not required for 
> individual
> node statistics.
>
>
>
>> Thanks
>> Joao & Sarah
>>
>> On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Joao & Sarah,
>>>
>>> Thanks for reviewing the patch.
>>>

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-05-04 Thread Khushboo Vashi
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.va...@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 problem if the PG
>>> function changes. Our suggestion is to use a single function 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 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 <
>>> khushboo.va...@enterprisedb.com> wrote:
>>>
 Hi Joao & Sarah,

 On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
 jdealmeidapere...@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 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
>>> approach.
>>
>>
> We are not totally sure we understood what you meant on the previous
> statement. Are you saying that the conditionals in SQL are used to ensure
> that we can apply a javascript function at column level instead 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 implement.
 I have checked the pg_size_pretty function code and it supports till 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 Javascript code
> to support sometimes prettifying and sometimes not prettify the same
> 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 and
> 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 require to
 put overhead on cell level fields as sorting is not required for individual
 node statistics.



> Thanks
> Joao & Sarah
>
> On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
> khushboo.va...@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 <
>> jdealmeidapere...@pivotal.io> 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.
>>>
>>> When I have implemented this, my first thought is exactly same as
>> you suggested but  while looking at the code I felt its not a good 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 prettified.
>> So, only for 2 lines, I didn't find any reason to make a function.
>>
>>> *Javascript:*
>>> ​
>>>
>>>- The class 

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-28 Thread Matthew Kleiman
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.va...@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 problem if the PG
>> function changes. Our suggestion is to use a single function 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 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 <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi Joao & Sarah,
>>>
>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
>>> jdealmeidapere...@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 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
>> approach.
>
>
 We are not totally sure we understood what you meant on the previous
 statement. Are you saying that the conditionals in SQL are used to ensure
 that we can apply a javascript function at column level instead 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 implement.
>>> I have checked the pg_size_pretty function code and it supports till 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 Javascript code
 to support sometimes prettifying and sometimes not prettify the same
 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 and
 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 require to
>>> put overhead on cell level fields as sorting is not required for individual
>>> node statistics.
>>>
>>>
>>>
 Thanks
 Joao & Sarah

 On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
 khushboo.va...@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 <
> jdealmeidapere...@pivotal.io> 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.
>>
>> When I have implemented this, my first thought is exactly same as you
> suggested but  while looking at the code I felt its not a good 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 prettified. So, only for 2
> lines, I didn't find any reason to make a function.
>
>> *Javascript:*
>> ​
>>
>>- 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.
>

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-28 Thread Khushboo Vashi
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 problem if the PG
> function changes. Our suggestion is to use a single function 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 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 <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi Joao & Sarah,
>>
>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@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 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
> approach.


>>> We are not totally sure we understood what you meant on the previous
>>> statement. Are you saying that the conditionals in SQL are used to ensure
>>> that we can apply a javascript function at column level instead 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 implement.
>> I have checked the pg_size_pretty function code and it supports till 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 Javascript code
>>> to support sometimes prettifying and sometimes not prettify the same
>>> 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 and 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 require to
>> put overhead on cell level fields as sorting is not required for individual
>> node statistics.
>>
>>
>>
>>> Thanks
>>> Joao & Sarah
>>>
>>> On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
>>> khushboo.va...@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 <
 jdealmeidapere...@pivotal.io> 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.
>
> When I have implemented this, my first thought is exactly same as you
 suggested but  while looking at the code I felt its not a good 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 prettified. So, only for 2
 lines, I didn't find any reason to make a function.

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

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-27 Thread Sarah McAlear
Hi Kushboo!

We understand your point, but we believe that relying on 2 independent
functions to deliver the same formatting can become a problem if the PG
function changes. Our suggestion is to use a single function in our
javascript code to do this formatting.

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.va...@enterprisedb.com> wrote:

> Hi Joao & Sarah,
>
> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@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 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
 approach.
>>>
>>>
>> We are not totally sure we understood what you meant on the previous
>> statement. Are you saying that the conditionals in SQL are used to ensure
>> that we can apply a javascript function at column level instead 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 implement.
> I have checked the pg_size_pretty function code and it supports till 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 Javascript code
>> to support sometimes prettifying and sometimes not prettify the same
>> 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 and 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 require to put
> overhead on cell level fields as sorting is not required for individual
> node statistics.
>
>
>
>> Thanks
>> Joao & Sarah
>>
>> On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
>> khushboo.va...@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 <
>>> jdealmeidapere...@pivotal.io> 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.

 When I have implemented this, my first thought is exactly same as you
>>> suggested but  while looking at the code I felt its not a good 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 prettified. So, only for 2
>>> lines, I didn't find any reason to make a function.
>>>
 *Javascript:*
 ​

- 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 = 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".


 I will change the variable name from  "c" to  "rawColumn" but I think
>>> "col" is appropriate as we already have columns variable and anyone can
>>> 

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-26 Thread Khushboo Vashi
Hi Joao & Sarah,

On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@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 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
>>> approach.
>>
>>
> We are not totally sure we understood what you meant on the previous
> statement. Are you saying that the conditionals in SQL are used to ensure
> that we can apply a javascript function at column level instead 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 implement.
I have checked the pg_size_pretty function code and it supports till 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 Javascript code
> to support sometimes prettifying and sometimes not prettify the same
> 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 and 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 require to put
overhead on cell level fields as sorting is not required for individual
node statistics.



> Thanks
> Joao & Sarah
>
> On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
> khushboo.va...@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 <
>> jdealmeidapere...@pivotal.io> 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.
>>>
>>> When I have implemented this, my first thought is exactly same as you
>> suggested but  while looking at the code I felt its not a good 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 prettified. So, only for 2
>> lines, I didn't find any reason to make a function.
>>
>>> *Javascript:*
>>> ​
>>>
>>>- 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 = 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".
>>>
>>>
>>> I will change the variable name from  "c" to  "rawColumn" but I think
>> "col" is appropriate as we already have columns variable and anyone can
>> confuse while reading the code (for columns and column).
>>
>>>
>>> *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 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 

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-26 Thread Joao Pedro De Almeida Pereira
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 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
>> approach.
>
>
We are not totally sure we understood what you meant on the previous
statement. Are you saying that the conditionals in SQL are used to ensure
that we can apply a javascript function at column level instead of cell
level?

Our concern is that the templates are being made more complex and
inconsistencies are introduced in the code and the UI. 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 Javascript code to support sometimes
prettifying and sometimes not prettify the same fields.

What we were thinking was to maybe not specify on the SQL level and have
the same format for "Size" everywhere in the UI.

Thanks
Joao & Sarah

On Tue, Apr 25, 2017 at 11:48 PM, Khushboo Vashi <
khushboo.va...@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 <
> jdealmeidapere...@pivotal.io> 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.
>>
>> When I have implemented this, my first thought is exactly same as you
> suggested but  while looking at the code I felt its not a good 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 prettified. So, only for 2
> lines, I didn't find any reason to make a function.
>
>> *Javascript:*
>> ​
>>
>>- 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 = 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".
>>
>>
>> I will change the variable name from  "c" to  "rawColumn" but I think
> "col" is appropriate as we already have columns variable and anyone can
> confuse while reading the code (for columns and column).
>
>>
>> *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 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
> approach.
>
>>
>> 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?
>>
>> Only for the Databases (collection node), the client side functionality
> is implemented not for individual node , so this behaviour is different.
> For the individual node still, we are using pg_size_pretty 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.va...@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
 

Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-25 Thread Khushboo Vashi
Hi Joao & Sarah,

Thanks for reviewing the patch.

On Tue, Apr 25, 2017 at 8:34 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> 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.
>
> When I have implemented this, my first thought is exactly same as you
suggested but  while looking at the code I felt its not a good 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 prettified. So, only for 2
lines, I didn't find any reason to make a function.

> *Javascript:*
> ​
>
>- 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 = 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".
>
>
> I will change the variable name from  "c" to  "rawColumn" but I think
"col" is appropriate as we already have columns variable and anyone can
confuse while reading the code (for columns and column).

>
> *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 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
approach.

>
> 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?
>
> Only for the Databases (collection node), the client side functionality is
implemented not for individual node , so this behaviour is different. For
the individual node still, we are using pg_size_pretty 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.va...@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 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


Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-25 Thread Dave Page
On Tue, Apr 25, 2017 at 4:04 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> 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  wrote:
>
>> Ashesh, can you review/commit this please?
>>
>> Thanks.
>>
>> On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi <
>> khushboo.va...@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 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
>>
>
>


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

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


Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-25 Thread Joao Pedro De Almeida Pereira
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  wrote:

> Ashesh, can you review/commit this please?
>
> Thanks.
>
> On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi <
> khushboo.va...@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 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
>


Re: [pgadmin-hackers] [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken

2017-04-25 Thread Dave Page
Ashesh, can you review/commit this please?

Thanks.

On Tue, Apr 25, 2017 at 10:18 AM, Khushboo Vashi <
khushboo.va...@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 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