Subject: [pgAdmin4][Patch]: RM #3135 - [Web based] Syntax error displayed when user try to insert data on table where primray key is in captial letters and table contains OIDS

From: Khushboo Vashi <[email-masked]>
Date: Thu, 1 Mar 2018 12:57:55 +0530

Hi,

Please find the attached patch to fix RM # 3135 - [Web based] Syntax error
displayed when user try to insert data on table where primray key is in
captial letters and table contains OIDS

Thanks,
Khushboo
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
index a4ed872..7e3f8a0 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
@@ -6,5 +6,5 @@ INSERT INTO {{ conn|qtIdent(nsp_name, object_name) }} (
 {% for col in data_to_be_saved %}
 {% if not loop.first %}, {% endif %}%({{ col }})s::{{ data_type[col] }}{% endfor %}
 )
-{% if pk_names %} returning {{pk_names}}{% endif %}
+{% if pk_names and not has_oids %} returning {{pk_names}}{% endif %}
 {% if has_oids %} returning oid{% endif %};

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Thu, 01 Mar 2018 14:36:21 +0000

Hello Kushboo,

Can we add some Unit test to ensure this does not happen again?

Thanks
Joao

---

From: Khushboo Vashi <[email-masked]>
Date: Tue, 6 Mar 2018 18:03:15 +0530

Hi Joao,

Thanks for reviewing.

On Thu, Mar 1, 2018 at 8:06 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

> Please find the attached patch with the unit tests.

Khushboo

>> Please find the attached patch to fix RM # 3135 - [Web based] Syntax
>> error displayed when user try to insert data on table where primray key is
>> in captial letters and table contains OIDS
>>
>> Thanks,
>> Khushboo
>>
>
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
index a4ed872..7e3f8a0 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/insert.sql
@@ -6,5 +6,5 @@ INSERT INTO {{ conn|qtIdent(nsp_name, object_name) }} (
 {% for col in data_to_be_saved %}
 {% if not loop.first %}, {% endif %}%({{ col }})s::{{ data_type[col] }}{% endfor %}
 )
-{% if pk_names %} returning {{pk_names}}{% endif %}
+{% if pk_names and not has_oids %} returning {{pk_names}}{% endif %}
 {% if has_oids %} returning oid{% endif %};
