public inbox for [email protected]
help / color / mirror / Atom feedFrom: Khushboo Vashi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch] : RE-SQL tests for Collation node
Date: Fri, 12 Jul 2019 16:24:00 +0530
Message-ID: <CAFOhELdMcQYy9KtsFr9mJ-h9XbsWNYHTtjUCYHDv5jGLebdrsw@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoyU9pdK6LNv+zQD14SkbFcwU0tHiCXND6kN8btMuqv_mg@mail.gmail.com>
References: <CAFOhELcvULp8TPxOGw_C7eQ6dmq6ddo=HGifCSW0EpHqD+d64g@mail.gmail.com>
<CA+OCxoyU9pdK6LNv+zQD14SkbFcwU0tHiCXND6kN8btMuqv_mg@mail.gmail.com>
Hi Dave,
Please find the attached updated patch.
On Fri, Jul 12, 2019 at 4:07 PM Dave Page <[email protected]> wrote:
> Hi
>
> On Fri, Jul 12, 2019 at 10:46 AM Khushboo Vashi <
> [email protected]> wrote:
>
>> Hi,
>>
>> Please find the attached patch for the RE-SQL tests for collation node.
>> This patch also includes the *modified SQL tests* as well as fixes for
>> the RE-SQL in the collation node which I found while implementing this.
>>
>> To add the modified SQL tests, 2 optional parameters are introduced in
>> the JSON file, i.e.
>> *msql_endpoint* and *expected_msql_file.*
>> These parameters need to be included in the Alter scenarios.
>>
>> I have modified the RE-SQL framework to support modified SQL.
>>
>
> This fails on EPAS 9.4:
>
> ... 2019-07-12 11:35:09,672: ERROR flask.app: Failed to execute query
> (execute_scalar) for the server #5 - DB:test_db_18bdb (Query-id: 9091709):
> Error Message:ERROR: role "postgres" does not exist
>
> Create Collation... FAIL
> Traceback (most recent call last):
> File
> "/Users/dpage/git/pgadmin4/web/regression/re_sql/tests/test_resql.py", line
> 205, in execute_test_case
> self.assertEquals(response.status_code, 200)
> File
> "/opt/local/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py",
> line 1338, in deprecated_func
> return original_func(*args, **kwargs)
> File
> "/opt/local/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py",
> line 839, in assertEqual
> assertion_func(first, second, msg=msg)
> File
> "/opt/local/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/case.py",
> line 832, in _baseAssertEqual
> raise self.failureException(msg)
> AssertionError: 500 != 200
> ERROR
>
> Fixed.
> I guess we need to create a role first?
>
> Also, please keep your error messages consistent with the others, e.g.
> "... FAIL" instead of " ..................FAIL".
>
> Removed.
> Thanks!
>
>
Thanks,
Khushboo
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
Attachments:
[application/octet-stream] resql_collation_v1.patch (11.9K, 3-resql_collation_v1.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/__init__.py
index 2aef28b7..dd606457 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/__init__.py
@@ -511,24 +511,22 @@ class CollationView(PGChildNodeView):
SQL = render_template("/".join([self.template_path,
'get_name.sql']),
scid=scid, coid=coid)
- status, name = self.conn.execute_scalar(SQL)
+ status, res = self.conn.execute_dict(SQL)
if not status:
- return internal_server_error(errormsg=name)
-
- if name is None:
- return make_json_response(
- success=0,
- errormsg=gettext(
- 'Error: Object not found.'
- ),
- info=gettext(
- 'The specified collation could not be found.\n'
- )
- )
+ return internal_server_error(errormsg=res)
+
+ if len(res['rows']) == 0:
+ return gone(gettext(
+ "Could not find the collation object in the database."
+ ))
+
+ data = res['rows'][0]
SQL = render_template("/".join([self.template_path,
'delete.sql']),
- name=name, cascade=cascade,
+ name=data['name'],
+ nspname=data['schema'],
+ cascade=cascade,
conn=self.conn)
status, res = self.conn.execute_scalar(SQL)
if not status:
@@ -700,7 +698,8 @@ class CollationView(PGChildNodeView):
sql_header += render_template("/".join([self.template_path,
'delete.sql']),
- name=data['name'])
+ name=data['name'],
+ nspname=data['schema'])
SQL = sql_header + '\n\n' + SQL.strip('\n')
return ajax_response(response=SQL)
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/delete.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/delete.sql
index 5b1f8ce0..bbca449f 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/delete.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/delete.sql
@@ -1 +1 @@
-DROP COLLATION {{name}}{% if cascade%} CASCADE{% endif %};
+DROP COLLATION {{ conn|qtIdent(nspname, name) }}{% if cascade%} CASCADE{% endif %};
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/get_name.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/get_name.sql
index f5dda005..b0d95ed9 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/get_name.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/templates/collations/sql/default/get_name.sql
@@ -1,4 +1,4 @@
-SELECT concat(quote_ident(nspname), '.', quote_ident(collname)) AS name
+SELECT nspname AS schema, collname AS name
FROM pg_collation c, pg_namespace n
WHERE c.collnamespace = n.oid AND
n.oid = {{ scid }}::oid AND
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/alter_collation.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/alter_collation.sql
new file mode 100644
index 00000000..c1f9dace
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/alter_collation.sql
@@ -0,0 +1,12 @@
+-- Collation: Cl1_$%{}[]()&*^!@"'`\/#a;
+
+-- DROP COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#a";
+
+CREATE COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#a"
+ (LC_COLLATE = 'C', LC_CTYPE = 'C');
+
+ALTER COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#a"
+ OWNER TO <OWNER>;
+
+COMMENT ON COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#a"
+ IS 'Description for alter';
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/create_collation.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/create_collation.sql
new file mode 100644
index 00000000..9d44cddc
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/create_collation.sql
@@ -0,0 +1,12 @@
+-- Collation: Cl1_$%{}[]()&*^!@"'`\/#;
+
+-- DROP COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#";
+
+CREATE COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#"
+ (LC_COLLATE = 'C', LC_CTYPE = 'C');
+
+ALTER COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#"
+ OWNER TO <OWNER>;
+
+COMMENT ON COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#"
+ IS 'Description';
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/msql_collation.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/msql_collation.sql
new file mode 100644
index 00000000..f58616dc
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/msql_collation.sql
@@ -0,0 +1,5 @@
+COMMENT ON COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#"
+ IS 'Description for alter';
+
+ALTER COLLATION testschema."Cl1_$%{}[]()&*^!@""'`\/#"
+ RENAME TO "Cl1_$%{}[]()&*^!@""'`\/#a";
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/test.json b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/test.json
new file mode 100644
index 00000000..9b40cb99
--- /dev/null
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/collations/tests/default/test.json
@@ -0,0 +1,37 @@
+{
+ "scenarios": [
+ {
+ "type": "create",
+ "name": "Create Collation",
+ "endpoint": "NODE-collation.obj",
+ "sql_endpoint": "NODE-collation.sql_id",
+ "data": {
+ "name": "Cl1_$%{}[]()&*^!@\"'`\\/#",
+ "schema": "testschema",
+ "copy_collation": "pg_catalog.\"C\"",
+ "description": "Description"
+ },
+ "expected_sql_file": "create_collation.sql"
+ }, {
+ "type": "alter",
+ "name": "Alter Collation",
+ "endpoint": "NODE-collation.obj_id",
+ "sql_endpoint": "NODE-collation.sql_id",
+ "msql_endpoint": "NODE-collation.msql_id",
+ "data": {
+ "name": "Cl1_$%{}[]()&*^!@\"'`\\/#a",
+ "schema": "testschema",
+ "description": "Description for alter"
+ },
+ "expected_sql_file": "alter_collation.sql",
+ "expected_msql_file": "msql_collation.sql"
+ }, {
+ "type": "delete",
+ "name": "Drop Collation",
+ "endpoint": "NODE-collation.delete_id",
+ "data": {
+ "name": "Cl1_$%{}[]()&*^!@\"'`\\/#a"
+ }
+ }
+ ]
+}
diff --git a/web/regression/re_sql/tests/test_resql.py b/web/regression/re_sql/tests/test_resql.py
index 7d626c45..41442b1b 100644
--- a/web/regression/re_sql/tests/test_resql.py
+++ b/web/regression/re_sql/tests/test_resql.py
@@ -9,6 +9,7 @@
from __future__ import print_function
import json
import os
+import urllib
import traceback
from flask import url_for
import regression
@@ -223,6 +224,19 @@ class ReverseEngineeredSQLTestCases(BaseTestGenerator):
continue
elif 'type' in scenario and scenario['type'] == 'alter':
# Get the url and create the specific node.
+
+ # If msql_endpoint exists then validate the modified sql
+ if 'msql_endpoint' in scenario\
+ and scenario['msql_endpoint']:
+ if not self.check_msql(scenario, object_id):
+ print_msg = scenario['name']
+ if 'expected_msql_file' in scenario:
+ print_msg += " Expected MSQL File:" + scenario[
+ 'expected_msql_file']
+ print_msg = print_msg + "... FAIL"
+ print(print_msg)
+ continue
+
alter_url = self.get_url(scenario['endpoint'], object_id)
response = self.tester.put(alter_url,
data=json.dumps(scenario['data']),
@@ -295,6 +309,66 @@ class ReverseEngineeredSQLTestCases(BaseTestGenerator):
return False, None
+ def check_msql(self, scenario, object_id):
+ """
+ This function is used to check the modified SQL.
+ :param scenario:
+ :param object_id:
+ :return:
+ """
+
+ msql_url = self.get_url(scenario['msql_endpoint'],
+ object_id)
+
+ params = urllib.parse.urlencode(scenario['data'])
+ url = msql_url + "?%s" % params
+ response = self.tester.get(url,
+ follow_redirects=True)
+ try:
+ self.assertEquals(response.status_code, 200)
+ except Exception as e:
+ self.final_test_status = False
+ print(scenario['name'] + "... FAIL")
+ traceback.print_exc()
+
+ resp = json.loads(response.data)
+ resp_sql = resp['data']
+
+ # Remove first and last double quotes
+ if resp_sql.startswith('"') and resp_sql.endswith('"'):
+ resp_sql = resp_sql[1:-1]
+ resp_sql = resp_sql.rstrip()
+
+ # Check if expected sql is given in JSON file or path of the output
+ # file is given
+ if 'expected_msql_file' in scenario:
+ output_file = os.path.join(self.test_folder,
+ scenario['expected_msql_file'])
+
+ if os.path.exists(output_file):
+ fp = open(output_file, "r")
+ # Used rstrip to remove trailing \n
+ sql = fp.read().rstrip()
+ # Replace place holder <owner> with the current username
+ # used to connect to the database
+ if 'username' in self.server:
+ sql = sql.replace(self.JSON_PLACEHOLDERS['owner'],
+ self.server['username'])
+ try:
+ self.assertEquals(sql, resp_sql)
+ except Exception as e:
+ self.final_test_status = False
+ traceback.print_exc()
+ return False
+ else:
+ try:
+ self.assertFalse("Expected SQL File not found")
+ except Exception as e:
+ self.final_test_status = False
+ traceback.print_exc()
+ return False
+ return True
+
def check_re_sql(self, scenario, object_id):
"""
This function is used to get the reverse engineering SQL.
@@ -302,11 +376,14 @@ class ReverseEngineeredSQLTestCases(BaseTestGenerator):
:param object_id:
:return:
"""
+
sql_url = self.get_url(scenario['sql_endpoint'], object_id)
response = self.tester.get(sql_url)
+
try:
self.assertEquals(response.status_code, 200)
except Exception as e:
+
self.final_test_status = False
traceback.print_exc()
return False
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin4][Patch] : RE-SQL tests for Collation node
In-Reply-To: <CAFOhELdMcQYy9KtsFr9mJ-h9XbsWNYHTtjUCYHDv5jGLebdrsw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox