[GSoC] Query History Integration with updatable query resultsets and improvements

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[GSoC] Query History Integration with updatable query resultsets and improvements

Yosry Muhammad
Hi Dave and Hackers,

Please find attached a work-in-progress patch with the following modifications to the query history:

1- Queries generated by pgAdmin in Save Data operations are now recorded in query history. This includes transaction control commands such as 'COMMIT, ROLLBACK, SAVEPOINT, etc.'

2- Queries are now recorded in a correct (mogrified) form after parameters injection - as opposed to older versions < 4.10. They also appear with the correct start time (they used to appear with the start time of the previously executed query - a bug I found).

3- Save Data Queries are visually distinguishable in the query history.

4- Save Data Queries can be shown/hidden from history using a button.

5- Query Tool and View Data now share a common history - this makes more sense now as data changes can be done from both modes. I had a thought to remove the Query History from View Data mode entirely, but I thought it might be useful for some users? I don't know.

I am done with all the functionality code, what is left is the design of the toggling button/checkbox. For now, I am using an empty button at the end of the toolbar (next to download button) for experimental purposes.

Do you think a button or a checkbox is more appropriate? If a button, I would need a design to use. If a checkbox, I am going to need more help as I am not so good with the design parts. Where should it be placed ( I am thinking above the list of history entries) ? How should it be styled?

For now, I will start working on tests and documentation updates for this. Looking forward to your feedback and comments !

P.S I have done a lot of refactoring especially in save_data_changes.py, I would really appreciate it if someone reviewed these changes.

Thanks!
--
Yosry Muhammad Yosry

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.

query_history.patch (58K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [GSoC] Query History Integration with updatable query resultsets and improvements

Dave Page-7
Hi

Seems to work well :-). A few comments:

- Should we have an icon by the non-generated queries (the lightning flash) and the COMMITs etc (the commit/rollback icon as appropriate) as well? I think just having the icon for generated queries looks a little odd.

- PEP-8 checks failed:

pycodestyle --config=.pycodestyle web/
web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py:119: [E251] unexpected spaces around keyword / parameter equals
web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py:214: [E251] unexpected spaces around keyword / parameter equals
2       E251 unexpected spaces around keyword / parameter equals
2
make: *** [check-pep8] Error 1

Aditya; can you cast your eyes over it as well please?

Thanks!




On Tue, Aug 6, 2019 at 1:16 AM Yosry Muhammad <[hidden email]> wrote:
Hi Dave and Hackers,

Please find attached a work-in-progress patch with the following modifications to the query history:

1- Queries generated by pgAdmin in Save Data operations are now recorded in query history. This includes transaction control commands such as 'COMMIT, ROLLBACK, SAVEPOINT, etc.'

2- Queries are now recorded in a correct (mogrified) form after parameters injection - as opposed to older versions < 4.10. They also appear with the correct start time (they used to appear with the start time of the previously executed query - a bug I found).

3- Save Data Queries are visually distinguishable in the query history.

4- Save Data Queries can be shown/hidden from history using a button.

5- Query Tool and View Data now share a common history - this makes more sense now as data changes can be done from both modes. I had a thought to remove the Query History from View Data mode entirely, but I thought it might be useful for some users? I don't know.

I am done with all the functionality code, what is left is the design of the toggling button/checkbox. For now, I am using an empty button at the end of the toolbar (next to download button) for experimental purposes.

Do you think a button or a checkbox is more appropriate? If a button, I would need a design to use. If a checkbox, I am going to need more help as I am not so good with the design parts. Where should it be placed ( I am thinking above the list of history entries) ? How should it be styled?

For now, I will start working on tests and documentation updates for this. Looking forward to your feedback and comments !

P.S I have done a lot of refactoring especially in save_data_changes.py, I would really appreciate it if someone reviewed these changes.

Thanks!
--
Yosry Muhammad Yosry

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Reply | Threaded
Open this post in threaded view
|

Re: [GSoC] Query History Integration with updatable query resultsets and improvements