diff --git a/web/pgadmin/tools/sqleditor/tests/test_view_data_templates.py b/web/pgadmin/tools/sqleditor/tests/test_view_data_templates.py
new file mode 100644
index 0000000..026f70b
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/tests/test_view_data_templates.py
@@ -0,0 +1,213 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+import os
+import re
+
+from flask import Flask, render_template
+from jinja2 import FileSystemLoader
+
+from pgadmin import VersionedTemplateLoader
+from pgadmin.utils.route import BaseTestGenerator
+from pgadmin.utils.driver import get_driver
+from config import PG_DEFAULT_DRIVER
+try:
+    from collections import OrderedDict
+except ImportError:
+    from ordereddict import OrderedDict
+
+
+class TestViewDataTemplates(BaseTestGenerator):
+    """
+    This class validates the template query for
+    inserting and selecting table data.
+    """
+    data_to_be_saved = OrderedDict()
+    data_to_be_saved['id'] = '1'
+    data_to_be_saved['text'] = 'just test'
+    scenarios = [
+        (
+            'When inserting and selecting table data with only PK',
+            dict(
+                insert_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'insert.sql'),
+                insert_parameters=dict(
+                    data_to_be_saved=data_to_be_saved,
+                    primary_keys=None,
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    data_type={'text': 'text', 'id': 'integer'},
+                    pk_names='id',
+                    has_oids=False
+                ),
+                insert_expected_return_value='INSERT INTO'
+                                             ' test_schema.test_table'
+                                             ' (id, text) VALUES'
+                                             ' (%(id)s::integer, '
+                                             '%(text)s::text)'
+                                             ' returning id;',
+                select_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'select.sql'),
+                select_parameters=dict(
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    primary_keys=OrderedDict([('id', 'int4')]),
+                    has_oids=False
+                ),
+                select_expected_return_value='SELECT * FROM '
+                                             'test_schema.test_table'
+                                             'WHERE id = %(id)s;'
+            )),
+        (
+            'When inserting and selecting table data with multiple PK',
+            dict(
+                insert_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'insert.sql'),
+                insert_parameters=dict(
+                    data_to_be_saved=data_to_be_saved,
+                    primary_keys=None,
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    data_type={'text': 'text', 'id': 'integer'},
+                    pk_names='id, text',
+                    has_oids=False
+                ),
+                insert_expected_return_value='INSERT INTO'
+                                             ' test_schema.test_table'
+                                             ' (id, text)'
+                                             ' VALUES (%(id)s::integer,'
+                                             ' %(text)s::text)'
+                                             ' returning id, text;',
+                select_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'select.sql'),
+                select_parameters=dict(
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    primary_keys=OrderedDict([('id', 'int4'),
+                                              ('text', 'text')]),
+                    has_oids=False
+                ),
+                select_expected_return_value='SELECT * FROM'
+                                             ' test_schema.test_table'
+                                             'WHERE id = %(id)s AND'
+                                             ' text = %(text)s;'
+            )),
+        (
+            'When inserting and selecting table data with PK and OID',
+            dict(
+                insert_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'insert.sql'),
+                insert_parameters=dict(
+                    data_to_be_saved=data_to_be_saved,
+                    primary_keys=None,
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    data_type={'text': 'text', 'id': 'integer'},
+                    pk_names='id',
+                    has_oids=True
+                ),
+                insert_expected_return_value='INSERT INTO'
+                                             ' test_schema.test_table'
+                                             ' (id, text) VALUES'
+                                             ' (%(id)s::integer, '
+                                             '%(text)s::text) '
+                                             'returning oid;',
+                select_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'select.sql'),
+                select_parameters=dict(
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    primary_keys=OrderedDict([('id', 'int4')]),
+                    has_oids=True
+                ),
+                select_expected_return_value='SELECT oid, * '
+                                             'FROM test_schema.test_table'
+                                             'WHERE oid = %(oid)s;'
+            )),
+        (
+            'When inserting and selecting table data with only OID',
+            dict(
+                insert_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'insert.sql'),
+                insert_parameters=dict(
+                    data_to_be_saved=data_to_be_saved,
+                    primary_keys=None,
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    data_type={'text': 'text', 'id': 'integer'},
+                    pk_names=None,
+                    has_oids=True
+                ),
+                insert_expected_return_value='INSERT INTO'
+                                             ' test_schema.test_table'
+                                             ' (id, text) VALUES'
+                                             ' (%(id)s::integer,'
+                                             ' %(text)s::text)'
+                                             ' returning oid;',
+                select_template_path=os.path.join('sqleditor',
+                                                  'sql',
+                                                  'default',
+                                                  'select.sql'),
+                select_parameters=dict(
+                    object_name='test_table',
+                    nsp_name='test_schema',
+                    primary_keys=None,
+                    has_oids=True
+                ),
+                select_expected_return_value='SELECT oid, * FROM'
+                                             ' test_schema.test_table'
+                                             'WHERE oid = %(oid)s;'
+            )
+        )
+    ]
+
+    def setUp(self):
+        self.loader = VersionedTemplateLoader(FakeApp())
+
+    def runTest(self):
+        with FakeApp().app_context():
+            result = render_template(self.insert_template_path,
+                                     **self.insert_parameters)
+            self.assertEqual(
+                re.sub(' +', ' ', str(result).replace("\n", "")),
+                re.sub(' +', ' ',
+                       self.insert_expected_return_value.replace("\n", "")))
+
+            result = render_template(self.select_template_path,
+                                     **self.select_parameters)
+            self.assertEqual(
+                re.sub(' +', ' ', str(result).replace("\n", "")),
+                re.sub(' +', ' ',
+                       self.select_expected_return_value.replace("\n", "")))
+
+
+class FakeApp(Flask):
+    def __init__(self):
+        super(FakeApp, self).__init__("")
+        driver = get_driver(PG_DEFAULT_DRIVER, self)
+        self.jinja_env.filters['qtLiteral'] = driver.qtLiteral
+        self.jinja_env.filters['qtIdent'] = driver.qtIdent
+        self.jinja_env.filters['qtTypeIdent'] = driver.qtTypeIdent
+        self.jinja_loader = FileSystemLoader(
+            os.path.dirname(os.path.realpath(__file__)) + "/../templates"
+        )

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Tue, 06 Mar 2018 15:05:49 +0000

Hello Khushboo,

All tests pass on CI, and the code looks good. The only issue that I found
was the line:
 os.path.dirname(os.path.realpath(__file__)) + "/../templates"

that will not work on windows.
So I updated your patch with the change and it should be ready to merge.

Thanks
Joao

index a4ed8729..7e3f8a07 100644
index 00000000..5d02af2b
@@ -0,0 +1,217 @@
+            os.path.join(
+                os.path.dirname(os.path.realpath(__file__)),
+                os.pardir,
+                'templates'
+            )
+        )

---

From: Khushboo Vashi <[email-masked]>
Date: Wed, 7 Mar 2018 10:00:36 +0530

On Tue, Mar 6, 2018 at 8:35 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

> Thanks Joao.

Thanks

---

From: Dave Page <[email-masked]>
Date: Wed, 7 Mar 2018 16:06:54 +0000

Thanks - applied.

On Wed, Mar 7, 2018 at 4:30 AM, Khushboo Vashi <
[email-masked]> wrote:

>> All tests pass on CI, and the code looks good. The only issue that I
>> found was the line:

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

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