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
On Tue, Mar 6, 2018 at 7:33 AM Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:
> Hi Joao,
>
> Thanks for reviewing.
>
> On Thu, Mar 1, 2018 at 8:06 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hello Kushboo,
>>
>> Can we add some Unit test to ensure this does not happen again?
>>
>> Please find the attached patch with the unit tests.
>
>> Thanks
>> Joao
>>
>> Thanks,
> Khushboo
>
>> On Thu, Mar 1, 2018 at 2:28 AM Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> 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 a4ed8729..7e3f8a07 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 00000000..5d02af2b
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/tests/test_view_data_templates.py
@@ -0,0 +1,217 @@
+##########################################################################
+#
+# 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.join(
+ os.path.dirname(os.path.realpath(__file__)),
+ os.pardir,
+ 'templates'
+ )
+ )
