Please find separate patch files attached. I believe there isn't any
dependency between the patches.
#4525 and #4529 are duplicates - both are fixed by the patch.
On Wed, Jul 31, 2019 at 3:49 PM Dave Page <[email protected]> wrote:
> Hi
>
> On Wed, Jul 31, 2019 at 6:05 AM Aditya Toshniwal <
> [email protected]> wrote:
>
>> Hi Yosry,
>>
>> The changes to fix 4520 are in python side. I would suggest if you can
>> separate it out from this patch, so that we can ask the users(affected by
>> 4520) to just replace the python files in their installed pgAdmin4. This
>> fix will not be available till next release, but we can give the workaround
>> files. Most users are unable to upgrade psycopg2 on their systems.
>>
>
> Agreed. As a general rule, we try to keep to one fix per patch/commit as
> it makes it much easier to understand changes, especially after some time.
> Having a stated dependency between patches is fine if that's necessary.
>
> Can you split the changes up please?
>
> Thanks.
>
>
>>
>> On Tue, Jul 30, 2019 at 8:10 PM Yosry Muhammad <[email protected]>
>> wrote:
>>
>>> Hi,
>>>
>>> Please find attached a patch with the following bug fixes for the query
>>> tool:
>>> - View data not working correctly when the table/view does not contain
>>> any columns (#4492).
>>> - Query tool does not produce results when psycopg2 version is below 2.8
>>> (#4520).
>>> - Command tags and number of affected rows not shown in the query result
>>> messages (#4525 and #4529 - duplicates).
>>>
>>> Note that:
>>> #4524 and #4522 are feature requests for already existing features.
>>>
>>> Please review !
>>> Thanks
>>>
>>>
>>> --
>>> *Yosry Muhammad Yosry*
>>>
>>> Computer Engineering student,
>>> The Faculty of Engineering,
>>> Cairo University (2021).
>>> Class representative of CMP 2021.
>>> https://www.linkedin.com/in/yosrym93/
>>>
>>
>>
>> --
>> 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
>
--
*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 4fe4d7da..5091c11d 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -399,17 +399,6 @@ def poll(trans_id):
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
- else:
- result = None
-
if st:
if 'primary_keys' in session_obj:
primary_keys = session_obj['primary_keys']
@@ -496,6 +485,7 @@ def poll(trans_id):
col_info['pgadmin_alias'] = \
re.sub("[%()]+", "|", col_name)
session_obj['columns_info'] = columns
+
# status of async_fetchmany_2darray is True and result is none
# means nothing to fetch
if result and rows_affected > -1:
@@ -516,6 +506,17 @@ def poll(trans_id):
# restore it and update the session variable.
update_session_grid_transaction(trans_id, session_obj)
+ # 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 is not None and additional_messages is not None:
+ result = additional_messages + result
+ else:
+ result = result if result is not None \
+ else additional_messages
+
elif status == ASYNC_EXECUTION_ABORTED:
status = 'Cancel'
else:
diff --git a/web/pgadmin/tools/sqleditor/tests/test_view_data.py b/web/pgadmin/tools/sqleditor/tests/test_view_data.py
index dc96f9cd..b02e0e90 100644
--- a/web/pgadmin/tools/sqleditor/tests/test_view_data.py
+++ b/web/pgadmin/tools/sqleditor/tests/test_view_data.py
@@ -33,7 +33,7 @@ class TestViewData(BaseTestGenerator):
json_val json Not Null,
Constraint table_pk Primary Key(id)
);""",
- result_data=None,
+ result_data='SELECT 0',
rows_fetched_to=0
)
)
diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py b/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py
index 1745a990..8a4a0bd8 100644
--- a/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py
+++ b/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py
@@ -251,7 +251,7 @@ class TestSaveChangedData(BaseTestGenerator):
},
save_status=True,
check_sql='SELECT * FROM %s WHERE pk_col = 2',
- check_result=None
+ check_result='SELECT 0'
)),
]
diff --git a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
index 21734410..59f18776 100644
--- a/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/static/js/sqleditor.js
@@ -2464,8 +2464,8 @@ define('tools.querytool', [
var explain_data_array = [],
explain_data_json = null;
-
- if(self.colinfo[0].name == 'QUERY PLAN' && data.result
+ if(data.result && !_.isEmpty(self.colinfo)
+ && self.colinfo[0].name == 'QUERY PLAN' && !_.isEmpty(data.types)
&& data.types[0] && data.types[0].typname === 'json') {
/* json is sent as text, parse it */
explain_data_json = JSON.parse(data.result[0][0]);
diff --git a/docs/en_US/query_tool.rst b/docs/en_US/query_tool.rst
index 00c0ebf8..6116df61 100644
--- a/docs/en_US/query_tool.rst
+++ b/docs/en_US/query_tool.rst
@@ -134,6 +134,9 @@ A result set is updatable if:
* All the primary keys or OIDs of the table are explicitly selected.
* No columns are duplicated.
+The driver (psycopg2) version should be equal to or above 2.8 for modifying
+query resultsets to work.
+
An updatable result set can be modified just like in
:ref:`View/Edit Data <modifying-data-grid>` mode.
diff --git a/web/pgadmin/feature_tests/query_tool_journey_test.py b/web/pgadmin/feature_tests/query_tool_journey_test.py
index 27163b43..8208b18e 100644
--- a/web/pgadmin/feature_tests/query_tool_journey_test.py
+++ b/web/pgadmin/feature_tests/query_tool_journey_test.py
@@ -50,6 +50,9 @@ class QueryToolJourneyTest(BaseFeatureTest):
self.page.add_server(self.server)
+ driver_version = test_utils.get_driver_version()
+ self.driver_version = float('.'.join(driver_version.split('.')[:2]))
+
def runTest(self):
self._navigate_to_query_tool()
self._execute_query(
@@ -190,6 +193,9 @@ class QueryToolJourneyTest(BaseFeatureTest):
self._assert_clickable(query_we_need_to_scroll_to)
def _test_updatable_resultset(self):
+ if self.driver_version < 2.8:
+ return
+
self.page.click_tab("Query Editor")
# Insert data into test table
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index c344f5d7..4fe4d7da 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -424,16 +424,16 @@ def poll(trans_id):
# If trans_obj is a QueryToolCommand then check for updatable
# resultsets and primary keys
if isinstance(trans_obj, QueryToolCommand):
- trans_obj.check_updatable_results_pkeys_oids()
- pk_names, primary_keys = trans_obj.get_primary_keys()
- session_obj['has_oids'] = trans_obj.has_oids()
- # Update command_obj in session obj
- session_obj['command_obj'] = pickle.dumps(
- trans_obj, -1)
- # 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
+ if trans_obj.check_updatable_results_pkeys_oids():
+ pk_names, primary_keys = trans_obj.get_primary_keys()
+ session_obj['has_oids'] = trans_obj.has_oids()
+ # Update command_obj in session obj
+ session_obj['command_obj'] = pickle.dumps(
+ trans_obj, -1)
+ # 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
if 'has_oids' in session_obj:
has_oids = session_obj['has_oids']
diff --git a/web/pgadmin/tools/sqleditor/command.py b/web/pgadmin/tools/sqleditor/command.py
index 258f6c60..01815f8c 100644
--- a/web/pgadmin/tools/sqleditor/command.py
+++ b/web/pgadmin/tools/sqleditor/command.py
@@ -896,6 +896,13 @@ class QueryToolCommand(BaseCommand, FetchedRowTracker):
manager = driver.connection_manager(self.sid)
conn = manager.connection(did=self.did, conn_id=self.conn_id)
+ # Get the driver version as a float
+ driver_version = float('.'.join(driver.Version().split('.')[:2]))
+
+ # Checking for updatable resultsets uses features in psycopg 2.8
+ if driver_version < 2.8:
+ return False
+
# Get the path to the sql templates
sql_path = 'sqleditor/sql/#{0}#'.format(manager.version)
@@ -918,6 +925,7 @@ class QueryToolCommand(BaseCommand, FetchedRowTracker):
self.__set_updatable_results_attrs(sql_path=sql_path,
table_oid=table_oid,
conn=conn)
+ return self.is_updatable_resultset
def save(self,
changed_data,
diff --git a/web/pgadmin/tools/sqleditor/tests/test_is_query_resultset_updatable.py b/web/pgadmin/tools/sqleditor/utils/tests/test_is_query_resultset_updatable.py
similarity index 93%
rename from web/pgadmin/tools/sqleditor/tests/test_is_query_resultset_updatable.py
rename to web/pgadmin/tools/sqleditor/utils/tests/test_is_query_resultset_updatable.py
index 5c249c5b..6a3ea38e 100644
--- a/web/pgadmin/tools/sqleditor/tests/test_is_query_resultset_updatable.py
+++ b/web/pgadmin/tools/sqleditor/utils/tests/test_is_query_resultset_updatable.py
@@ -15,7 +15,7 @@ from pgadmin.browser.server_groups.servers.databases.tests import utils as \
from pgadmin.utils.route import BaseTestGenerator
from regression import parent_node_dict
from regression.python_test_utils import test_utils as utils
-from .execute_query_utils import execute_query
+from pgadmin.tools.sqleditor.tests.execute_query_utils import execute_query
class TestQueryUpdatableResultset(BaseTestGenerator):
@@ -120,6 +120,7 @@ class TestQueryUpdatableResultset(BaseTestGenerator):
def _initialize_database_connection(self):
database_info = parent_node_dict["database"][-1]
+ self.db_name = database_info["db_name"]
self.server_id = database_info["server_id"]
self.server_version = parent_node_dict["schema"][-1]["server_version"]
@@ -128,6 +129,12 @@ class TestQueryUpdatableResultset(BaseTestGenerator):
self.skipTest('Tables with OIDs are not supported starting '
'PostgreSQL 12')
+ driver_version = utils.get_driver_version()
+ driver_version = float('.'.join(driver_version.split('.')[:2]))
+
+ if driver_version < 2.8:
+ self.skipTest('Updatable resultsets require pyscopg 2.8 or later')
+
self.db_id = database_info["db_id"]
db_con = database_utils.connect_database(self,
utils.SERVER_GROUP,
@@ -172,10 +179,5 @@ class TestQueryUpdatableResultset(BaseTestGenerator):
else:
create_sql += ';'
- is_success, _ = \
- execute_query(tester=self.tester,
- query=create_sql,
- start_query_tool_url=self.start_query_tool_url,
- poll_url=self.poll_url)
- self.assertEquals(is_success, True)
+ utils.create_table_with_query(self.server, self.db_name, create_sql)
return test_table_name
diff --git a/web/pgadmin/tools/sqleditor/tests/test_save_changed_data.py b/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py
similarity index 96%
rename from web/pgadmin/tools/sqleditor/tests/test_save_changed_data.py
rename to web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py
index 29e9cce3..1745a990 100644
--- a/web/pgadmin/tools/sqleditor/tests/test_save_changed_data.py
+++ b/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py
@@ -15,11 +15,11 @@ from pgadmin.browser.server_groups.servers.databases.tests import utils as \
from pgadmin.utils.route import BaseTestGenerator
from regression import parent_node_dict
from regression.python_test_utils import test_utils as utils
-from .execute_query_utils import execute_query
+from pgadmin.tools.sqleditor.tests.execute_query_utils import execute_query
class TestSaveChangedData(BaseTestGenerator):
- """ This class tests saving data changes in the grid to the database """
+ """ This class tests saving data changes to updatable query resultsets """
scenarios = [
('When inserting new valid row', dict(
save_payload={
@@ -304,6 +304,7 @@ class TestSaveChangedData(BaseTestGenerator):
def _initialize_database_connection(self):
database_info = parent_node_dict["database"][-1]
+ self.db_name = database_info["db_name"]
self.server_id = database_info["server_id"]
self.db_id = database_info["db_id"]
@@ -311,6 +312,13 @@ class TestSaveChangedData(BaseTestGenerator):
utils.SERVER_GROUP,
self.server_id,
self.db_id)
+
+ driver_version = utils.get_driver_version()
+ driver_version = float('.'.join(driver_version.split('.')[:2]))
+
+ if driver_version < 2.8:
+ self.skipTest('Updatable resultsets require pyscopg 2.8 or later')
+
if not db_con["info"] == "Database connected.":
raise Exception("Could not connect to the database.")
@@ -347,9 +355,5 @@ class TestSaveChangedData(BaseTestGenerator):
self.test_table_name,
self.test_table_name)
self.select_sql = 'SELECT * FROM %s;' % self.test_table_name
- is_success, _ = \
- execute_query(tester=self.tester,
- query=create_sql,
- start_query_tool_url=self.start_query_tool_url,
- poll_url=self.poll_url)
- self.assertEquals(is_success, True)
+
+ utils.create_table_with_query(self.server, self.db_name, create_sql)
diff --git a/web/regression/python_test_utils/test_utils.py b/web/regression/python_test_utils/test_utils.py
index 4413705e..855f1243 100644
--- a/web/regression/python_test_utils/test_utils.py
+++ b/web/regression/python_test_utils/test_utils.py
@@ -1042,3 +1042,8 @@ def get_watcher_dialogue_status(self):
else:
break
return status
+
+
+def get_driver_version():
+ version = getattr(psycopg2, '__version__', None)
+ return version
