Thread: [GSoC] Query History Integration with updatable query resultsets and improvements
[GSoC] Query History Integration with updatable query resultsets and improvements
From
Yosry Muhammad
Date:
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.
Attachment
Re: [GSoC] Query History Integration with updatable query resultsetsand improvements
From
Dave Page
Date:
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
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 <yosrym93@gmail.com> 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 YosryComputer 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
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: [GSoC] Query History Integration with updatable query resultsetsand improvements
From
Yosry Muhammad
Date:
Hi,
On Tue, Aug 6, 2019, 6:01 PM Dave Page <dpage@pgadmin.org> wrote:
HiSeems 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 .
Re: [GSoC] Query History Integration with updatable query resultsetsand improvements
From
Yosry Muhammad
Date:
Hi,
On Tue, Aug 6, 2019, 6:46 PM Yosry Muhammad <yosrym93@gmail.com> wrote:
Hi,On Tue, Aug 6, 2019, 6:01 PM Dave Page <dpage@pgadmin.org> wrote:HiSeems 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?
Re: [GSoC] Query History Integration with updatable query resultsetsand improvements
From
Dave Page
Date:
Hi
On Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <yosrym93@gmail.com> wrote:
Hi,On Tue, Aug 6, 2019, 6:01 PM Dave Page <dpage@pgadmin.org> wrote:HiSeems 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 1It'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
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: [GSoC] Query History Integration with updatable query resultsetsand improvements
From
Yosry Muhammad
Date:
Hi,
On Wed, Aug 7, 2019, 10:31 AM Dave Page <dpage@pgadmin.org> wrote:
HiOn Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <yosrym93@gmail.com> wrote:Hi,On Tue, Aug 6, 2019, 6:01 PM Dave Page <dpage@pgadmin.org> wrote:HiSeems 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.
Re: [GSoC] Query History Integration with updatable query resultsetsand improvements
From
Aditya Toshniwal
Date:
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 <yosrym93@gmail.com> wrote:
Hi,On Wed, Aug 7, 2019, 10:31 AM Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <yosrym93@gmail.com> wrote:Hi,On Tue, Aug 6, 2019, 6:01 PM Dave Page <dpage@pgadmin.org> wrote:HiSeems 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"
Re: [GSoC] Query History Integration with updatable query resultsetsand improvements
From
Yosry Muhammad
Date:
Will work on that too.
On Wed, Aug 7, 2019, 11:06 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> 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 <yosrym93@gmail.com> wrote:Hi,On Wed, Aug 7, 2019, 10:31 AM Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, Aug 6, 2019 at 5:46 PM Yosry Muhammad <yosrym93@gmail.com> wrote:Hi,On Tue, Aug 6, 2019, 6:01 PM Dave Page <dpage@pgadmin.org> wrote:HiSeems 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 ToshniwalSoftware Engineer | EnterpriseDB India | Pune"Don't Complain about Heat, Plant a TREE"