public inbox for [email protected]help / color / mirror / Atom feed
PATCH: To fix the performance of delete operation (pgAdmin4) 4+ messages / 2 participants [nested] [flat]
* PATCH: To fix the performance of delete operation (pgAdmin4) @ 2016-09-15 10:08 Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Murtuza Zabuawala @ 2016-09-15 10:08 UTC (permalink / raw) To: pgadmin-hackers Hi, PFA patch to fix the issue of slow performance while deleting rows from grid. RM#1696 *Issue:* As per logic implemented for Backgrid, we were deleting individual rows, means If user selects 100 rows to delete then we were rendering sql template 100 times to generate sql for each individual rows and then execute all 100 delete statements one by one due to which we were getting poor performance. Please review. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers Attachments: [application/octet-stream] RM_1696.patch (5.3K, 3-RM_1696.patch) download | inline diff: diff --git a/web/pgadmin/tools/sqleditor/command.py b/web/pgadmin/tools/sqleditor/command.py index 4b8db6e..b5b8bde 100644 --- a/web/pgadmin/tools/sqleditor/command.py +++ b/web/pgadmin/tools/sqleditor/command.py @@ -465,14 +465,27 @@ class TableCommand(GridCommand): # For deleted rows elif of_type == 'deleted': + is_first = True + rows_to_delete = [] + keys = None + no_of_keys = None for each_row in changed_data[of_type]: - data = changed_data[of_type][each_row] - sql = render_template("/".join([self.sql_path, 'delete.sql']), - data=data, - object_name=self.object_name, - nsp_name=self.nsp_name) - list_of_sql.append(sql) - list_of_rowid.append(data) + rows_to_delete.append(changed_data[of_type][each_row]) + # Fetch the keys for SQL generation + if is_first: + # We need to covert dict_keys to normal list in Python3 + # In Python2, it's already a list + keys = list(changed_data[of_type][each_row].keys()) + no_of_keys = len(keys) + is_first = False + + sql = render_template("/".join([self.sql_path, 'delete.sql']), + data=rows_to_delete, + primary_key_lables=keys, + no_of_keys=no_of_keys, + object_name=self.object_name, + nsp_name=self.nsp_name) + list_of_sql.append(sql) for i, sql in enumerate(list_of_sql): if sql: diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js index d81e79d..cde28a7 100644 --- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js @@ -2069,17 +2069,25 @@ define( data: JSON.stringify(self.data_store), success: function(res) { var grid = self.slickgrid, - data = grid.getData();; + data = grid.getData(); if (res.data.status) { // Remove deleted rows from client as well if(is_deleted) { var rows = grid.getSelectedRows(); - // Reverse the deletion from array - // so that when we remove it does not affect index - rows = rows.sort().reverse(); - rows.forEach(function(idx) { - data.splice(idx, 1); - }); + /* In JavaScript sorting by default is lexical, + * To make sorting numerical we need to pass function + * After that we will Reverse the order of sorted array + * so that when we remove it does not affect array index + */ + if(data.length == rows.length) { + // This means all rows are selected, clear all data + data = []; + } else { + rows = rows.sort(function(a,b){return a - b}).reverse(); + rows.forEach(function(idx) { + data.splice(idx, 1); + }); + } grid.setData(data, true); grid.setSelectedRows([]); } diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql index e5dbc9b..3a552b7 100644 --- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql +++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql @@ -1,4 +1,26 @@ -{# Delete the row with primary keys #} DELETE FROM {{ conn|qtIdent(nsp_name, object_name) }} WHERE -{% for pk_key in data %} -{% if not loop.first %} AND {% endif %}{{ conn|qtIdent(pk_key) }} = {{ data[pk_key]|qtLiteral }}{% endfor %}; \ No newline at end of file +{### If there is only one primary key ###} +{% if no_of_keys == 1 %} +{{ conn|qtIdent(primary_key_lables[0]) }} IN +{### If there are multiple primary keys ###} +{% elif no_of_keys > 1 %} +({% for each_label in primary_key_lables %} +{% if not loop.first %}, {% endif %}{{ conn|qtIdent(each_label) }} +{% endfor %}) IN +{% endif %} +( +{### Rows to delete ###} +{% for obj in data %} +{% if not loop.first %}, {% endif %} +{% if no_of_keys == 1 %} +{{ obj[primary_key_lables[0]]|qtLiteral }} +{% elif no_of_keys > 1 %} +{### Here we need to make tuple for each row ###} +( +{% for each_label in primary_key_lables %} +{% if not loop.first %}, {% endif %}{{ obj[each_label]|qtLiteral }} +{% endfor %} +) +{% endif %} +{% endfor %} +); \ No newline at end of file ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: PATCH: To fix the performance of delete operation (pgAdmin4) @ 2016-09-16 10:01 Dave Page <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Dave Page @ 2016-09-16 10:01 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers Can you rebase this please? Thanks. On Thu, Sep 15, 2016 at 11:08 AM, Murtuza Zabuawala <[email protected]> wrote: > Hi, > > PFA patch to fix the issue of slow performance while deleting rows from > grid. > RM#1696 > > Issue: > As per logic implemented for Backgrid, we were deleting individual rows, > means If user selects 100 rows to delete then we were rendering sql template > 100 times to generate sql for each individual rows and then execute all 100 > delete statements one by one due to which we were getting poor performance. > > Please review. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > -- > 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 -- 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] 4+ messages in thread
* Re: PATCH: To fix the performance of delete operation (pgAdmin4) @ 2016-09-16 11:34 Murtuza Zabuawala <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Murtuza Zabuawala @ 2016-09-16 11:34 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: pgadmin-hackers Hi Dave, Please find updated patch. Regards, Murtuza -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company On Fri, Sep 16, 2016 at 3:31 PM, Dave Page <[email protected]> wrote: > Can you rebase this please? > > Thanks. > > On Thu, Sep 15, 2016 at 11:08 AM, Murtuza Zabuawala > <[email protected]> wrote: > > Hi, > > > > PFA patch to fix the issue of slow performance while deleting rows from > > grid. > > RM#1696 > > > > Issue: > > As per logic implemented for Backgrid, we were deleting individual rows, > > means If user selects 100 rows to delete then we were rendering sql > template > > 100 times to generate sql for each individual rows and then execute all > 100 > > delete statements one by one due to which we were getting poor > performance. > > > > Please review. > > > > -- > > Regards, > > Murtuza Zabuawala > > EnterpriseDB: http://www.enterprisedb.com > > The Enterprise PostgreSQL Company > > > > > > -- > > 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 > -- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers Attachments: [application/octet-stream] RM_1696_v2.patch (5.1K, 3-RM_1696_v2.patch) download | inline diff: diff --git a/web/pgadmin/tools/sqleditor/command.py b/web/pgadmin/tools/sqleditor/command.py index 4b8db6e..b5b8bde 100644 --- a/web/pgadmin/tools/sqleditor/command.py +++ b/web/pgadmin/tools/sqleditor/command.py @@ -465,14 +465,27 @@ class TableCommand(GridCommand): # For deleted rows elif of_type == 'deleted': + is_first = True + rows_to_delete = [] + keys = None + no_of_keys = None for each_row in changed_data[of_type]: - data = changed_data[of_type][each_row] - sql = render_template("/".join([self.sql_path, 'delete.sql']), - data=data, - object_name=self.object_name, - nsp_name=self.nsp_name) - list_of_sql.append(sql) - list_of_rowid.append(data) + rows_to_delete.append(changed_data[of_type][each_row]) + # Fetch the keys for SQL generation + if is_first: + # We need to covert dict_keys to normal list in Python3 + # In Python2, it's already a list + keys = list(changed_data[of_type][each_row].keys()) + no_of_keys = len(keys) + is_first = False + + sql = render_template("/".join([self.sql_path, 'delete.sql']), + data=rows_to_delete, + primary_key_lables=keys, + no_of_keys=no_of_keys, + object_name=self.object_name, + nsp_name=self.nsp_name) + list_of_sql.append(sql) for i, sql in enumerate(list_of_sql): if sql: diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js index f306520..74502c7 100644 --- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js @@ -2149,12 +2149,20 @@ define( // Remove deleted rows from client as well if(is_deleted) { var rows = grid.getSelectedRows(); - // Reverse the deletion from array - // so that when we remove it does not affect index - rows = rows.sort().reverse(); - rows.forEach(function(idx) { - data.splice(idx, 1); - }); + /* In JavaScript sorting by default is lexical, + * To make sorting numerical we need to pass function + * After that we will Reverse the order of sorted array + * so that when we remove it does not affect array index + */ + if(data.length == rows.length) { + // This means all the rows are selected, clear all data + data = []; + } else { + rows = rows.sort(function(a,b){return a - b}).reverse(); + rows.forEach(function(idx) { + data.splice(idx, 1); + }); + } grid.setData(data, true); grid.setSelectedRows([]); } diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql index e5dbc9b..3a552b7 100644 --- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql +++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/9.1_plus/delete.sql @@ -1,4 +1,26 @@ -{# Delete the row with primary keys #} DELETE FROM {{ conn|qtIdent(nsp_name, object_name) }} WHERE -{% for pk_key in data %} -{% if not loop.first %} AND {% endif %}{{ conn|qtIdent(pk_key) }} = {{ data[pk_key]|qtLiteral }}{% endfor %}; \ No newline at end of file +{### If there is only one primary key ###} +{% if no_of_keys == 1 %} +{{ conn|qtIdent(primary_key_lables[0]) }} IN +{### If there are multiple primary keys ###} +{% elif no_of_keys > 1 %} +({% for each_label in primary_key_lables %} +{% if not loop.first %}, {% endif %}{{ conn|qtIdent(each_label) }} +{% endfor %}) IN +{% endif %} +( +{### Rows to delete ###} +{% for obj in data %} +{% if not loop.first %}, {% endif %} +{% if no_of_keys == 1 %} +{{ obj[primary_key_lables[0]]|qtLiteral }} +{% elif no_of_keys > 1 %} +{### Here we need to make tuple for each row ###} +( +{% for each_label in primary_key_lables %} +{% if not loop.first %}, {% endif %}{{ obj[each_label]|qtLiteral }} +{% endfor %} +) +{% endif %} +{% endfor %} +); \ No newline at end of file ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: PATCH: To fix the performance of delete operation (pgAdmin4) @ 2016-09-16 15:48 Dave Page <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Dave Page @ 2016-09-16 15:48 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers Thanks - committed with a couple of changes: - I re-formatted the SQL - s/lables/labels Awesome speedup BTW - deleting 25K rows went from >10 minutes to virtually instant. Good work. On Fri, Sep 16, 2016 at 12:34 PM, Murtuza Zabuawala <[email protected]> wrote: > Hi Dave, > > Please find updated patch. > > > Regards, > Murtuza > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > On Fri, Sep 16, 2016 at 3:31 PM, Dave Page <[email protected]> wrote: >> >> Can you rebase this please? >> >> Thanks. >> >> On Thu, Sep 15, 2016 at 11:08 AM, Murtuza Zabuawala >> <[email protected]> wrote: >> > Hi, >> > >> > PFA patch to fix the issue of slow performance while deleting rows from >> > grid. >> > RM#1696 >> > >> > Issue: >> > As per logic implemented for Backgrid, we were deleting individual rows, >> > means If user selects 100 rows to delete then we were rendering sql >> > template >> > 100 times to generate sql for each individual rows and then execute all >> > 100 >> > delete statements one by one due to which we were getting poor >> > performance. >> > >> > Please review. >> > >> > -- >> > Regards, >> > Murtuza Zabuawala >> > EnterpriseDB: http://www.enterprisedb.com >> > The Enterprise PostgreSQL Company >> > >> > >> > -- >> > 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 -- 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] 4+ messages in thread
end of thread, other threads:[~2016-09-16 15:48 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2016-09-15 10:08 PATCH: To fix the performance of delete operation (pgAdmin4) Murtuza Zabuawala <[email protected]> 2016-09-16 10:01 ` Dave Page <[email protected]> 2016-09-16 11:34 ` Murtuza Zabuawala <[email protected]> 2016-09-16 15:48 ` 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