public inbox for [email protected]help / color / mirror / Atom feed
[pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken 15+ messages / 5 participants [nested] [flat]
* [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-25 09:18 Khushboo Vashi <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Khushboo Vashi @ 2017-04-25 09:18 UTC (permalink / raw) To: pgadmin-hackers 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 Attachments: [text/x-patch] RM_2315.patch (10.7K, 3-RM_2315.patch) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/__init__.py index 761f757..858171e 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/__init__.py @@ -842,6 +842,13 @@ class DatabaseView(PGChildNodeView): if not status: return internal_server_error(errormsg=res) + if not did: + # Whether the column needs prettify at the client side or not + if 'columns' in res: + for col in res['columns']: + if col['name'] in ['Size', 'Size of temporary files']: + col['can_prettify'] = True + return make_json_response( data=res, status=200 diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py index e118cab..d15c7ce 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py @@ -2913,6 +2913,13 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings): conn=self.conn, schema_name=schema_name ) ) + + # Whether the column needs prettify at the client side or not + if 'columns' in res: + for col in res['columns']: + if col['name'] in ['Size']: + col['can_prettify'] = True + else: # For Individual table stats diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/__init__.py index 6d8680d..e4064f7 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/__init__.py @@ -978,6 +978,12 @@ class IndexesView(PGChildNodeView): ) ) + # Whether the column needs prettify at the client side or not + if 'columns' in res: + for col in res['columns']: + if col['name'] in ['Size']: + col['can_prettify'] = True + if not status: return internal_server_error(errormsg=res) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql index bca2d09..6cade6f 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql @@ -3,7 +3,7 @@ SELECT idx_scan AS {{ conn|qtIdent(_('Index scans')) }}, idx_tup_read AS {{ conn|qtIdent(_('Index tuples read')) }}, idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, - pg_size_pretty(pg_relation_size(indexrelid)) AS {{ conn|qtIdent(_('Size')) }} + pg_relation_size(indexrelid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_all_indexes stat JOIN pg_class cls ON cls.oid=indexrelid diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql index ee3ffa3..63a6826 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql @@ -14,12 +14,12 @@ SELECT autovacuum_count AS {{ conn|qtIdent(_('Autovacuum counter')) }}, analyze_count AS {{ conn|qtIdent(_('Analyze counter')) }}, autoanalyze_count AS {{ conn|qtIdent(_('Autoanalyze counter')) }}, - pg_size_pretty(pg_relation_size(st.relid) + pg_relation_size(st.relid) + CASE WHEN cl.reltoastrelid = 0 THEN 0 ELSE pg_relation_size(cl.reltoastrelid) + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) FROM pg_index WHERE indrelid=cl.reltoastrelid)::int8, 0) END + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) - FROM pg_index WHERE indrelid=st.relid)::int8, 0)) AS {{ conn|qtIdent(_('Size')) }} + FROM pg_index WHERE indrelid=st.relid)::int8, 0) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_all_tables st JOIN diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql index e0c7e6b..5a40841 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql @@ -17,11 +17,11 @@ SELECT slave.confl_bufferpin AS {{ conn|qtIdent(_('Bufferpin conflicts')) }}, slave.confl_deadlock AS {{ conn|qtIdent(_('Deadlock conflicts')) }}, temp_files AS {{ conn|qtIdent(_("Temporary files")) }}, - pg_size_pretty(temp_bytes) AS {{ conn|qtIdent(_("Size of temporary files")) }}, + {% if not did %}temp_bytes{% else %}pg_size_pretty(temp_bytes){% endif %} AS {{ conn|qtIdent(_("Size of temporary files")) }}, deadlocks AS {{ conn|qtIdent(_("Deadlocks")) }}, blk_read_time AS {{ conn|qtIdent(_("Block read time")) }}, blk_write_time AS {{ conn|qtIdent(_("Block write time")) }}, - pg_size_pretty(pg_database_size(db.datid)) AS {{ conn|qtIdent(_('Size')) }} + {% if not did %}pg_database_size(db.datid){% else %}pg_size_pretty(pg_database_size(db.datid)){% endif %} AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_database db LEFT JOIN pg_stat_database_conflicts slave ON db.datid=slave.datid diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql index 82b52d0..20661eb 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql @@ -16,7 +16,7 @@ SELECT slave.confl_snapshot AS {{ conn|qtIdent(_('Snapshot conflicts')) }}, slave.confl_bufferpin AS {{ conn|qtIdent(_('Bufferpin conflicts')) }}, slave.confl_deadlock AS {{ conn|qtIdent(_('Deadlock conflicts')) }}, - pg_size_pretty(pg_database_size(db.datid)) AS {{ conn|qtIdent(_('Size')) }} + {% if not did %}pg_database_size(db.datid){% else %}pg_size_pretty(pg_database_size(db.datid)){% endif %} AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_database db LEFT JOIN pg_stat_database_conflicts slave ON db.datid=slave.datid diff --git a/web/pgadmin/browser/server_groups/servers/tablespaces/__init__.py b/web/pgadmin/browser/server_groups/servers/tablespaces/__init__.py index 86effed..88f92c8 100644 --- a/web/pgadmin/browser/server_groups/servers/tablespaces/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/tablespaces/__init__.py @@ -589,6 +589,13 @@ class TablespaceView(PGChildNodeView): if not status: return internal_server_error(errormsg=res) + if not tsid: + # Whether the column needs prettify at the client side or not + if 'columns' in res: + for col in res['columns']: + if col['name'] in ['Size']: + col['can_prettify'] = True + return make_json_response( data=res, status=200 diff --git a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql index 3f91b02..485b3fc 100644 --- a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql @@ -3,7 +3,7 @@ SELECT pg_size_pretty(pg_tablespace_size({{ tsid|qtLiteral }}::OID)) AS {{ conn|qtIdent(_('Size')) }} {% else %} SELECT ts.spcname AS {{ conn|qtIdent(_('Name')) }}, - pg_size_pretty(pg_tablespace_size(ts.oid)) AS {{ conn|qtIdent(_('Size')) }} + pg_tablespace_size(ts.oid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_catalog.pg_tablespace ts; {% endif %} diff --git a/web/pgadmin/misc/statistics/static/js/statistics.js b/web/pgadmin/misc/statistics/static/js/statistics.js index 1191b16..3337960 100644 --- a/web/pgadmin/misc/statistics/static/js/statistics.js +++ b/web/pgadmin/misc/statistics/static/js/statistics.js @@ -12,6 +12,29 @@ define([ return pgBrowser.NodeStatistics; } + var SizeFormatter = Backgrid.SizeFormatter = function () {}; + _.extend(SizeFormatter.prototype, { + /** + Takes a raw value from a model and returns the human readable formatted + string for display. + + @member Backgrid.SizeFormatter + @param {*} rawData + @param {Backbone.Model} model Used for more complicated formatting + @return {*} + */ + fromRaw: function (rawData, model) { + var sizes = ['Bytes', 'KB', 'MB', 'GB', 'TB']; + if (rawData == 0) return 0; + var i = parseInt(Math.floor(Math.log(rawData) / Math.log(1024))); + if (i == 0) return rawData + ' ' + sizes[i]; + return (rawData / Math.pow(1024, i)).toFixed(1) + ' ' + sizes[i]; + }, + toRaw: function (formattedData, model) { + return formattedData; + } + }); + var PGBooleanCell = Backgrid.Extension.SwitchCell.extend({ defaults: _.extend({}, Backgrid.Extension.SwitchCell.prototype.defaults) }), @@ -300,12 +323,17 @@ define([ this.columns = []; for (var idx in columns) { - var c = columns[idx]; - this.columns.push({ - editable: false, - name: c['name'], - cell: typeCellMapper[c['type_code']] || 'string' - }); + var c = columns[idx], + col = { + editable: false, + name: c['name'], + cell: typeCellMapper[c['type_code']] || 'string' + }; + if ('can_prettify' in c && c['can_prettify']) { + col['formatter'] = SizeFormatter + } + this.columns.push(col); + } this.collection.reset(rows); ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-25 11:58 Dave Page <[email protected]> parent: Khushboo Vashi <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Dave Page @ 2017-04-25 11:58 UTC (permalink / raw) To: Khushboo Vashi <[email protected]>; +Cc: pgadmin-hackers; Ashesh Vashi <[email protected]> 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 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-25 15:04 Joao Pedro De Almeida Pereira <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 2 replies; 15+ messages in thread From: Joao Pedro De Almeida Pereira @ 2017-04-25 15:04 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: Khushboo Vashi <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> 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 > ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-25 15:53 Dave Page <[email protected]> parent: Joao Pedro De Almeida Pereira <[email protected]> 1 sibling, 0 replies; 15+ messages in thread From: Dave Page @ 2017-04-25 15:53 UTC (permalink / raw) To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: Khushboo Vashi <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> 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 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-26 03:48 Khushboo Vashi <[email protected]> parent: Joao Pedro De Almeida Pereira <[email protected]> 1 sibling, 1 reply; 15+ messages in thread From: Khushboo Vashi @ 2017-04-26 03:48 UTC (permalink / raw) To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> Hi Joao & Sarah, Thanks for reviewing the patch. On Tue, Apr 25, 2017 at 8:34 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. > > 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 <[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 >> > > Thanks, Khushboo ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-26 15:16 Joao Pedro De Almeida Pereira <[email protected]> parent: Khushboo Vashi <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Joao Pedro De Almeida Pereira @ 2017-04-26 15:16 UTC (permalink / raw) To: Khushboo Vashi <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> 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 < [email protected]> wrote: > Hi Joao & Sarah, > > Thanks for reviewing the patch. > > On Tue, Apr 25, 2017 at 8:34 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. >> >> 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 <[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 >>> >> >> > Thanks, > Khushboo > ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-27 05:50 Khushboo Vashi <[email protected]> parent: Joao Pedro De Almeida Pereira <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Khushboo Vashi @ 2017-04-27 05:50 UTC (permalink / raw) To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> Hi Joao & Sarah, On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < [email protected]> 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 < > [email protected]> wrote: > >> Hi Joao & Sarah, >> >> Thanks for reviewing the patch. >> >> On Tue, Apr 25, 2017 at 8:34 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. >>> >>> 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 <[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 >>>> >>> >>> >> Thanks, >> Khushboo >> > > Thanks, Khushboo ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-27 14:08 Sarah McAlear <[email protected]> parent: Khushboo Vashi <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Sarah McAlear @ 2017-04-27 14:08 UTC (permalink / raw) To: Khushboo Vashi <[email protected]>; +Cc: Joao Pedro De Almeida Pereira <[email protected]>; Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> 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 < [email protected]> wrote: > Hi Joao & Sarah, > > On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < > [email protected]> 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 < >> [email protected]> wrote: >> >>> Hi Joao & Sarah, >>> >>> Thanks for reviewing the patch. >>> >>> On Tue, Apr 25, 2017 at 8:34 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. >>>> >>>> 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 <[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 >>>>> >>>> >>>> >>> Thanks, >>> Khushboo >>> >> >> > Thanks, > Khushboo > ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-28 08:35 Khushboo Vashi <[email protected]> parent: Sarah McAlear <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Khushboo Vashi @ 2017-04-28 08:35 UTC (permalink / raw) To: Sarah McAlear <[email protected]>; +Cc: Joao Pedro De Almeida Pereira <[email protected]>; Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> Hi Sarah, On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear <[email protected]> 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 < > [email protected]> wrote: > >> Hi Joao & Sarah, >> >> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >> [email protected]> 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 < >>> [email protected]> wrote: >>> >>>> Hi Joao & Sarah, >>>> >>>> Thanks for reviewing the patch. >>>> >>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>> >>>>> 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 <[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] >>>>>>> g) >>>>>>> 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 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-04-28 14:28 Matthew Kleiman <[email protected]> parent: Khushboo Vashi <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Matthew Kleiman @ 2017-04-28 14:28 UTC (permalink / raw) To: Khushboo Vashi <[email protected]>; +Cc: Sarah McAlear <[email protected]>; Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> 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 < [email protected]> wrote: > Hi Sarah, > > On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear <[email protected]> > 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 < >> [email protected]> wrote: >> >>> Hi Joao & Sarah, >>> >>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >>> [email protected]> 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 < >>>> [email protected]> wrote: >>>> >>>>> Hi Joao & Sarah, >>>>> >>>>> Thanks for reviewing the patch. >>>>> >>>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>>> >>>>>> 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 <[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 >>>>>>> >>>>>> >>>>>> >>>>> Thanks, >>>>> Khushboo >>>>> >>>> >>>> >>> Thanks, >>> Khushboo >>> >> >> > Thanks, > Khushboo > ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-05-04 09:02 Khushboo Vashi <[email protected]> parent: Matthew Kleiman <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Khushboo Vashi @ 2017-05-04 09:02 UTC (permalink / raw) To: Matthew Kleiman <[email protected]>; +Cc: Sarah McAlear <[email protected]>; Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]> Hi, Please find the attached updated patch. Thanks, Khushboo On Fri, Apr 28, 2017 at 7:58 PM, Matthew Kleiman <[email protected]> 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 < > [email protected]> wrote: > >> Hi Sarah, >> >> On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear <[email protected]> >> 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 < >>> [email protected]> wrote: >>> >>>> Hi Joao & Sarah, >>>> >>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >>>> [email protected]> 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 < >>>>> [email protected]> wrote: >>>>> >>>>>> Hi Joao & Sarah, >>>>>> >>>>>> Thanks for reviewing the patch. >>>>>> >>>>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>>>> >>>>>>> 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 <[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 >>>>>>>> >>>>>>> >>>>>>> >>>>>> Thanks, >>>>>> Khushboo >>>>>> >>>>> >>>>> >>>> Thanks, >>>> Khushboo >>>> >>> >>> >> Thanks, >> Khushboo >> > > -- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers Attachments: [text/x-patch] RM_2315_ver1.patch (25.3K, 3-RM_2315_ver1.patch) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js index 02b77e1..eee9ad2 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js @@ -627,6 +627,7 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) { canDropCascade: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Index size'], Init: function() { /* Avoid multiple registration of menus */ if (this.initialized) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js index 6439caf..a792e31 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js @@ -19,6 +19,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Index size'], parent_type: 'table', canDrop: true, canDropCascade: true, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js index 8739322..249fbaf 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js @@ -12,7 +12,8 @@ function($, _, S, pgAdmin, pgBrowser, Backform, alertify) { sqlAlterHelp: 'sql-alterindex.html', sqlCreateHelp: 'sql-createindex.html', columns: ['name', 'description'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size', 'Index size'] }); }; @@ -215,6 +216,7 @@ function($, _, S, pgAdmin, pgBrowser, Backform, alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size', 'Index size'], Init: function() { /* Avoid mulitple registration of menus */ if (this.initialized) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql index b779e62..6e170b9 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql @@ -4,12 +4,12 @@ SELECT idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, idx_blks_read AS {{ conn|qtIdent(_('Index blocks read')) }}, idx_blks_hit AS {{ conn|qtIdent(_('Index blocks hit')) }}, - pg_size_pretty(pg_relation_size({{ exid }}::OID)) AS {{ conn|qtIdent(_('Index size')) }} + pg_relation_size({{ exid }}::OID) AS {{ conn|qtIdent(_('Index size')) }} {#=== Extended stats ===#} {% if is_pgstattuple %} ,version AS {{ conn|qtIdent(_('Version')) }}, tree_level AS {{ conn|qtIdent(_('Tree level')) }}, - pg_size_pretty(index_size) AS {{ conn|qtIdent(_('Index size')) }}, + index_size AS {{ conn|qtIdent(_('Index size')) }}, root_block_no AS {{ conn|qtIdent(_('Root block no')) }}, internal_pages AS {{ conn|qtIdent(_('Internal pages')) }}, leaf_pages AS {{ conn|qtIdent(_('Leaf pages')) }}, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql index bca2d09..6cade6f 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql @@ -3,7 +3,7 @@ SELECT idx_scan AS {{ conn|qtIdent(_('Index scans')) }}, idx_tup_read AS {{ conn|qtIdent(_('Index tuples read')) }}, idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, - pg_size_pretty(pg_relation_size(indexrelid)) AS {{ conn|qtIdent(_('Size')) }} + pg_relation_size(indexrelid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_all_indexes stat JOIN pg_class cls ON cls.oid=indexrelid diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql index 44571e6..459a7aa 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql @@ -4,12 +4,12 @@ SELECT idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, idx_blks_read AS {{ conn|qtIdent(_('Index blocks read')) }}, idx_blks_hit AS {{ conn|qtIdent(_('Index blocks hit')) }}, - pg_size_pretty(pg_relation_size({{ idx }}::OID)) AS {{ conn|qtIdent(_('Index size')) }} + pg_relation_size({{ idx }}::OID) AS {{ conn|qtIdent(_('Index size')) }} {#=== Extended stats ===#} {% if is_pgstattuple %} ,version AS {{ conn|qtIdent(_('Version')) }}, tree_level AS {{ conn|qtIdent(_('Tree level')) }}, - pg_size_pretty(index_size) AS {{ conn|qtIdent(_('Index size')) }}, + index_size AS {{ conn|qtIdent(_('Index size')) }}, root_block_no AS {{ conn|qtIdent(_('Root block no')) }}, internal_pages AS {{ conn|qtIdent(_('Internal pages')) }}, leaf_pages AS {{ conn|qtIdent(_('Leaf pages')) }}, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql index c111a83..793d780 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql @@ -4,12 +4,12 @@ SELECT idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, idx_blks_read AS {{ conn|qtIdent(_('Index blocks read')) }}, idx_blks_hit AS {{ conn|qtIdent(_('Index blocks hit')) }}, - pg_size_pretty(pg_relation_size({{ cid }}::OID)) AS {{ conn|qtIdent(_('Index size')) }} + pg_relation_size({{ cid }}::OID) AS {{ conn|qtIdent(_('Index size')) }} {#=== Extended stats ===#} {% if is_pgstattuple %} ,version AS {{ conn|qtIdent(_('Version')) }}, tree_level AS {{ conn|qtIdent(_('Tree level')) }}, - pg_size_pretty(index_size) AS {{ conn|qtIdent(_('Index size')) }}, + index_size AS {{ conn|qtIdent(_('Index size')) }}, root_block_no AS {{ conn|qtIdent(_('Root block no')) }}, internal_pages AS {{ conn|qtIdent(_('Internal pages')) }}, leaf_pages AS {{ conn|qtIdent(_('Leaf pages')) }}, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js index 3487e7e..bf6b76f 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js @@ -13,7 +13,10 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { label: '{{ _('Tables') }}', type: 'coll-table', columns: ['name', 'relowner', 'description'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size', 'Indexes size', 'Table size', + 'Toast table size', 'Tuple length', + 'Dead tuple length', 'Free space'] }); }; @@ -25,6 +28,9 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size', 'Indexes size', 'Table size', + 'Toast table size', 'Tuple length', + 'Dead tuple length', 'Free space'], sqlAlterHelp: 'sql-altertable.html', sqlCreateHelp: 'sql-createtable.html', dialogHelp: '{{ url_for('help.static', filename='table_dialog.html') }}', diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql index ee3ffa3..63a6826 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql @@ -14,12 +14,12 @@ SELECT autovacuum_count AS {{ conn|qtIdent(_('Autovacuum counter')) }}, analyze_count AS {{ conn|qtIdent(_('Analyze counter')) }}, autoanalyze_count AS {{ conn|qtIdent(_('Autoanalyze counter')) }}, - pg_size_pretty(pg_relation_size(st.relid) + pg_relation_size(st.relid) + CASE WHEN cl.reltoastrelid = 0 THEN 0 ELSE pg_relation_size(cl.reltoastrelid) + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) FROM pg_index WHERE indrelid=cl.reltoastrelid)::int8, 0) END + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) - FROM pg_index WHERE indrelid=st.relid)::int8, 0)) AS {{ conn|qtIdent(_('Size')) }} + FROM pg_index WHERE indrelid=st.relid)::int8, 0) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_all_tables st JOIN diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql index 6a73e53..30137a8 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql @@ -25,23 +25,23 @@ SELECT autovacuum_count AS {{ conn|qtIdent(_('Autovacuum counter')) }}, analyze_count AS {{ conn|qtIdent(_('Analyze counter')) }}, autoanalyze_count AS {{ conn|qtIdent(_('Autoanalyze counter')) }}, - pg_size_pretty(pg_relation_size(stat.relid)) AS {{ conn|qtIdent(_('Table size')) }}, - CASE WHEN cl.reltoastrelid = 0 THEN NULL ELSE pg_size_pretty(pg_relation_size(cl.reltoastrelid) + pg_relation_size(stat.relid) AS {{ conn|qtIdent(_('Table size')) }}, + CASE WHEN cl.reltoastrelid = 0 THEN NULL ELSE pg_relation_size(cl.reltoastrelid) + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) - FROM pg_index WHERE indrelid=cl.reltoastrelid)::int8, 0)) + FROM pg_index WHERE indrelid=cl.reltoastrelid)::int8, 0) END AS {{ conn|qtIdent(_('Toast table size')) }}, - pg_size_pretty(COALESCE((SELECT SUM(pg_relation_size(indexrelid)) - FROM pg_index WHERE indrelid=stat.relid)::int8, 0)) + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) + FROM pg_index WHERE indrelid=stat.relid)::int8, 0) AS {{ conn|qtIdent(_('Indexes size')) }} {% if is_pgstattuple %} {#== EXTENDED STATS ==#} ,tuple_count AS {{ conn|qtIdent(_('Tuple count')) }}, - pg_size_pretty(tuple_len) AS {{ conn|qtIdent(_('Tuple length')) }}, + tuple_len AS {{ conn|qtIdent(_('Tuple length')) }}, tuple_percent AS {{ conn|qtIdent(_('Tuple percent')) }}, dead_tuple_count AS {{ conn|qtIdent(_('Dead tuple count')) }}, - pg_size_pretty(dead_tuple_len) AS {{ conn|qtIdent(_('Dead tuple length')) }}, + dead_tuple_len AS {{ conn|qtIdent(_('Dead tuple length')) }}, dead_tuple_percent AS {{ conn|qtIdent(_('Dead tuple percent')) }}, - pg_size_pretty(free_space) AS {{ conn|qtIdent(_('Free space')) }}, + free_space AS {{ conn|qtIdent(_('Free space')) }}, free_percent AS {{ conn|qtIdent(_('Free percent')) }} FROM pgstattuple('{{schema_name}}.{{table_name}}'), pg_stat_all_tables stat diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js index c567fb7..bbe73fe 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js @@ -12,7 +12,8 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) { label: '{{ _('Databases') }}', type: 'coll-database', columns: ['name', 'datowner', 'comments'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size', 'Size of temporary files'] }); }; @@ -26,6 +27,7 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size', 'Size of temporary files'], canDrop: function(node) { return node.canDrop; }, diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql index e0c7e6b..5b19243 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql @@ -17,11 +17,11 @@ SELECT slave.confl_bufferpin AS {{ conn|qtIdent(_('Bufferpin conflicts')) }}, slave.confl_deadlock AS {{ conn|qtIdent(_('Deadlock conflicts')) }}, temp_files AS {{ conn|qtIdent(_("Temporary files")) }}, - pg_size_pretty(temp_bytes) AS {{ conn|qtIdent(_("Size of temporary files")) }}, + temp_bytes AS {{ conn|qtIdent(_("Size of temporary files")) }}, deadlocks AS {{ conn|qtIdent(_("Deadlocks")) }}, blk_read_time AS {{ conn|qtIdent(_("Block read time")) }}, blk_write_time AS {{ conn|qtIdent(_("Block write time")) }}, - pg_size_pretty(pg_database_size(db.datid)) AS {{ conn|qtIdent(_('Size')) }} + pg_database_size(db.datid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_database db LEFT JOIN pg_stat_database_conflicts slave ON db.datid=slave.datid diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql index 82b52d0..4f23b9d 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql @@ -16,7 +16,7 @@ SELECT slave.confl_snapshot AS {{ conn|qtIdent(_('Snapshot conflicts')) }}, slave.confl_bufferpin AS {{ conn|qtIdent(_('Bufferpin conflicts')) }}, slave.confl_deadlock AS {{ conn|qtIdent(_('Deadlock conflicts')) }}, - pg_size_pretty(pg_database_size(db.datid)) AS {{ conn|qtIdent(_('Size')) }} + pg_database_size(db.datid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_database db LEFT JOIN pg_stat_database_conflicts slave ON db.datid=slave.datid diff --git a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js index 601372e..14feb80 100644 --- a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js +++ b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js @@ -9,7 +9,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { label: '{{ _('Tablespaces') }}', type: 'coll-tablespace', columns: ['name', 'spcuser', 'description'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size'] }); }; @@ -25,6 +26,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { canDrop: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size'], Init: function() { /* Avoid mulitple registration of menus */ if (this.initialized) diff --git a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql index 3f91b02..5fc429e 100644 --- a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql @@ -1,9 +1,9 @@ {### SQL to fetch tablespace object stats ###} {% if tsid %} -SELECT pg_size_pretty(pg_tablespace_size({{ tsid|qtLiteral }}::OID)) AS {{ conn|qtIdent(_('Size')) }} +SELECT pg_tablespace_size({{ tsid|qtLiteral }}::OID) AS {{ conn|qtIdent(_('Size')) }} {% else %} SELECT ts.spcname AS {{ conn|qtIdent(_('Name')) }}, - pg_size_pretty(pg_tablespace_size(ts.oid)) AS {{ conn|qtIdent(_('Size')) }} + pg_tablespace_size(ts.oid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_catalog.pg_tablespace ts; {% endif %} diff --git a/web/pgadmin/misc/statistics/static/js/statistics.js b/web/pgadmin/misc/statistics/static/js/statistics.js index 1191b16..e1bbd55 100644 --- a/web/pgadmin/misc/statistics/static/js/statistics.js +++ b/web/pgadmin/misc/statistics/static/js/statistics.js @@ -1,7 +1,7 @@ define([ 'underscore', 'underscore.string', 'jquery', 'pgadmin.browser', 'backgrid', - 'alertify', 'wcdocker', 'pgadmin.backgrid', 'pgadmin.alertifyjs', 'pgadmin.browser.messages', -], function(_, S, $, pgBrowser, Backgrid, Alertify) { + 'alertify', 'sources/size_prettify' +], function(_, S, $, pgBrowser, Backgrid, Alertify, sizePrettify) { if (pgBrowser.NodeStatistics) return pgBrowser.NodeStatistics; @@ -12,6 +12,25 @@ define([ return pgBrowser.NodeStatistics; } + var SizeFormatter = Backgrid.SizeFormatter = function () {}; + _.extend(SizeFormatter.prototype, { + /** + Takes a raw value from a model and returns the human readable formatted + string for display. + + @member Backgrid.SizeFormatter + @param {*} rawData + @param {Backbone.Model} model Used for more complicated formatting + @return {*} + */ + fromRaw: function (rawData, model) { + return sizePrettify(rawData); + }, + toRaw: function (formattedData, model) { + return formattedData; + } + }); + var PGBooleanCell = Backgrid.Extension.SwitchCell.extend({ defaults: _.extend({}, Backgrid.Extension.SwitchCell.prototype.defaults) }), @@ -200,9 +219,9 @@ define([ if (res.data) { var data = res.data; if (node.hasCollectiveStatistics || data['rows'].length > 1) { - self.__createMultiLineStatistics.call(self, data); + self.__createMultiLineStatistics.call(self, data, node.statsPrettifyFields); } else { - self.__createSingleLineStatistics.call(self, data); + self.__createSingleLineStatistics.call(self, data, node.statsPrettifyFields); } if (self.grid) { @@ -294,24 +313,29 @@ define([ } }, - __createMultiLineStatistics: function(data) { + __createMultiLineStatistics: function(data, prettifyFields) { var rows = data['rows'], columns = data['columns']; this.columns = []; for (var idx in columns) { - var c = columns[idx]; - this.columns.push({ - editable: false, - name: c['name'], - cell: typeCellMapper[c['type_code']] || 'string' - }); + var rawColumn = columns[idx], + col = { + editable: false, + name: rawColumn['name'], + cell: typeCellMapper[rawColumn['type_code']] || 'string' + }; + if (_.indexOf(prettifyFields, rawColumn['name']) != -1) { + col['formatter'] = SizeFormatter + } + this.columns.push(col); + } this.collection.reset(rows); }, - __createSingleLineStatistics: function(data) { + __createSingleLineStatistics: function(data, prettifyFields) { var row = data['rows'][0], columns = data['columns'] res = []; @@ -322,7 +346,7 @@ define([ res.push({ 'statistics': name, // Check if row is undefined? - 'value': row && row[name] ? row[name] : null + 'value': row && row[name] ? ((_.indexOf(prettifyFields, name) != -1) ? sizePrettify(row[name]) : row[name]) : null }); } diff --git a/web/pgadmin/static/js/size_prettify.js b/web/pgadmin/static/js/size_prettify.js new file mode 100644 index 0000000..13a26ae --- /dev/null +++ b/web/pgadmin/static/js/size_prettify.js @@ -0,0 +1,42 @@ +define([], + function () { + var sizePrettify = function (rawSize) { + var size = Math.abs(rawSize), + limit = 10 * 1024, + limit2 = limit - 1; + + if (size < limit) + return size + " bytes"; + else + { + size = size / 1024; /* keep one extra bit for rounding */ + if (size < limit2) + return Math.round(size) + " kB"; + else + { + size = size / 1024; + if (size < limit2) + return Math.round(size) + " MB"; + else + { + size = size / 1024; + if (size < limit2) + return Math.round(size) + " GB"; + else + { + size = size / 1024; + if (size < limit2) + return Math.round(size) + " TB"; + else + { + size = size / 1024; + return Math.round(size) + " PB"; + } + } + } + } + } + }; + + return sizePrettify; +}); diff --git a/web/regression/javascript/size_prettify_spec.js b/web/regression/javascript/size_prettify_spec.js new file mode 100644 index 0000000..9e7fd1d --- /dev/null +++ b/web/regression/javascript/size_prettify_spec.js @@ -0,0 +1,71 @@ +////////////////////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +////////////////////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2017, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////////////////// + +define(["sources/size_prettify"], function (sizePrettify) { + describe("sizePrettify", function () { + describe("when size is 0", function () { + it("returns 0 bytes", function () { + expect(sizePrettify(0)).toEqual("0 bytes"); + }); + }); + + describe("when size >= 10kB and size < 10 MB", function () { + it("returns size in kB", function () { + expect(sizePrettify(10240)).toEqual("10 kB"); + }); + + it("returns size in kB", function () { + expect(sizePrettify(99999)).toEqual("98 kB"); + }); + }); + + + describe("when size >= 10MB and size < 10 GB", function () { + it("returns size in MB", function () { + expect(sizePrettify(10485760)).toEqual("10 MB"); + }); + + it("returns size in MB", function () { + expect(sizePrettify(44040192)).toEqual("42 MB"); + }); + }); + + + describe("when size >= 10GB and size < 10 TB", function () { + it("returns size in GB", function () { + expect(sizePrettify(10737418240)).toEqual("10 GB"); + }); + + it("returns size in GB", function () { + expect(sizePrettify(10736344498176)).toEqual("9999 GB"); + }); + }); + + describe("when size >= 10TB and size < 10 PB", function () { + it("returns size in TB", function () { + expect(sizePrettify(10995116277760)).toEqual("10 TB"); + }); + + it("returns size in TB", function () { + expect(sizePrettify(29995116277760)).toEqual("27 TB"); + }); + }); + + describe("when size >= 10 PB", function () { + it("returns size in PB", function () { + expect(sizePrettify(11258999068426200)).toEqual("10 PB"); + }); + + }); + + }); +}); ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-05-05 15:30 Joao Pedro De Almeida Pereira <[email protected]> parent: Khushboo Vashi <[email protected]> 0 siblings, 2 replies; 15+ messages in thread From: Joao Pedro De Almeida Pereira @ 2017-05-05 15:30 UTC (permalink / raw) To: Khushboo Vashi <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]>; George Gelashvili <[email protected]> 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 10000 of the lower unit, a 9999GB) - 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 < [email protected]> wrote: > Hi, > > Please find the attached updated patch. > > Thanks, > Khushboo > > On Fri, Apr 28, 2017 at 7:58 PM, Matthew Kleiman <[email protected]> > 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 < >> [email protected]> wrote: >> >>> Hi Sarah, >>> >>> On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear <[email protected]> >>> 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 < >>>> [email protected]> wrote: >>>> >>>>> Hi Joao & Sarah, >>>>> >>>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >>>>> [email protected]> 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 < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi Joao & Sarah, >>>>>>> >>>>>>> Thanks for reviewing the patch. >>>>>>> >>>>>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>>>>> >>>>>>>> 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 <[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 >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> Thanks, >>>>>>> Khushboo >>>>>>> >>>>>> >>>>>> >>>>> Thanks, >>>>> Khushboo >>>>> >>>> >>>> >>> Thanks, >>> Khushboo >>> >> >> > > > -- > Sent via pgadmin-hackers mailing list ([email protected]) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > > ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-05-05 15:43 Khushboo Vashi <[email protected]> parent: Joao Pedro De Almeida Pereira <[email protected]> 1 sibling, 0 replies; 15+ messages in thread From: Khushboo Vashi @ 2017-05-05 15:43 UTC (permalink / raw) To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]>; George Gelashvili <[email protected]> Hi Joao & George, On Fri, May 5, 2017 at 9:00 PM, Joao Pedro De Almeida Pereira < [email protected]> 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 > ok. will change. > - it looks like the previous and new functions have different behaviors > (where the new behavior changes units on 10000 of the lower unit, a 9999GB) > It is the same behaviour as pg_size_pretty function of PostgreSQL > - 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. > > I have followed the code structure of the pg_size_pretty function of PostgreSQL :) . Thanks, > Joao and George > > > > On Thu, May 4, 2017 at 5:02 AM, Khushboo Vashi < > [email protected]> wrote: > >> Hi, >> >> Please find the attached updated patch. >> >> Thanks, >> Khushboo >> >> On Fri, Apr 28, 2017 at 7:58 PM, Matthew Kleiman <[email protected]> >> 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 < >>> [email protected]> wrote: >>> >>>> Hi Sarah, >>>> >>>> On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear <[email protected]> >>>> 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 < >>>>> [email protected]> wrote: >>>>> >>>>>> Hi Joao & Sarah, >>>>>> >>>>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >>>>>> [email protected]> 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 < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Hi Joao & Sarah, >>>>>>>> >>>>>>>> Thanks for reviewing the patch. >>>>>>>> >>>>>>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>>>>>> >>>>>>>>> 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 <[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 >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> Thanks, >>>>>>>> Khushboo >>>>>>>> >>>>>>> >>>>>>> >>>>>> Thanks, >>>>>> Khushboo >>>>>> >>>>> >>>>> >>>> Thanks, >>>> Khushboo >>>> >>> >>> >> >> >> -- >> Sent via pgadmin-hackers mailing list ([email protected]) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgadmin-hackers >> >> > ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-05-08 07:08 Khushboo Vashi <[email protected]> parent: Joao Pedro De Almeida Pereira <[email protected]> 1 sibling, 1 reply; 15+ messages in thread From: Khushboo Vashi @ 2017-05-08 07:08 UTC (permalink / raw) To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]>; George Gelashvili <[email protected]> Hi, Please find the attached updated patch. Thanks, Khushboo On Fri, May 5, 2017 at 9:00 PM, Joao Pedro De Almeida Pereira < [email protected]> 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 9999GB) > - 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 < > [email protected]> wrote: > >> Hi, >> >> Please find the attached updated patch. >> >> Thanks, >> Khushboo >> >> On Fri, Apr 28, 2017 at 7:58 PM, Matthew Kleiman <[email protected]> >> 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 < >>> [email protected]> wrote: >>> >>>> Hi Sarah, >>>> >>>> On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear <[email protected]> >>>> 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 < >>>>> [email protected]> wrote: >>>>> >>>>>> Hi Joao & Sarah, >>>>>> >>>>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >>>>>> [email protected]> 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 < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Hi Joao & Sarah, >>>>>>>> >>>>>>>> Thanks for reviewing the patch. >>>>>>>> >>>>>>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>>>>>> >>>>>>>>> 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 <[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 >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> Thanks, >>>>>>>> Khushboo >>>>>>>> >>>>>>> >>>>>>> >>>>>> Thanks, >>>>>> Khushboo >>>>>> >>>>> >>>>> >>>> Thanks, >>>> Khushboo >>>> >>> >>> >> >> >> -- >> Sent via pgadmin-hackers mailing list ([email protected]) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgadmin-hackers >> >> > -- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers Attachments: [text/x-patch] RM_2315_ver2.patch (24.7K, 3-RM_2315_ver2.patch) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js index 02b77e1..eee9ad2 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js @@ -627,6 +627,7 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) { canDropCascade: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Index size'], Init: function() { /* Avoid multiple registration of menus */ if (this.initialized) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js index 6439caf..a792e31 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/templates/index_constraint/js/index_constraint.js @@ -19,6 +19,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Index size'], parent_type: 'table', canDrop: true, canDropCascade: true, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js index 8739322..249fbaf 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/templates/index/js/index.js @@ -12,7 +12,8 @@ function($, _, S, pgAdmin, pgBrowser, Backform, alertify) { sqlAlterHelp: 'sql-alterindex.html', sqlCreateHelp: 'sql-createindex.html', columns: ['name', 'description'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size', 'Index size'] }); }; @@ -215,6 +216,7 @@ function($, _, S, pgAdmin, pgBrowser, Backform, alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size', 'Index size'], Init: function() { /* Avoid mulitple registration of menus */ if (this.initialized) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql index b779e62..6e170b9 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/default/stats.sql @@ -4,12 +4,12 @@ SELECT idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, idx_blks_read AS {{ conn|qtIdent(_('Index blocks read')) }}, idx_blks_hit AS {{ conn|qtIdent(_('Index blocks hit')) }}, - pg_size_pretty(pg_relation_size({{ exid }}::OID)) AS {{ conn|qtIdent(_('Index size')) }} + pg_relation_size({{ exid }}::OID) AS {{ conn|qtIdent(_('Index size')) }} {#=== Extended stats ===#} {% if is_pgstattuple %} ,version AS {{ conn|qtIdent(_('Version')) }}, tree_level AS {{ conn|qtIdent(_('Tree level')) }}, - pg_size_pretty(index_size) AS {{ conn|qtIdent(_('Index size')) }}, + index_size AS {{ conn|qtIdent(_('Index size')) }}, root_block_no AS {{ conn|qtIdent(_('Root block no')) }}, internal_pages AS {{ conn|qtIdent(_('Internal pages')) }}, leaf_pages AS {{ conn|qtIdent(_('Leaf pages')) }}, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql index bca2d09..6cade6f 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/coll_stats.sql @@ -3,7 +3,7 @@ SELECT idx_scan AS {{ conn|qtIdent(_('Index scans')) }}, idx_tup_read AS {{ conn|qtIdent(_('Index tuples read')) }}, idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, - pg_size_pretty(pg_relation_size(indexrelid)) AS {{ conn|qtIdent(_('Size')) }} + pg_relation_size(indexrelid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_all_indexes stat JOIN pg_class cls ON cls.oid=indexrelid diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql index 44571e6..459a7aa 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index/sql/default/stats.sql @@ -4,12 +4,12 @@ SELECT idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, idx_blks_read AS {{ conn|qtIdent(_('Index blocks read')) }}, idx_blks_hit AS {{ conn|qtIdent(_('Index blocks hit')) }}, - pg_size_pretty(pg_relation_size({{ idx }}::OID)) AS {{ conn|qtIdent(_('Index size')) }} + pg_relation_size({{ idx }}::OID) AS {{ conn|qtIdent(_('Index size')) }} {#=== Extended stats ===#} {% if is_pgstattuple %} ,version AS {{ conn|qtIdent(_('Version')) }}, tree_level AS {{ conn|qtIdent(_('Tree level')) }}, - pg_size_pretty(index_size) AS {{ conn|qtIdent(_('Index size')) }}, + index_size AS {{ conn|qtIdent(_('Index size')) }}, root_block_no AS {{ conn|qtIdent(_('Root block no')) }}, internal_pages AS {{ conn|qtIdent(_('Internal pages')) }}, leaf_pages AS {{ conn|qtIdent(_('Leaf pages')) }}, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql index c111a83..793d780 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/index_constraint/sql/stats.sql @@ -4,12 +4,12 @@ SELECT idx_tup_fetch AS {{ conn|qtIdent(_('Index tuples fetched')) }}, idx_blks_read AS {{ conn|qtIdent(_('Index blocks read')) }}, idx_blks_hit AS {{ conn|qtIdent(_('Index blocks hit')) }}, - pg_size_pretty(pg_relation_size({{ cid }}::OID)) AS {{ conn|qtIdent(_('Index size')) }} + pg_relation_size({{ cid }}::OID) AS {{ conn|qtIdent(_('Index size')) }} {#=== Extended stats ===#} {% if is_pgstattuple %} ,version AS {{ conn|qtIdent(_('Version')) }}, tree_level AS {{ conn|qtIdent(_('Tree level')) }}, - pg_size_pretty(index_size) AS {{ conn|qtIdent(_('Index size')) }}, + index_size AS {{ conn|qtIdent(_('Index size')) }}, root_block_no AS {{ conn|qtIdent(_('Root block no')) }}, internal_pages AS {{ conn|qtIdent(_('Internal pages')) }}, leaf_pages AS {{ conn|qtIdent(_('Leaf pages')) }}, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js index 3487e7e..bf6b76f 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/js/table.js @@ -13,7 +13,10 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { label: '{{ _('Tables') }}', type: 'coll-table', columns: ['name', 'relowner', 'description'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size', 'Indexes size', 'Table size', + 'Toast table size', 'Tuple length', + 'Dead tuple length', 'Free space'] }); }; @@ -25,6 +28,9 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size', 'Indexes size', 'Table size', + 'Toast table size', 'Tuple length', + 'Dead tuple length', 'Free space'], sqlAlterHelp: 'sql-altertable.html', sqlCreateHelp: 'sql-createtable.html', dialogHelp: '{{ url_for('help.static', filename='table_dialog.html') }}', diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql index ee3ffa3..63a6826 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/coll_table_stats.sql @@ -14,12 +14,12 @@ SELECT autovacuum_count AS {{ conn|qtIdent(_('Autovacuum counter')) }}, analyze_count AS {{ conn|qtIdent(_('Analyze counter')) }}, autoanalyze_count AS {{ conn|qtIdent(_('Autoanalyze counter')) }}, - pg_size_pretty(pg_relation_size(st.relid) + pg_relation_size(st.relid) + CASE WHEN cl.reltoastrelid = 0 THEN 0 ELSE pg_relation_size(cl.reltoastrelid) + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) FROM pg_index WHERE indrelid=cl.reltoastrelid)::int8, 0) END + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) - FROM pg_index WHERE indrelid=st.relid)::int8, 0)) AS {{ conn|qtIdent(_('Size')) }} + FROM pg_index WHERE indrelid=st.relid)::int8, 0) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_all_tables st JOIN diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql index 6a73e53..30137a8 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/table/sql/default/stats.sql @@ -25,23 +25,23 @@ SELECT autovacuum_count AS {{ conn|qtIdent(_('Autovacuum counter')) }}, analyze_count AS {{ conn|qtIdent(_('Analyze counter')) }}, autoanalyze_count AS {{ conn|qtIdent(_('Autoanalyze counter')) }}, - pg_size_pretty(pg_relation_size(stat.relid)) AS {{ conn|qtIdent(_('Table size')) }}, - CASE WHEN cl.reltoastrelid = 0 THEN NULL ELSE pg_size_pretty(pg_relation_size(cl.reltoastrelid) + pg_relation_size(stat.relid) AS {{ conn|qtIdent(_('Table size')) }}, + CASE WHEN cl.reltoastrelid = 0 THEN NULL ELSE pg_relation_size(cl.reltoastrelid) + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) - FROM pg_index WHERE indrelid=cl.reltoastrelid)::int8, 0)) + FROM pg_index WHERE indrelid=cl.reltoastrelid)::int8, 0) END AS {{ conn|qtIdent(_('Toast table size')) }}, - pg_size_pretty(COALESCE((SELECT SUM(pg_relation_size(indexrelid)) - FROM pg_index WHERE indrelid=stat.relid)::int8, 0)) + COALESCE((SELECT SUM(pg_relation_size(indexrelid)) + FROM pg_index WHERE indrelid=stat.relid)::int8, 0) AS {{ conn|qtIdent(_('Indexes size')) }} {% if is_pgstattuple %} {#== EXTENDED STATS ==#} ,tuple_count AS {{ conn|qtIdent(_('Tuple count')) }}, - pg_size_pretty(tuple_len) AS {{ conn|qtIdent(_('Tuple length')) }}, + tuple_len AS {{ conn|qtIdent(_('Tuple length')) }}, tuple_percent AS {{ conn|qtIdent(_('Tuple percent')) }}, dead_tuple_count AS {{ conn|qtIdent(_('Dead tuple count')) }}, - pg_size_pretty(dead_tuple_len) AS {{ conn|qtIdent(_('Dead tuple length')) }}, + dead_tuple_len AS {{ conn|qtIdent(_('Dead tuple length')) }}, dead_tuple_percent AS {{ conn|qtIdent(_('Dead tuple percent')) }}, - pg_size_pretty(free_space) AS {{ conn|qtIdent(_('Free space')) }}, + free_space AS {{ conn|qtIdent(_('Free space')) }}, free_percent AS {{ conn|qtIdent(_('Free percent')) }} FROM pgstattuple('{{schema_name}}.{{table_name}}'), pg_stat_all_tables stat diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js index c567fb7..bbe73fe 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js @@ -12,7 +12,8 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) { label: '{{ _('Databases') }}', type: 'coll-database', columns: ['name', 'datowner', 'comments'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size', 'Size of temporary files'] }); }; @@ -26,6 +27,7 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) { hasSQL: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size', 'Size of temporary files'], canDrop: function(node) { return node.canDrop; }, diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql index e0c7e6b..5b19243 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/9.2_plus/stats.sql @@ -17,11 +17,11 @@ SELECT slave.confl_bufferpin AS {{ conn|qtIdent(_('Bufferpin conflicts')) }}, slave.confl_deadlock AS {{ conn|qtIdent(_('Deadlock conflicts')) }}, temp_files AS {{ conn|qtIdent(_("Temporary files")) }}, - pg_size_pretty(temp_bytes) AS {{ conn|qtIdent(_("Size of temporary files")) }}, + temp_bytes AS {{ conn|qtIdent(_("Size of temporary files")) }}, deadlocks AS {{ conn|qtIdent(_("Deadlocks")) }}, blk_read_time AS {{ conn|qtIdent(_("Block read time")) }}, blk_write_time AS {{ conn|qtIdent(_("Block write time")) }}, - pg_size_pretty(pg_database_size(db.datid)) AS {{ conn|qtIdent(_('Size')) }} + pg_database_size(db.datid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_database db LEFT JOIN pg_stat_database_conflicts slave ON db.datid=slave.datid diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql index 82b52d0..4f23b9d 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/sql/default/stats.sql @@ -16,7 +16,7 @@ SELECT slave.confl_snapshot AS {{ conn|qtIdent(_('Snapshot conflicts')) }}, slave.confl_bufferpin AS {{ conn|qtIdent(_('Bufferpin conflicts')) }}, slave.confl_deadlock AS {{ conn|qtIdent(_('Deadlock conflicts')) }}, - pg_size_pretty(pg_database_size(db.datid)) AS {{ conn|qtIdent(_('Size')) }} + pg_database_size(db.datid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_stat_database db LEFT JOIN pg_stat_database_conflicts slave ON db.datid=slave.datid diff --git a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js index 601372e..14feb80 100644 --- a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js +++ b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/js/tablespaces.js @@ -9,7 +9,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { label: '{{ _('Tablespaces') }}', type: 'coll-tablespace', columns: ['name', 'spcuser', 'description'], - hasStatistics: true + hasStatistics: true, + statsPrettifyFields: ['Size'] }); }; @@ -25,6 +26,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify) { canDrop: true, hasDepends: true, hasStatistics: true, + statsPrettifyFields: ['Size'], Init: function() { /* Avoid mulitple registration of menus */ if (this.initialized) diff --git a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql index 3f91b02..5fc429e 100644 --- a/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql +++ b/web/pgadmin/browser/server_groups/servers/tablespaces/templates/tablespaces/sql/default/stats.sql @@ -1,9 +1,9 @@ {### SQL to fetch tablespace object stats ###} {% if tsid %} -SELECT pg_size_pretty(pg_tablespace_size({{ tsid|qtLiteral }}::OID)) AS {{ conn|qtIdent(_('Size')) }} +SELECT pg_tablespace_size({{ tsid|qtLiteral }}::OID) AS {{ conn|qtIdent(_('Size')) }} {% else %} SELECT ts.spcname AS {{ conn|qtIdent(_('Name')) }}, - pg_size_pretty(pg_tablespace_size(ts.oid)) AS {{ conn|qtIdent(_('Size')) }} + pg_tablespace_size(ts.oid) AS {{ conn|qtIdent(_('Size')) }} FROM pg_catalog.pg_tablespace ts; {% endif %} diff --git a/web/pgadmin/misc/statistics/static/js/statistics.js b/web/pgadmin/misc/statistics/static/js/statistics.js index 1191b16..e1bbd55 100644 --- a/web/pgadmin/misc/statistics/static/js/statistics.js +++ b/web/pgadmin/misc/statistics/static/js/statistics.js @@ -1,7 +1,7 @@ define([ 'underscore', 'underscore.string', 'jquery', 'pgadmin.browser', 'backgrid', - 'alertify', 'wcdocker', 'pgadmin.backgrid', 'pgadmin.alertifyjs', 'pgadmin.browser.messages', -], function(_, S, $, pgBrowser, Backgrid, Alertify) { + 'alertify', 'sources/size_prettify' +], function(_, S, $, pgBrowser, Backgrid, Alertify, sizePrettify) { if (pgBrowser.NodeStatistics) return pgBrowser.NodeStatistics; @@ -12,6 +12,25 @@ define([ return pgBrowser.NodeStatistics; } + var SizeFormatter = Backgrid.SizeFormatter = function () {}; + _.extend(SizeFormatter.prototype, { + /** + Takes a raw value from a model and returns the human readable formatted + string for display. + + @member Backgrid.SizeFormatter + @param {*} rawData + @param {Backbone.Model} model Used for more complicated formatting + @return {*} + */ + fromRaw: function (rawData, model) { + return sizePrettify(rawData); + }, + toRaw: function (formattedData, model) { + return formattedData; + } + }); + var PGBooleanCell = Backgrid.Extension.SwitchCell.extend({ defaults: _.extend({}, Backgrid.Extension.SwitchCell.prototype.defaults) }), @@ -200,9 +219,9 @@ define([ if (res.data) { var data = res.data; if (node.hasCollectiveStatistics || data['rows'].length > 1) { - self.__createMultiLineStatistics.call(self, data); + self.__createMultiLineStatistics.call(self, data, node.statsPrettifyFields); } else { - self.__createSingleLineStatistics.call(self, data); + self.__createSingleLineStatistics.call(self, data, node.statsPrettifyFields); } if (self.grid) { @@ -294,24 +313,29 @@ define([ } }, - __createMultiLineStatistics: function(data) { + __createMultiLineStatistics: function(data, prettifyFields) { var rows = data['rows'], columns = data['columns']; this.columns = []; for (var idx in columns) { - var c = columns[idx]; - this.columns.push({ - editable: false, - name: c['name'], - cell: typeCellMapper[c['type_code']] || 'string' - }); + var rawColumn = columns[idx], + col = { + editable: false, + name: rawColumn['name'], + cell: typeCellMapper[rawColumn['type_code']] || 'string' + }; + if (_.indexOf(prettifyFields, rawColumn['name']) != -1) { + col['formatter'] = SizeFormatter + } + this.columns.push(col); + } this.collection.reset(rows); }, - __createSingleLineStatistics: function(data) { + __createSingleLineStatistics: function(data, prettifyFields) { var row = data['rows'][0], columns = data['columns'] res = []; @@ -322,7 +346,7 @@ define([ res.push({ 'statistics': name, // Check if row is undefined? - 'value': row && row[name] ? row[name] : null + 'value': row && row[name] ? ((_.indexOf(prettifyFields, name) != -1) ? sizePrettify(row[name]) : row[name]) : null }); } diff --git a/web/pgadmin/static/js/size_prettify.js b/web/pgadmin/static/js/size_prettify.js new file mode 100644 index 0000000..afb50b8 --- /dev/null +++ b/web/pgadmin/static/js/size_prettify.js @@ -0,0 +1,24 @@ +define([], + function () { + var sizePrettify = function (rawSize) { + var size = Math.abs(rawSize), + limit = 10 * 1024, + limit2 = limit - 1, + cnt = 0, + sizeUnits = ['bytes', 'kB', 'MB', 'GB', 'TB', 'PB']; + + if (size < limit) + return size + ' ' + sizeUnits[cnt]; // return in bytes format + else + { + do { + size = size / 1024; + cnt += 1; + } while (size > limit2); + + return Math.round(size) + ' ' + sizeUnits[cnt]; + } + }; + + return sizePrettify; +}); diff --git a/web/regression/javascript/size_prettify_spec.js b/web/regression/javascript/size_prettify_spec.js new file mode 100644 index 0000000..7d370fc --- /dev/null +++ b/web/regression/javascript/size_prettify_spec.js @@ -0,0 +1,68 @@ +////////////////////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2017, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////////////////// + +define(["sources/size_prettify"], function (sizePrettify) { + describe("sizePrettify", function () { + describe("when size is 0", function () { + it("returns 0 bytes", function () { + expect(sizePrettify(0)).toEqual("0 bytes"); + }); + }); + + describe("when size >= 10kB and size < 10 MB", function () { + it("returns size in kB", function () { + expect(sizePrettify(10240)).toEqual("10 kB"); + }); + + it("returns size in kB", function () { + expect(sizePrettify(99999)).toEqual("98 kB"); + }); + }); + + + describe("when size >= 10MB and size < 10 GB", function () { + it("returns size in MB", function () { + expect(sizePrettify(10485760)).toEqual("10 MB"); + }); + + it("returns size in MB", function () { + expect(sizePrettify(44040192)).toEqual("42 MB"); + }); + }); + + + describe("when size >= 10GB and size < 10 TB", function () { + it("returns size in GB", function () { + expect(sizePrettify(10737418240)).toEqual("10 GB"); + }); + + it("returns size in GB", function () { + expect(sizePrettify(10736344498176)).toEqual("9999 GB"); + }); + }); + + describe("when size >= 10TB and size < 10 PB", function () { + it("returns size in TB", function () { + expect(sizePrettify(10995116277760)).toEqual("10 TB"); + }); + + it("returns size in TB", function () { + expect(sizePrettify(29995116277760)).toEqual("27 TB"); + }); + }); + + describe("when size >= 10 PB", function () { + it("returns size in PB", function () { + expect(sizePrettify(11258999068426200)).toEqual("10 PB"); + }); + + }); + + }); +}); ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken @ 2017-05-08 10:29 Dave Page <[email protected]> parent: Khushboo Vashi <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Dave Page @ 2017-05-08 10:29 UTC (permalink / raw) To: Khushboo Vashi <[email protected]>; +Cc: Joao Pedro De Almeida Pereira <[email protected]>; pgadmin-hackers; Ashesh Vashi <[email protected]>; George Gelashvili <[email protected]> Thanks - patch applied. On Mon, May 8, 2017 at 8:08 AM, Khushboo Vashi < [email protected]> wrote: > Hi, > > Please find the attached updated patch. > > Thanks, > Khushboo > > On Fri, May 5, 2017 at 9:00 PM, Joao Pedro De Almeida Pereira < > [email protected]> 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 9999GB) >> - 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 < >> [email protected]> wrote: >> >>> Hi, >>> >>> Please find the attached updated patch. >>> >>> Thanks, >>> Khushboo >>> >>> On Fri, Apr 28, 2017 at 7:58 PM, Matthew Kleiman <[email protected]> >>> 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 < >>>> [email protected]> wrote: >>>> >>>>> Hi Sarah, >>>>> >>>>> On Thu, Apr 27, 2017 at 7:38 PM, Sarah McAlear <[email protected]> >>>>> 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 < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi Joao & Sarah, >>>>>>> >>>>>>> On Wed, Apr 26, 2017 at 8:46 PM, Joao Pedro De Almeida Pereira < >>>>>>> [email protected]> 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 < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> Hi Joao & Sarah, >>>>>>>>> >>>>>>>>> Thanks for reviewing the patch. >>>>>>>>> >>>>>>>>> On Tue, Apr 25, 2017 at 8:34 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. >>>>>>>>>> >>>>>>>>>> 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 <[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 >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Khushboo >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> Thanks, >>>>>>> Khushboo >>>>>>> >>>>>> >>>>>> >>>>> 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 ^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2017-05-08 10:29 UTC | newest] Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2017-04-25 09:18 [pgAdmin4][Patch]: Fixed RM #2315 : Sorting by size is broken Khushboo Vashi <[email protected]> 2017-04-25 11:58 ` Dave Page <[email protected]> 2017-04-25 15:04 ` Joao Pedro De Almeida Pereira <[email protected]> 2017-04-25 15:53 ` Dave Page <[email protected]> 2017-04-26 03:48 ` Khushboo Vashi <[email protected]> 2017-04-26 15:16 ` Joao Pedro De Almeida Pereira <[email protected]> 2017-04-27 05:50 ` Khushboo Vashi <[email protected]> 2017-04-27 14:08 ` Sarah McAlear <[email protected]> 2017-04-28 08:35 ` Khushboo Vashi <[email protected]> 2017-04-28 14:28 ` Matthew Kleiman <[email protected]> 2017-05-04 09:02 ` Khushboo Vashi <[email protected]> 2017-05-05 15:30 ` Joao Pedro De Almeida Pereira <[email protected]> 2017-05-05 15:43 ` Khushboo Vashi <[email protected]> 2017-05-08 07:08 ` Khushboo Vashi <[email protected]> 2017-05-08 10:29 ` Dave Page <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox