pgAdmin 4 commit: Ensure strings are properly encoded in the Query Hist

[Dave Page]

Ensure strings are properly encoded in the Query History. Fixes #4349

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=3b69f92d70f4ec9a2f1624238d92c2af7a792f2d
Author: Aditya Toshniwal 

Modified Files
--
docs/en_US/release_notes_4_9.rst   |  4 +-
.../xss_checks_panels_and_query_tool_test.py   | 56 ++
.../js/sqleditor/history/query_history_details.js  |  8 ++--
.../js/sqleditor/history/query_history_entries.js  |  3 +-
.../javascript/history/query_history_spec.js   |  4 +-
5 files changed, 65 insertions(+), 10 deletions(-)

Re: [pgAdmin4][Patch]: Feature #4202 Implement new framework to test Reverse Engineering SQL

[Dave Page]

Hi

On Thu, Jun 13, 2019 at 12:52 PM Akshay Joshi 
wrote:

> Hi Hackers
>
> I have implemented the new test framework to test the Reverse Engineering
> SQL. I have integrated it as a part of API/Regression test suite. It will
> work when we run all the test cases or module wise test case.
>
> *How it works*: Attached patch contains the generic framework to read all
> the JSON files from the *tests->version based (example 9.6_plus, 10_plus
> or default) folder. *Run all the test scenarios present in the JSON file
> in sequential order.
>
> Format of the JSON file is mentioned in
> "web/pgadmin/browser/server_groups/servers/databases/casts/tests/default/test.json"
>
> For expected SQL we will have following two options:
>
>- Provide the expected sql in scenario itself as parameter *"expected_sql"
>: ""*.
>- Create a output file with any name in the same directory where the
>JSON file resides and specify the parameter "*expected_sql_file":
>""*
>
> Attached patch contains both the above mentioned examples.
>
> Please review it.
>

Nice!

A few comments:

- The scenario name should be "Reverse Engineered SQL Test Cases"
- After the scenario name is output, can we output a \n so the next line
isn't appended to the name?
- How do we run only the re_sql tests? I tried the obvious ways
(e.g. python runtests.py --pkg
regression.re_sql.tests.test_resql.ReverseEngineeringSQLTestCase) but got
errors. Please add an example to web/regression/README.
- Once we have a way to run these tests only, please add a "make
check-resql" target to the Makefile.
- Can the expected output be formatted in the JSON such that it doesn't use
\n, but uses regular line breaks? That would make it easier to copy/paste.

Thanks.

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin][RM4329] Initialization error when parameterised functions debugged in parallel in two separate tabs

[Aditya Toshniwal]

Hi Hackers,

I have missed a line while implementing this. Attached is the patch to fix
that.
Although it has not caused any trouble, but still it should be changed.
Kindly review.

On Mon, Jun 10, 2019 at 7:28 PM Dave Page  wrote:

> Thanks, patch applied.
>
> On Mon, Jun 10, 2019 at 1:58 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Attached is the updated patch with fixes.
>>
>> On Mon, Jun 10, 2019 at 12:58 PM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Aditya
>>>
>>> Following are the review comments:
>>>
>>>- "Set breakpoint" option not working, when click it throws an error.
>>>
>>> Fixed.
>>
>>>
>>>- Create an empty function and try to debug that. It should show
>>>proper error message.
>>>
>>> This seems to be a bug in the debugger itself. I'll raise a bug with
>> simulation steps if it is. But, not sure where to raise.
>>
>>>
>>>- Got the following backend error when closing the connection,
>>>please fix this:
>>>   - File
>>>   "E:\Projects\pgadmin4\web\pgadmin\tools\debugger\__init__.py", line 
>>> 2053,
>>>   in close_debugger_session
>>>   conn_id=dbg_obj['exe_conn_id'])
>>>
>>> Fixed.
>>
>>>
>>> On Fri, Jun 7, 2019 at 12:21 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Hackers,

 Attached is the patch for debugger improvements. The changes include:
 1) Change the way debug info is stored in session. Removed redundant
 session related code in debugger code. All the session related handling
 done at one place.
 2) Fixed a bug where debugger was not opening for EPAS package function.
 3) If a package is defined without body and we try to debug a
 proc/func, the debugger opened a blank window. Changes made so that it will
 throw error as "XYZ is not defined in package body."

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

>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB India | Pune
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


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


RM4329_v2.part2.patch
Description: Binary data

pgAdmin 4 commit: Add minor change missed in previous commit. Fixes #43

[Dave Page]

Add minor change missed in previous commit. Fixes #4329

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=2894c6cd14a16016ed74bfa028dfa09f95917bf9
Author: Aditya Toshniwal 

Modified Files
--
web/pgadmin/tools/debugger/__init__.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Re: [pgAdmin][RM4329] Initialization error when parameterised functions debugged in parallel in two separate tabs

[Dave Page]

Thanks, applied.

On Fri, Jun 14, 2019 at 10:38 AM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> I have missed a line while implementing this. Attached is the patch to fix
> that.
> Although it has not caused any trouble, but still it should be changed.
> Kindly review.
>
> On Mon, Jun 10, 2019 at 7:28 PM Dave Page  wrote:
>
>> Thanks, patch applied.
>>
>> On Mon, Jun 10, 2019 at 1:58 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached is the updated patch with fixes.
>>>
>>> On Mon, Jun 10, 2019 at 12:58 PM Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Aditya

 Following are the review comments:

- "Set breakpoint" option not working, when click it throws an
error.

 Fixed.
>>>

- Create an empty function and try to debug that. It should show
proper error message.

 This seems to be a bug in the debugger itself. I'll raise a bug with
>>> simulation steps if it is. But, not sure where to raise.
>>>

- Got the following backend error when closing the connection,
please fix this:
   - File
   "E:\Projects\pgadmin4\web\pgadmin\tools\debugger\__init__.py", line 
 2053,
   in close_debugger_session
   conn_id=dbg_obj['exe_conn_id'])

 Fixed.
>>>

 On Fri, Jun 7, 2019 at 12:21 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> Attached is the patch for debugger improvements. The changes include:
> 1) Change the way debug info is stored in session. Removed redundant
> session related code in debugger code. All the session related handling
> done at one place.
> 2) Fixed a bug where debugger was not opening for EPAS package
> function.
> 3) If a package is defined without body and we try to debug a
> proc/func, the debugger opened a blank window. Changes made so that it 
> will
> throw error as "XYZ is not defined in package body."
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>


 --
 *Thanks & Regards*
 *Akshay Joshi*

 *Sr. Software Architect*
 *EnterpriseDB Software India Private Limited*
 *Mobile: +91 976-788-8246*

>>>
>>>
>>> --
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> Software Engineer | EnterpriseDB India | Pune
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][Patch]: Feature #4202 Implement new framework to test Reverse Engineering SQL

[Akshay Joshi]

On Fri, Jun 14, 2019 at 1:59 PM Dave Page  wrote:

> Hi
>
> On Thu, Jun 13, 2019 at 12:52 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Hackers
>>
>> I have implemented the new test framework to test the Reverse Engineering
>> SQL. I have integrated it as a part of API/Regression test suite. It will
>> work when we run all the test cases or module wise test case.
>>
>> *How it works*: Attached patch contains the generic framework to read
>> all the JSON files from the *tests->version based (example 9.6_plus,
>> 10_plus or default) folder. *Run all the test scenarios present in the
>> JSON file in sequential order.
>>
>> Format of the JSON file is mentioned in
>> "web/pgadmin/browser/server_groups/servers/databases/casts/tests/default/test.json"
>>
>> For expected SQL we will have following two options:
>>
>>- Provide the expected sql in scenario itself as parameter *"expected_sql"
>>: ""*.
>>- Create a output file with any name in the same directory where the
>>JSON file resides and specify the parameter "*expected_sql_file":
>>""*
>>
>> Attached patch contains both the above mentioned examples.
>>
>> Please review it.
>>
>
> Nice!
>
> A few comments:
>
> - The scenario name should be "Reverse Engineered SQL Test Cases"
> - After the scenario name is output, can we output a \n so the next line
> isn't appended to the name?
>

   Will fix the above.

> - How do we run only the re_sql tests? I tried the obvious ways
> (e.g. python runtests.py --pkg
> regression.re_sql.tests.test_resql.ReverseEngineeringSQLTestCase) but got
> errors. Please add an example to web/regression/README.
>

   It is not a pgadmin module and we have kept it in regression folder, so
will have to change the existing code. I have tried but facing issues when
run only  "regression.re_sql.tests", will continue working on this.


> - Once we have a way to run these tests only, please add a "make
> check-resql" target to the Makefile.
> - Can the expected output be formatted in the JSON such that it doesn't
> use \n, but uses regular line breaks? That would make it easier to
> copy/paste.
>

I have tried that during implementation, but JSON does not allow
line-breaks.

>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*

usability issues

[Phil Sevenants]

I'm new to pgAdmin and giving it a chance to show it's utility to me. A
response into the following bug/user issues will help keep me as a user:

   1. Loss of  to cycle through windows of queries (I develop
   bunches of queries). I'll have to try having each query in a separate
   browser giving me  switching which I see is an option
   2. Disappearing curser frustrates constantly -- just isolated at least
   one way to replicate and will share on demand.
   3. File Save dialog box is frustrating with long path names at the end
   of the file name text input -- wouldn't it be more useful to see the end of
   the path/filename string? I think so.

Thanks for reading.

-- 
Phil Sevenants

[GSoC][Patch] Automatic Mode Detection V1

[Yosry Muhammad]

Dear all,

This is my first patch of my GSoC project, query tool automatic mode
detection.

In this patch, the initial (basic) version of the project is implemented.
In this version, query resultsets are updatable if and only if:
- All the columns belong to a single table
- No duplicate columns are available
- All the primary keys of the table are available

Inserts, updates and deletes work automatically when the resultset is
updatable.

The 'save' button in the query tool works automatically to save the changes
in the resultset if the query is the updatable, and saves the query to a
file otherwise. The 'save as' button stays as is.

I will work on improving and adding features to this version throughout my
work during the summer according to what has the highest priorities
(supporting duplicate columns or columns produced by functions or
aggregations as read-only columns in the results seems like a good next
move).

Please give me your feedback of the changes I made, and any hints or
comments that will improve my code in any aspect.

I also have a couple of questions,
- Should the save button in the query tool work the way I am using it now?
or should there be a new dedicated button for saving the query to a file?

- What documentations or unit tests should I write? any guidelines here
would be appreciated.

Thanks a lot!


-- 
*Yosry Muhammad Yosry*

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.
https://www.linkedin.com/in/yosrym93/
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 16f7133f..22050451 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -384,6 +384,8 @@ def poll(trans_id):
 rset = None
 has_oids = False
 oids = None
+additional_messages = None
+notifies = None
 
 # Check the transaction and connection status
 status, error_msg, conn, trans_obj, session_obj = \
@@ -422,6 +424,22 @@ def poll(trans_id):
 
 st, result = conn.async_fetchmany_2darray(ON_DEMAND_RECORD_COUNT)
 
+# There may be additional messages even if result is present
+# eg: Function can provide result as well as RAISE messages
+messages = conn.messages()
+if messages:
+additional_messages = ''.join(messages)
+notifies = conn.get_notifies()
+
+# Procedure/Function output may comes in the form of Notices
+# from the database server, so we need to append those outputs
+# with the original result.
+if result is None:
+result = conn.status_message()
+if (result != 'SELECT 1' or result != 'SELECT 0') and \
+result is not None and additional_messages:
+result = additional_messages + result
+
 if st:
 if 'primary_keys' in session_obj:
 primary_keys = session_obj['primary_keys']
@@ -438,10 +456,22 @@ def poll(trans_id):
 )
 session_obj['client_primary_key'] = client_primary_key
 
-if columns_info is not None:
+# If trans_obj is a QueryToolCommand then check for updatable
+# resultsets and primary keys
+if isinstance(trans_obj, QueryToolCommand):
+trans_obj.check_for_updatable_resultset_and_primary_keys()
+pk_names, primary_keys = trans_obj.get_primary_keys()
+# If primary_keys exist, add them to the session_obj to
+# allow for saving any changes to the data
+if primary_keys is not None:
+session_obj['primary_keys'] = primary_keys
 
-command_obj = pickle.loads(session_obj['command_obj'])
-if hasattr(command_obj, 'obj_id'):
+if columns_info is not None:
+# If it is a QueryToolCommand that has obj_id attribute
+# then it should also be editable
+if hasattr(trans_obj, 'obj_id') and \
+(not isinstance(trans_obj, QueryToolCommand) or
+ trans_obj.can_edit()):
 # Get the template path for the column
 template_path = 'columns/sql/#{0}#'.format(
 conn.manager.version
@@ -449,7 +479,7 @@ def poll(trans_id):
 
 SQL = render_template(
 "/".join([template_path, 'nodes.sql']),
-tid=command_obj.obj_id,
+tid=trans_obj.obj_id,
 has_oids=True
 )
 # rows with attribute not_null
@@ -520,26 +550,8 @@ def poll(trans_id):
 status = 'NotConnected'
 result = error_