public inbox for [email protected]
help / color / mirror / Atom feedpgAdmin 4 commit: Cleanup handling of default/null values when data edi
5+ messages / 3 participants
[nested] [flat]
* pgAdmin 4 commit: Cleanup handling of default/null values when data edi
@ 2017-05-27 18:51 Dave Page <[email protected]>
2017-05-28 17:25 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Dave Page @ 2017-05-27 18:51 UTC (permalink / raw)
To: pgadmin-hackers
Cleanup handling of default/null values when data editting. FIxes #2400
Branch
------
master
Details
-------
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=1f26953504a963d2c2223c51f9da29db4d48...
Author: Surinder Kumar <[email protected]>
Modified Files
--------------
.../static/js/slickgrid/slick.pgadmin.editors.js | 58 ++++-
web/pgadmin/tools/sqleditor/command.py | 5 +
.../sqleditor/templates/sqleditor/js/sqleditor.js | 254 ++++++++++++---------
3 files changed, 205 insertions(+), 112 deletions(-)
--
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] 5+ messages in thread
* Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi
2017-05-27 18:51 pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
@ 2017-05-28 17:25 ` Harshal Dhumal <[email protected]>
2017-05-28 23:49 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Harshal Dhumal @ 2017-05-28 17:25 UTC (permalink / raw)
To: Surinder Kumar <[email protected]>; Dave Page <[email protected]>; +Cc: pgadmin-hackers
Hi,
This commit has some performance issues with row paste functionality.
For 2K copied rows with 3 columns (2 integer and one null column) it took
near about 10 seconds to complete paste operation. And entire application
becomes unresponsive for those 10 seconds.
This is mainly because for each single pasted row entire grid is
re-rendered ( is what I see in code).
Ideally grid should be re-rendered only once after all rows are provided to
grid.
below code snippet from _paste_rows function
_.each(copied_rows, function(row) {
var new_row = arr_to_object(row);
new_row.is_row_copied = true;
row = new_row;
self.temp_new_rows.push(count);
grid.onAddNewRow.notify(
{item: new_row, column: self.columns[0] , grid:grid}
)
grid.setSelectedRows([]);
count++;
});
The statement
grid.onAddNewRow.notify(
{item: new_row, column: self.columns[0] , grid:grid}
)
causes grid to re-render (as we listener on onAddNewRow event where we
re-render the grid)
--
*Harshal Dhumal*
*Sr. Software Engineer*
EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Sun, May 28, 2017 at 12:21 AM, Dave Page <[email protected]> wrote:
> Cleanup handling of default/null values when data editting. FIxes #2400
>
> Branch
> ------
> master
>
> Details
> -------
> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=
> 1f26953504a963d2c2223c51f9da29db4d48594b
> Author: Surinder Kumar <[email protected]>
>
> Modified Files
> --------------
> .../static/js/slickgrid/slick.pgadmin.editors.js | 58 ++++-
> web/pgadmin/tools/sqleditor/command.py | 5 +
> .../sqleditor/templates/sqleditor/js/sqleditor.js | 254
> ++++++++++++---------
> 3 files changed, 205 insertions(+), 112 deletions(-)
>
>
> --
> 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] 5+ messages in thread
* Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi
2017-05-27 18:51 pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
2017-05-28 17:25 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Harshal Dhumal <[email protected]>
@ 2017-05-28 23:49 ` Dave Page <[email protected]>
2017-05-29 06:08 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Surinder Kumar <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Dave Page @ 2017-05-28 23:49 UTC (permalink / raw)
To: Harshal Dhumal <[email protected]>; +Cc: Surinder Kumar <[email protected]>; pgadmin-hackers
On Sun, May 28, 2017 at 1:25 PM, Harshal Dhumal
<[email protected]> wrote:
> Hi,
>
> This commit has some performance issues with row paste functionality.
> For 2K copied rows with 3 columns (2 integer and one null column) it took
> near about 10 seconds to complete paste operation. And entire application
> becomes unresponsive for those 10 seconds.
>
> This is mainly because for each single pasted row entire grid is re-rendered
> ( is what I see in code).
> Ideally grid should be re-rendered only once after all rows are provided to
> grid.
>
> below code snippet from _paste_rows function
>
> _.each(copied_rows, function(row) {
> var new_row = arr_to_object(row);
> new_row.is_row_copied = true;
> row = new_row;
> self.temp_new_rows.push(count);
> grid.onAddNewRow.notify(
> {item: new_row, column: self.columns[0] , grid:grid}
> )
> grid.setSelectedRows([]);
> count++;
> });
>
> The statement
>
> grid.onAddNewRow.notify(
> {item: new_row, column: self.columns[0] , grid:grid}
> )
>
> causes grid to re-render (as we listener on onAddNewRow event where we
> re-render the grid)
Copying that number of rows is an extreme case of course, but still...
Is there an alternative way to batch notify?
--
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] 5+ messages in thread
* Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi
2017-05-27 18:51 pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
2017-05-28 17:25 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Harshal Dhumal <[email protected]>
2017-05-28 23:49 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
@ 2017-05-29 06:08 ` Surinder Kumar <[email protected]>
2017-05-30 15:22 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Surinder Kumar @ 2017-05-29 06:08 UTC (permalink / raw)
To: Dave Page <[email protected]>; +Cc: Harshal Dhumal <[email protected]>; pgadmin-hackers
Hi Dave,
The grid was being re-rendered after add new row and copy/paste to add a
blank row in the end of grid, but in case of copy/paste batch operation it
should run once, so that code is moved out of addNewRow(...) and put into a
function grid.addBlankRow() and called separately for copy/paste after
batch operation is completed.
Now copy/paste with 10k records took 2 seconds.
Please find attached patch.
On Mon, May 29, 2017 at 5:19 AM, Dave Page <[email protected]> wrote:
> On Sun, May 28, 2017 at 1:25 PM, Harshal Dhumal
> <[email protected]> wrote:
> > Hi,
> >
> > This commit has some performance issues with row paste functionality.
> > For 2K copied rows with 3 columns (2 integer and one null column) it took
> > near about 10 seconds to complete paste operation. And entire application
> > becomes unresponsive for those 10 seconds.
> >
> > This is mainly because for each single pasted row entire grid is
> re-rendered
> > ( is what I see in code).
> > Ideally grid should be re-rendered only once after all rows are provided
> to
> > grid.
> >
> > below code snippet from _paste_rows function
> >
> > _.each(copied_rows, function(row) {
> > var new_row = arr_to_object(row);
> > new_row.is_row_copied = true;
> > row = new_row;
> > self.temp_new_rows.push(count);
> > grid.onAddNewRow.notify(
> > {item: new_row, column: self.columns[0] , grid:grid}
> > )
> > grid.setSelectedRows([]);
> > count++;
> > });
> >
> > The statement
> >
> > grid.onAddNewRow.notify(
> > {item: new_row, column: self.columns[0] , grid:grid}
> > )
> >
> > causes grid to re-render (as we listener on onAddNewRow event where we
> > re-render the grid)
>
> Copying that number of rows is an extreme case of course, but still...
> Is there an alternative way to batch notify?
>
> --
> 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] render_grid_after_batch_copy_paste_rows.patch (2.0K, 3-render_grid_after_batch_copy_paste_rows.patch)
download | inline diff:
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 7d51193..597c436 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -910,6 +910,14 @@ define(
$("#btn-save").prop('disabled', false);
}.bind(editor_data));
+ grid.addBlankRow = function() {
+ // Add a blank row in the end of grid
+ this.setData(this.getData(), true);
+ this.updateRowCount();
+ this.invalidateAllRows();
+ this.render();
+ };
+
// Listener function which will be called when user adds new rows
grid.onAddNewRow.subscribe(function (e, args) {
// self.handler.data_store.added will holds all the newly added rows/data
@@ -941,10 +949,9 @@ define(
grid.render();
// Add a blank row after add row
- grid.setData(new_collection, true);
- grid.updateRowCount();
- grid.invalidateAllRows();
- grid.render();
+ if (!args.is_copy_row) {
+ grid.addBlankRow();
+ }
// Enable save button
$("#btn-save").prop('disabled', false);
@@ -3171,11 +3178,17 @@ define(
row = new_row;
self.temp_new_rows.push(count);
grid.onAddNewRow.notify(
- {item: new_row, column: self.columns[0] , grid:grid}
+ { item: new_row,
+ column: self.columns[0],
+ grid: grid,
+ is_copy_row: true
+ }
)
- grid.setSelectedRows([]);
count++;
});
+ // Add a blank row after copy/paste row
+ grid.addBlankRow();
+ grid.setSelectedRows([]);
}
},
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi
2017-05-27 18:51 pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
2017-05-28 17:25 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Harshal Dhumal <[email protected]>
2017-05-28 23:49 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
2017-05-29 06:08 ` Re: pgAdmin 4 commit: Cleanup handling of default/null values when data edi Surinder Kumar <[email protected]>
@ 2017-05-30 15:22 ` Dave Page <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Dave Page @ 2017-05-30 15:22 UTC (permalink / raw)
To: Surinder Kumar <[email protected]>; +Cc: Harshal Dhumal <[email protected]>; pgadmin-hackers
Thanks, applied.
On Mon, May 29, 2017 at 7:08 AM, Surinder Kumar
<[email protected]> wrote:
> Hi Dave,
>
> The grid was being re-rendered after add new row and copy/paste to add a
> blank row in the end of grid, but in case of copy/paste batch operation it
> should run once, so that code is moved out of addNewRow(...) and put into a
> function grid.addBlankRow() and called separately for copy/paste after batch
> operation is completed.
>
> Now copy/paste with 10k records took 2 seconds.
>
> Please find attached patch.
>
>
> On Mon, May 29, 2017 at 5:19 AM, Dave Page <[email protected]> wrote:
>>
>> On Sun, May 28, 2017 at 1:25 PM, Harshal Dhumal
>> <[email protected]> wrote:
>> > Hi,
>> >
>> > This commit has some performance issues with row paste functionality.
>> > For 2K copied rows with 3 columns (2 integer and one null column) it
>> > took
>> > near about 10 seconds to complete paste operation. And entire
>> > application
>> > becomes unresponsive for those 10 seconds.
>> >
>> > This is mainly because for each single pasted row entire grid is
>> > re-rendered
>> > ( is what I see in code).
>> > Ideally grid should be re-rendered only once after all rows are provided
>> > to
>> > grid.
>> >
>> > below code snippet from _paste_rows function
>> >
>> > _.each(copied_rows, function(row) {
>> > var new_row = arr_to_object(row);
>> > new_row.is_row_copied = true;
>> > row = new_row;
>> > self.temp_new_rows.push(count);
>> > grid.onAddNewRow.notify(
>> > {item: new_row, column: self.columns[0] , grid:grid}
>> > )
>> > grid.setSelectedRows([]);
>> > count++;
>> > });
>> >
>> > The statement
>> >
>> > grid.onAddNewRow.notify(
>> > {item: new_row, column: self.columns[0] , grid:grid}
>> > )
>> >
>> > causes grid to re-render (as we listener on onAddNewRow event where we
>> > re-render the grid)
>>
>> Copying that number of rows is an extreme case of course, but still...
>> Is there an alternative way to batch notify?
>>
>> --
>> 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] 5+ messages in thread
end of thread, other threads:[~2017-05-30 15:22 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-05-27 18:51 pgAdmin 4 commit: Cleanup handling of default/null values when data edi Dave Page <[email protected]>
2017-05-28 17:25 ` Harshal Dhumal <[email protected]>
2017-05-28 23:49 ` Dave Page <[email protected]>
2017-05-29 06:08 ` Surinder Kumar <[email protected]>
2017-05-30 15:22 ` 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