Yosry Muhammad
Hi,

On Tue, Aug 6, 2019, 6:01 PM Dave Page <[hidden email]> wrote:
Hi

Seems to work well :-). A few comments:

- Should we have an icon by the non-generated queries (the lightning flash) and the COMMITs etc (the commit/rollback icon as appropriate) as well? I think just having the icon for generated queries looks a little odd.

I am sorry I don't quite understand. Do you mean having an icon to show/hide user queries as well? I believe a checkbox is more appropriate anyway, maybe placed right above the history entries. I am going to need a design of the checkbox or styling guidlines though.


- PEP-8 checks failed:

pycodestyle --config=.pycodestyle web/
web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py:119: [E251] unexpected spaces around keyword / parameter equals
web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py:214: [E251] unexpected spaces around keyword / parameter equals
2       E251 unexpected spaces around keyword / parameter equals
2
make: *** [check-pep8] Error 1

It's okay I am still not done with the patch. 

Thanks .
Reply | Threaded
Open this post in threaded view
|

Re: [GSoC] Query History Integration with updatable query resultsets and improvements

Yosry Muhammad


Hi,

On Tue, Aug 6, 2019, 6:46 PM Yosry Muhammad <[hidden email]> wrote:
Hi,

On Tue, Aug 6, 2019, 6:01 PM Dave Page <[hidden email]> wrote:
Hi

Seems to work well :-). A few comments:

- Should we have an icon by the non-generated queries (the lightning flash) and the COMMITs etc (the commit/rollback icon as appropriate) as well? I think just having the icon for generated queries looks a little odd.

I am sorry I don't quite understand. Do you mean having an icon to show/hide user queries as well? I believe a checkbox is more appropriate anyway, maybe placed right above the history entries. I am going to need a design of the checkbox or styling guidlines though.

Another idea that came to my mind is to use a dropdown menu checkbox just next to the save data icon. Similar to auto-commot and auto-rollback checkboxs in the dropdown menu next to the execute (flash) icon. What do you think is better? This, or a checkbox right above the history entries?
Reply | Threaded
Open this post in threaded view
|

Re: [GSoC] Query History Integration with updatable query resultsets and improvements

Dave Page-7
In reply to this post by Yosry Muhammad
Hi

On Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <[hidden email]> wrote:
Hi,

On Tue, Aug 6, 2019, 6:01 PM Dave Page <[hidden email]> wrote:
Hi

Seems to work well :-). A few comments:

- Should we have an icon by the non-generated queries (the lightning flash) and the COMMITs etc (the commit/rollback icon as appropriate) as well? I think just having the icon for generated queries looks a little odd.

I am sorry I don't quite understand. Do you mean having an icon to show/hide user queries as well? I believe a checkbox is more appropriate anyway, maybe placed right above the history entries. I am going to need a design of the checkbox or styling guidlines though.

No, I mean put an icon by all queries to indicate the source of them. We'd probably want one for the EXPLAIN button too.

For showing/hiding generate queries, we should use one of our slider switches that we use elsewhere.

One other thing I found: The Copy to Query Editor button has gone missing, and the Copy button seems to have stopped working.
 


- PEP-8 checks failed:

pycodestyle --config=.pycodestyle web/
web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py:119: [E251] unexpected spaces around keyword / parameter equals
web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py:214: [E251] unexpected spaces around keyword / parameter equals
2       E251 unexpected spaces around keyword / parameter equals
2
make: *** [check-pep8] Error 1

It's okay I am still not done with the patch. 

Thanks .


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Reply | Threaded
Open this post in threaded view
|

Re: [GSoC] Query History Integration with updatable query resultsets and improvements

Yosry Muhammad
Hi,

On Wed, Aug 7, 2019, 10:31 AM Dave Page <[hidden email]> wrote:
Hi

On Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <[hidden email]> wrote:
Hi,

On Tue, Aug 6, 2019, 6:01 PM Dave Page <[hidden email]> wrote:
Hi

Seems to work well :-). A few comments:

- Should we have an icon by the non-generated queries (the lightning flash) and the COMMITs etc (the commit/rollback icon as appropriate) as well? I think just having the icon for generated queries looks a little odd.

I am sorry I don't quite understand. Do you mean having an icon to show/hide user queries as well? I believe a checkbox is more appropriate anyway, maybe placed right above the history entries. I am going to need a design of the checkbox or styling guidlines though.

No, I mean put an icon by all queries to indicate the source of them. We'd probably want one for the EXPLAIN button too.

Okay, got it.


For showing/hiding generate queries, we should use one of our slider switches that we use elsewhere.

One other thing I found: The Copy to Query Editor button has gone missing, and the Copy button seems to have stopped working.
 

I will check that, thanks for the feedback.
Reply | Threaded
Open this post in threaded view
|

Re: [GSoC] Query History Integration with updatable query resultsets and improvements

Aditya Toshniwal
Hi Yosry,

Users have also raised issue where they are not able to double click the cell and see the content. Now, if the result set is not updatable, double click does nothing. If the data is JSON, there is no way to see the content. It should show the editor without the Save button if the result set is not updatable.

On Wed, Aug 7, 2019 at 2:05 PM Yosry Muhammad <[hidden email]> wrote:
Hi,

On Wed, Aug 7, 2019, 10:31 AM Dave Page <[hidden email]> wrote:
Hi

On Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <[hidden email]> wrote:
Hi,

On Tue, Aug 6, 2019, 6:01 PM Dave Page <[hidden email]> wrote:
Hi

Seems to work well :-). A few comments:

- Should we have an icon by the non-generated queries (the lightning flash) and the COMMITs etc (the commit/rollback icon as appropriate) as well? I think just having the icon for generated queries looks a little odd.

I am sorry I don't quite understand. Do you mean having an icon to show/hide user queries as well? I believe a checkbox is more appropriate anyway, maybe placed right above the history entries. I am going to need a design of the checkbox or styling guidlines though.

No, I mean put an icon by all queries to indicate the source of them. We'd probably want one for the EXPLAIN button too.

Okay, got it.


For showing/hiding generate queries, we should use one of our slider switches that we use elsewhere.

One other thing I found: The Copy to Query Editor button has gone missing, and the Copy button seems to have stopped working.
 

I will check that, thanks for the feedback.


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
Reply | Threaded
Open this post in threaded view
|

Re: [GSoC] Query History Integration with updatable query resultsets and improvements

Yosry Muhammad
Will work on that too.

On Wed, Aug 7, 2019, 11:06 AM Aditya Toshniwal <[hidden email]> wrote:
Hi Yosry,

Users have also raised issue where they are not able to double click the cell and see the content. Now, if the result set is not updatable, double click does nothing. If the data is JSON, there is no way to see the content. It should show the editor without the Save button if the result set is not updatable.

On Wed, Aug 7, 2019 at 2:05 PM Yosry Muhammad <[hidden email]> wrote:
Hi,

On Wed, Aug 7, 2019, 10:31 AM Dave Page <[hidden email]> wrote:
Hi

On Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <[hidden email]> wrote:
Hi,

On Tue, Aug 6, 2019, 6:01 PM Dave Page <[hidden email]> wrote:
Hi

Seems to work well :-). A few comments:

- Should we have an icon by the non-generated queries (the lightning flash) and the COMMITs etc (the commit/rollback icon as appropriate) as well? I think just having the icon for generated queries looks a little odd.

I am sorry I don't quite understand. Do you mean having an icon to show/hide user queries as well? I believe a checkbox is more appropriate anyway, maybe placed right above the history entries. I am going to need a design of the checkbox or styling guidlines though.

No, I mean put an icon by all queries to indicate the source of them. We'd probably want one for the EXPLAIN button too.

Okay, got it.


For showing/hiding generate queries, we should use one of our slider switches that we use elsewhere.

One other thing I found: The Copy to Query Editor button has gone missing, and the Copy button seems to have stopped working.
 

I will check that, thanks for the feedback.


--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"