public inbox for [email protected]help / color / mirror / Atom feed
[pgAdmin4][RM#2989] To fix the issue in Table node 12+ messages / 3 participants [nested] [flat]
* [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-06 09:22 Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Murtuza Zabuawala @ 2018-03-06 09:22 UTC (permalink / raw) To: pgadmin-hackers Hi, PFA patch to fix the issue in Table node where wrong sql was generated while altering column. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company Attachments: [application/octet-stream] RM_2989.diff (6.7K, 3-RM_2989.diff) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py new file mode 100644 index 0000000..9c59c1e --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py @@ -0,0 +1,90 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2018, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +import json +import uuid + +from pgadmin.browser.server_groups.servers.databases.schemas.tests import \ + utils as schema_utils +from pgadmin.browser.server_groups.servers.databases.tests import utils as \ + database_utils +from pgadmin.utils.route import BaseTestGenerator +from regression import parent_node_dict +from regression.python_test_utils import test_utils as utils +from . import utils as tables_utils + + +class TableNotNullUpdateTestCase(BaseTestGenerator): + """This class will add new collation under schema node.""" + scenarios = [ + ('Update Table with not null field', dict(url='/browser/table/obj/')), + ] + + def setUp(self): + self.db_name = parent_node_dict["database"][-1]["db_name"] + schema_info = parent_node_dict["schema"][-1] + self.server_id = schema_info["server_id"] + self.db_id = schema_info["db_id"] + db_con = database_utils.connect_database( + self, utils.SERVER_GROUP, self.server_id, self.db_id + ) + if not db_con['data']["connected"]: + raise Exception("Could not connect to database to add a table.") + self.schema_id = schema_info["schema_id"] + self.schema_name = schema_info["schema_name"] + schema_response = schema_utils.verify_schemas(self.server, + self.db_name, + self.schema_name) + if not schema_response: + raise Exception("Could not find the schema to add a table.") + + self.table_name = "test_table_column_put_%s" % (str(uuid.uuid4())[1:8]) + + custom_sql = 'column_1 "char" NOT NULL' + + self.table_id = tables_utils.create_table( + self.server, + self.db_name, + self.schema_name, + self.table_name, + custom_sql + ) + + def runTest(self): + """This function will fetch added table under schema node.""" + table_response = tables_utils.verify_table( + self.server, self.db_name, self.table_id + ) + if not table_response: + raise Exception("Could not find the table to update.") + + data = { + "id": self.table_id, + "columns": { + "changed": [ + { + "attnum": 1, + "attnotnull": False + } + ] + } + } + + response = self.tester.put( + self.url + str(utils.SERVER_GROUP) + '/' + + str(self.server_id) + '/' + str(self.db_id) + '/' + + str(self.schema_id) + '/' + str(self.table_id), + data=json.dumps(data), follow_redirects=True + ) + + self.assertEquals(response.status_code, 200) + + def tearDown(self): + # Disconnect the database + database_utils.disconnect_database(self, self.server_id, self.db_id) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py index 4cec323..b722147 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py @@ -15,7 +15,8 @@ import traceback from regression.python_test_utils import test_utils as utils -def create_table(server, db_name, schema_name, table_name): +def create_table(server, db_name, schema_name, table_name, + custom_column_sql=None): """ This function creates a table under provided schema. :param server: server details @@ -39,9 +40,13 @@ def create_table(server, db_name, schema_name, table_name): old_isolation_level = connection.isolation_level connection.set_isolation_level(0) pg_cursor = connection.cursor() - query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, name text," \ - " location text)" %\ - (schema_name, table_name) + if custom_column_sql: + query = "CREATE TABLE %s.%s(%s)" % \ + (schema_name, table_name, custom_column_sql) + else: + query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, " \ + "name text, location text)" % \ + (schema_name, table_name) pg_cursor.execute(query) connection.set_isolation_level(old_isolation_level) connection.commit() diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index 45403d1..36b6a3b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -1779,8 +1779,18 @@ class BaseTableView(PGChildNodeView): ) or matchObj.group(1) c['attprecision'] = None else: - c['attlen'] = None - c['attprecision'] = None + # Use the old values to avoid unnecessary + # sql generation + if 'attlen' in old_data: + if old_data['attlen'] != '-1': + c['attlen'] = old_data.get( + 'attlen', None + ) + if 'attprecision' in old_data: + if old_data['attprecision'] != '-1': + c['attprecision'] = old_data.get( + 'attprecision', None + ) if 'cltype' in c: typename = c['cltype'] [application/octet-stream] fix_table_node_PEP8.diff (2.4K, 4-fix_table_node_PEP8.diff) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py index 2bd30d1..1160cfb 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py @@ -349,7 +349,6 @@ class TableView(BaseTableView, DataTypeReader, VacuumSettings): if not status: return internal_server_error(errormsg=rset) - for row in rset['rows']: if 'is_partitioned' in row and row['is_partitioned']: icon = "icon-partition" diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_msql.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_msql.py index 3cdc292..c5bfa54 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_msql.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/tests/test_column_msql.py @@ -167,16 +167,20 @@ class ColumnMsqlTestCase(BaseTestGenerator): if not expected_precision and hasattr(self, 'old_precision'): expected_precision = self.old_precision - self.assertEquals(response_data['data'], - self.expected_res.format( - **dict([('schema', self.schema_name), - ('table', self.table_name), - ('column', self.column_name), - ('len', expected_len), - ('precision', expected_precision) - ] - ) - )) + self.assertEquals( + response_data['data'], + self.expected_res.format( + **dict( + [ + ('schema', self.schema_name), + ('table', self.table_name), + ('column', self.column_name), + ('len', expected_len), + ('precision', expected_precision) + ] + ) + ) + ) def tearDown(self): # Disconnect the database ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-06 16:06 Joao De Almeida Pereira <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Joao De Almeida Pereira @ 2018-03-06 16:06 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers Hi Murtuza, The code change works, and I passed the patches through our pipeline and everything is green. Personally I would love this bug fixes to have refactored the function into smaller chunk and made it more readable so that the next time someone need to check out a problem in the same area it is easier. I understand that without a good test coverage it is hard to have confidence while refactoring, but we need to start somewhere. @Hackers Here is a video that I saw some time ago about refactoring existing code and code complexity that is very interesting https://www.youtube.com/watch?v=8bZh5LMaSmE In this video Sandi Metz does the Gilded Rose Kata in a talk in RailsConf 2014, and with it tries to demonstrate that code can be refactored and with that it make the code much more simpler. But the journey is not always simple and the complexity will increase before it get simpler. It is a good example of something that we can try with our code. Thanks Joao On Tue, Mar 6, 2018 at 4:23 AM Murtuza Zabuawala < [email protected]> wrote: > Hi, > > PFA patch to fix the issue in Table node where wrong sql was generated > while altering column. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-07 12:42 Dave Page <[email protected]> parent: Joao De Almeida Pereira <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Dave Page @ 2018-03-07 12:42 UTC (permalink / raw) To: Joao De Almeida Pereira <[email protected]>; +Cc: Murtuza Zabuawala <[email protected]>; pgadmin-hackers Hi On Tue, Mar 6, 2018 at 4:06 PM, Joao De Almeida Pereira < [email protected]> wrote: > Hi Murtuza, > > The code change works, and I passed the patches through our pipeline and > everything is green. > Personally I would love this bug fixes to have refactored the function > into smaller chunk and made it more readable so that the next time someone > need to check out a problem in the same area it is easier. I understand > that without a good test coverage it is hard to have confidence while > refactoring, but we need to start somewhere. > Are you planning to look into this Murtuza? > > @Hackers > Here is a video that I saw some time ago about refactoring existing code > and code complexity that is very interesting > https://www.youtube.com/watch?v=8bZh5LMaSmE > In this video Sandi Metz does the Gilded Rose Kata in a talk in RailsConf > 2014, and with it tries to demonstrate that code can be refactored and with > that it make the code much more simpler. But the journey is not always > simple and the complexity will increase before it get simpler. It is a good > example of something that we can try with our code. > Nice! -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-07 12:44 Murtuza Zabuawala <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Murtuza Zabuawala @ 2018-03-07 12:44 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers On Wed, Mar 7, 2018 at 6:12 PM, Dave Page <[email protected]> wrote: > Hi > > On Tue, Mar 6, 2018 at 4:06 PM, Joao De Almeida Pereira < > [email protected]> wrote: > >> Hi Murtuza, >> >> The code change works, and I passed the patches through our pipeline and >> everything is green. >> Personally I would love this bug fixes to have refactored the function >> into smaller chunk and made it more readable so that the next time someone >> need to check out a problem in the same area it is easier. I understand >> that without a good test coverage it is hard to have confidence while >> refactoring, but we need to start somewhere. >> > > Are you planning to look into this Murtuza? > Yes Dave, I am working on another issue, I'll pick up after that. > > >> >> @Hackers >> Here is a video that I saw some time ago about refactoring existing code >> and code complexity that is very interesting >> https://www.youtube.com/watch?v=8bZh5LMaSmE >> In this video Sandi Metz does the Gilded Rose Kata in a talk in RailsConf >> 2014, and with it tries to demonstrate that code can be refactored and with >> that it make the code much more simpler. But the journey is not always >> simple and the complexity will increase before it get simpler. It is a good >> example of something that we can try with our code. >> > > Nice! > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-07 14:59 Murtuza Zabuawala <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Murtuza Zabuawala @ 2018-03-07 14:59 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers Hi Dave, PFA updated patch. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company On Wed, Mar 7, 2018 at 6:14 PM, Murtuza Zabuawala < [email protected]> wrote: > > On Wed, Mar 7, 2018 at 6:12 PM, Dave Page <[email protected]> wrote: > >> Hi >> >> On Tue, Mar 6, 2018 at 4:06 PM, Joao De Almeida Pereira < >> [email protected]> wrote: >> >>> Hi Murtuza, >>> >>> The code change works, and I passed the patches through our pipeline and >>> everything is green. >>> Personally I would love this bug fixes to have refactored the function >>> into smaller chunk and made it more readable so that the next time someone >>> need to check out a problem in the same area it is easier. I understand >>> that without a good test coverage it is hard to have confidence while >>> refactoring, but we need to start somewhere. >>> >> >> Are you planning to look into this Murtuza? >> > Yes Dave, I am working on another issue, I'll pick up after that. > > >> >> >>> >>> @Hackers >>> Here is a video that I saw some time ago about refactoring existing code >>> and code complexity that is very interesting >>> https://www.youtube.com/watch?v=8bZh5LMaSmE >>> In this video Sandi Metz does the Gilded Rose Kata in a talk in >>> RailsConf 2014, and with it tries to demonstrate that code can be >>> refactored and with that it make the code much more simpler. But the >>> journey is not always simple and the complexity will increase before it get >>> simpler. It is a good example of something that we can try with our code. >>> >> >> Nice! >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > Attachments: [application/octet-stream] RM_2989_v1.diff (7.1K, 3-RM_2989_v1.diff) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py new file mode 100644 index 0000000..9c59c1e --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py @@ -0,0 +1,90 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2018, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +import json +import uuid + +from pgadmin.browser.server_groups.servers.databases.schemas.tests import \ + utils as schema_utils +from pgadmin.browser.server_groups.servers.databases.tests import utils as \ + database_utils +from pgadmin.utils.route import BaseTestGenerator +from regression import parent_node_dict +from regression.python_test_utils import test_utils as utils +from . import utils as tables_utils + + +class TableNotNullUpdateTestCase(BaseTestGenerator): + """This class will add new collation under schema node.""" + scenarios = [ + ('Update Table with not null field', dict(url='/browser/table/obj/')), + ] + + def setUp(self): + self.db_name = parent_node_dict["database"][-1]["db_name"] + schema_info = parent_node_dict["schema"][-1] + self.server_id = schema_info["server_id"] + self.db_id = schema_info["db_id"] + db_con = database_utils.connect_database( + self, utils.SERVER_GROUP, self.server_id, self.db_id + ) + if not db_con['data']["connected"]: + raise Exception("Could not connect to database to add a table.") + self.schema_id = schema_info["schema_id"] + self.schema_name = schema_info["schema_name"] + schema_response = schema_utils.verify_schemas(self.server, + self.db_name, + self.schema_name) + if not schema_response: + raise Exception("Could not find the schema to add a table.") + + self.table_name = "test_table_column_put_%s" % (str(uuid.uuid4())[1:8]) + + custom_sql = 'column_1 "char" NOT NULL' + + self.table_id = tables_utils.create_table( + self.server, + self.db_name, + self.schema_name, + self.table_name, + custom_sql + ) + + def runTest(self): + """This function will fetch added table under schema node.""" + table_response = tables_utils.verify_table( + self.server, self.db_name, self.table_id + ) + if not table_response: + raise Exception("Could not find the table to update.") + + data = { + "id": self.table_id, + "columns": { + "changed": [ + { + "attnum": 1, + "attnotnull": False + } + ] + } + } + + response = self.tester.put( + self.url + str(utils.SERVER_GROUP) + '/' + + str(self.server_id) + '/' + str(self.db_id) + '/' + + str(self.schema_id) + '/' + str(self.table_id), + data=json.dumps(data), follow_redirects=True + ) + + self.assertEquals(response.status_code, 200) + + def tearDown(self): + # Disconnect the database + database_utils.disconnect_database(self, self.server_id, self.db_id) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py index 4cec323..b722147 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py @@ -15,7 +15,8 @@ import traceback from regression.python_test_utils import test_utils as utils -def create_table(server, db_name, schema_name, table_name): +def create_table(server, db_name, schema_name, table_name, + custom_column_sql=None): """ This function creates a table under provided schema. :param server: server details @@ -39,9 +40,13 @@ def create_table(server, db_name, schema_name, table_name): old_isolation_level = connection.isolation_level connection.set_isolation_level(0) pg_cursor = connection.cursor() - query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, name text," \ - " location text)" %\ - (schema_name, table_name) + if custom_column_sql: + query = "CREATE TABLE %s.%s(%s)" % \ + (schema_name, table_name, custom_column_sql) + else: + query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, " \ + "name text, location text)" % \ + (schema_name, table_name) pg_cursor.execute(query) connection.set_isolation_level(old_isolation_level) connection.commit() diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index 45403d1..b4602b4 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -1779,8 +1779,8 @@ class BaseTableView(PGChildNodeView): ) or matchObj.group(1) c['attprecision'] = None else: - c['attlen'] = None - c['attprecision'] = None + # Use the old values to avoid unnecessary + self.assign_old_length_precision(c, old_data) if 'cltype' in c: typename = c['cltype'] diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py index ddebd5e..e1b50ad 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py @@ -320,6 +320,25 @@ class DataTypeReader: return type_name + @classmethod + def assign_old_length_precision(cls, data, old_data): + """ + Assign the old values to new which will avoid unnecessary + sql generation + + Args: + data: New values + old_data: Old values + """ + if 'attlen' in old_data: + if old_data['attlen'] != '-1': + data['attlen'] = old_data.get('attlen', None) + if 'attprecision' in old_data: + if old_data['attprecision'] != '-1': + data['attprecision'] = old_data.get( + 'attprecision', None + ) + def trigger_definition(data): """ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-07 16:29 Dave Page <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Dave Page @ 2018-03-07 16:29 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers Hi On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala < [email protected]> wrote: > Hi Dave, > > PFA updated patch. > > Using your example on the ticket, I added a "character varying (32)" column with NOT NULL to the table. When I then edit the column, and turn off NOT NULL (making no other changes), the SQL generated is: ALTER TABLE public.test_drop ALTER COLUMN col2 TYPE character varying (32) COLLATE pg_catalog."default"; ALTER TABLE public.test_drop ALTER COLUMN col2 DROP NOT NULL; I didn't see that when turning off NOT NULL for col1. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-08 09:00 Murtuza Zabuawala <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Murtuza Zabuawala @ 2018-03-08 09:00 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers Hi Dave, Please find updated patch & updated test case to cover that as well. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company On Wed, Mar 7, 2018 at 9:59 PM, Dave Page <[email protected]> wrote: > Hi > > On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala <murtuza.zabuawala@ > enterprisedb.com> wrote: > >> Hi Dave, >> >> PFA updated patch. >> >> > Using your example on the ticket, I added a "character varying (32)" > column with NOT NULL to the table. When I then edit the column, and turn > off NOT NULL (making no other changes), the SQL generated is: > > ALTER TABLE public.test_drop > ALTER COLUMN col2 TYPE character varying (32) COLLATE > pg_catalog."default"; > ALTER TABLE public.test_drop > ALTER COLUMN col2 DROP NOT NULL; > > I didn't see that when turning off NOT NULL for col1. > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > Attachments: [application/octet-stream] RM_2989_v2.diff (13.6K, 3-RM_2989_v2.diff) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py index 2b1ac92..f1ba6f6 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py @@ -364,20 +364,9 @@ class ColumnsView(PGChildNodeView, DataTypeReader): data['attlen'] = None data['attprecision'] = None - import re - - # If we have length & precision both - if length and precision: - matchObj = re.search(r'(\d+),(\d+)', fulltype) - if matchObj: - data['attlen'] = matchObj.group(1) - data['attprecision'] = matchObj.group(2) - elif length: - # If we have length only - matchObj = re.search(r'(\d+)', fulltype) - if matchObj: - data['attlen'] = matchObj.group(1) - data['attprecision'] = None + self.set_length_precision( + length, precision, fulltype, data + ) # We need to fetch inherited tables for each table SQL = render_template("/".join([self.template_path, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py new file mode 100644 index 0000000..1321d6f --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py @@ -0,0 +1,96 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2018, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +import json +import uuid + +from pgadmin.browser.server_groups.servers.databases.schemas.tests import \ + utils as schema_utils +from pgadmin.browser.server_groups.servers.databases.tests import utils as \ + database_utils +from pgadmin.utils.route import BaseTestGenerator +from regression import parent_node_dict +from regression.python_test_utils import test_utils as utils +from . import utils as tables_utils + + +class TableNotNullUpdateTestCase(BaseTestGenerator): + """This class will add new collation under schema node.""" + scenarios = [ + ('Update Table with not null field', dict(url='/browser/table/obj/')), + ] + + def setUp(self): + self.db_name = parent_node_dict["database"][-1]["db_name"] + schema_info = parent_node_dict["schema"][-1] + self.server_id = schema_info["server_id"] + self.db_id = schema_info["db_id"] + db_con = database_utils.connect_database( + self, utils.SERVER_GROUP, self.server_id, self.db_id + ) + if not db_con['data']["connected"]: + raise Exception("Could not connect to database to add a table.") + self.schema_id = schema_info["schema_id"] + self.schema_name = schema_info["schema_name"] + schema_response = schema_utils.verify_schemas(self.server, + self.db_name, + self.schema_name) + if not schema_response: + raise Exception("Could not find the schema to add a table.") + + self.table_name = "test_table_column_put_%s" % (str(uuid.uuid4())[1:8]) + + custom_sql = 'column_1 "char" NOT NULL, ' \ + 'column_2 character varying(10) NOT NULL' + + self.table_id = tables_utils.create_table( + self.server, + self.db_name, + self.schema_name, + self.table_name, + custom_sql + ) + + def runTest(self): + """This function will fetch added table under schema node.""" + table_response = tables_utils.verify_table( + self.server, self.db_name, self.table_id + ) + if not table_response: + raise Exception("Could not find the table to update.") + + data = { + "id": self.table_id, + "columns": { + "changed": [ + { + "attnum": 1, + "attnotnull": False + }, + { + "attnum": 2, + "attnotnull": False + } + + ] + } + } + + response = self.tester.put( + self.url + str(utils.SERVER_GROUP) + '/' + + str(self.server_id) + '/' + str(self.db_id) + '/' + + str(self.schema_id) + '/' + str(self.table_id), + data=json.dumps(data), follow_redirects=True + ) + + self.assertEquals(response.status_code, 200) + + def tearDown(self): + # Disconnect the database + database_utils.disconnect_database(self, self.server_id, self.db_id) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py index 4cec323..b722147 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py @@ -15,7 +15,8 @@ import traceback from regression.python_test_utils import test_utils as utils -def create_table(server, db_name, schema_name, table_name): +def create_table(server, db_name, schema_name, table_name, + custom_column_sql=None): """ This function creates a table under provided schema. :param server: server details @@ -39,9 +40,13 @@ def create_table(server, db_name, schema_name, table_name): old_isolation_level = connection.isolation_level connection.set_isolation_level(0) pg_cursor = connection.cursor() - query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, name text," \ - " location text)" %\ - (schema_name, table_name) + if custom_column_sql: + query = "CREATE TABLE %s.%s(%s)" % \ + (schema_name, table_name, custom_column_sql) + else: + query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, " \ + "name text, location text)" % \ + (schema_name, table_name) pg_cursor.execute(query) connection.set_isolation_level(old_isolation_level) connection.commit() diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index 45403d1..f6d1b22 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -1748,39 +1748,20 @@ class BaseTableView(PGChildNodeView): old_data['atttypmod'] ) - def get_type_attr(key, data): - """Utility function""" - if key in data: - return data[key] - return None - # If the column data type has not changed then fetch # old length and precision if 'elemoid' in old_data and 'cltype' not in c: length, precision, typeval = \ self.get_length_precision(old_data['elemoid']) + # Set proper values for old data + self.set_length_precision( + length, precision, fulltype, old_data + ) - # If we have length & precision both - if length and precision: - matchObj = re.search(r'(\d+),(\d+)', fulltype) - if matchObj: - c['attlen'] = get_type_attr( - 'attlen', c - ) or matchObj.group(1) - c['attprecision'] = get_type_attr( - 'attprecision', c - ) or matchObj.group(2) - elif length: - # If we have length only - matchObj = re.search(r'(\d+)', fulltype) - if matchObj: - c['attlen'] = get_type_attr( - 'attlen', c - ) or matchObj.group(1) - c['attprecision'] = None - else: - c['attlen'] = None - c['attprecision'] = None + # Set proper values for in new data + self.set_length_precision( + length, precision, fulltype, c, old_data + ) if 'cltype' in c: typename = c['cltype'] diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py index a5ed56a..2167a64 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py @@ -439,23 +439,15 @@ class TypeView(PGChildNodeView, DataTypeReader): if 'elemoid' in row: is_tlength, is_precision, typeval = self.get_length_precision(row['elemoid']) - # Below logic will allow us to split length, precision from type name for grid - import re - t_len = None - t_prec = None - - # If we have length & precision both - if is_tlength and is_precision: - matchObj = re.search(r'(\d+),(\d+)', row['fulltype']) - if matchObj: - t_len = matchObj.group(1) - t_prec = matchObj.group(2) - elif is_tlength: - # If we have length only - matchObj = re.search(r'(\d+)', row['fulltype']) - if matchObj: - t_len = matchObj.group(1) - t_prec = None + data = { + 'attlen': None, + 'attprecision': None + } + self.set_length_precision( + is_tlength, is_precision, row['fulltype'], data + ) + t_len = data['attlen'] + t_prec = data['attprecision'] type_name = DataTypeReader.parse_type_name(row['typname']) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py index ddebd5e..b68cf8b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py @@ -8,7 +8,7 @@ ########################################################################## """Schema collection node helper class""" - +import re import json from flask import render_template @@ -320,6 +320,50 @@ class DataTypeReader: return type_name + @classmethod + def set_length_precision(cls, length, precision, fulltype, data, + old_data=None): + """ + Parse length & precision from datatype and then assign it to datatype + according to client format + + Args: + length: Boolean flag for length + precision: Boolean flag for precision + fulltype: Type name with length & precision + data: New values + old_data: Old values + """ + # If we have length & precision both + + get_valid_value = lambda val: val if val and int(val) != -1 else None + + if length and precision: + matchObj = re.search(r'(\d+),(\d+)', fulltype) + if matchObj: + attlen = get_valid_value(data.get('attlen', None)) + data['attlen'] = attlen or matchObj.group(1) + attprecision = get_valid_value(data.get('attprecision', None)) + data['attprecision'] = attprecision or matchObj.group(2) + elif length: + # If we have length only + matchObj = re.search(r'(\d+)', fulltype) + if matchObj: + attlen = get_valid_value(data.get('attlen', None)) + data['attlen'] = attlen or matchObj.group(1) + data['attprecision'] = None + else: + # Use the old values to avoid unnecessary + if old_data: + if 'attlen' in old_data: + if old_data['attlen'] != '-1': + data['attlen'] = old_data.get('attlen', None) + if 'attprecision' in old_data: + if old_data['attprecision'] != '-1': + data['attprecision'] = old_data.get( + 'attprecision', None + ) + def trigger_definition(data): """ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-08 12:40 Dave Page <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Dave Page @ 2018-03-08 12:40 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers Can you rebase this please? Thanks. On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala < [email protected]> wrote: > Hi Dave, > > Please find updated patch & updated test case to cover that as well. > > > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > On Wed, Mar 7, 2018 at 9:59 PM, Dave Page <[email protected]> wrote: > >> Hi >> >> On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala < >> [email protected]> wrote: >> >>> Hi Dave, >>> >>> PFA updated patch. >>> >>> >> Using your example on the ticket, I added a "character varying (32)" >> column with NOT NULL to the table. When I then edit the column, and turn >> off NOT NULL (making no other changes), the SQL generated is: >> >> ALTER TABLE public.test_drop >> ALTER COLUMN col2 TYPE character varying (32) COLLATE >> pg_catalog."default"; >> ALTER TABLE public.test_drop >> ALTER COLUMN col2 DROP NOT NULL; >> >> I didn't see that when turning off NOT NULL for col1. >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-08 13:13 Murtuza Zabuawala <[email protected]> parent: Dave Page <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Murtuza Zabuawala @ 2018-03-08 13:13 UTC (permalink / raw) To: Dave Page <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers Hi Dave, Please find updated patch. -- Regards, Murtuza Zabuawala EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company On Thu, Mar 8, 2018 at 6:10 PM, Dave Page <[email protected]> wrote: > Can you rebase this please? > > Thanks. > > On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala <murtuza.zabuawala@ > enterprisedb.com> wrote: > >> Hi Dave, >> >> Please find updated patch & updated test case to cover that as well. >> >> >> >> -- >> Regards, >> Murtuza Zabuawala >> EnterpriseDB: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> >> On Wed, Mar 7, 2018 at 9:59 PM, Dave Page <[email protected]> wrote: >> >>> Hi >>> >>> On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala < >>> [email protected]> wrote: >>> >>>> Hi Dave, >>>> >>>> PFA updated patch. >>>> >>>> >>> Using your example on the ticket, I added a "character varying (32)" >>> column with NOT NULL to the table. When I then edit the column, and turn >>> off NOT NULL (making no other changes), the SQL generated is: >>> >>> ALTER TABLE public.test_drop >>> ALTER COLUMN col2 TYPE character varying (32) COLLATE >>> pg_catalog."default"; >>> ALTER TABLE public.test_drop >>> ALTER COLUMN col2 DROP NOT NULL; >>> >>> I didn't see that when turning off NOT NULL for col1. >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > Attachments: [application/octet-stream] RM_2989_v2_rebase.diff (13.5K, 3-RM_2989_v2_rebase.diff) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py index 2b1ac92..f1ba6f6 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py @@ -364,20 +364,9 @@ class ColumnsView(PGChildNodeView, DataTypeReader): data['attlen'] = None data['attprecision'] = None - import re - - # If we have length & precision both - if length and precision: - matchObj = re.search(r'(\d+),(\d+)', fulltype) - if matchObj: - data['attlen'] = matchObj.group(1) - data['attprecision'] = matchObj.group(2) - elif length: - # If we have length only - matchObj = re.search(r'(\d+)', fulltype) - if matchObj: - data['attlen'] = matchObj.group(1) - data['attprecision'] = None + self.set_length_precision( + length, precision, fulltype, data + ) # We need to fetch inherited tables for each table SQL = render_template("/".join([self.template_path, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py new file mode 100644 index 0000000..1321d6f --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py @@ -0,0 +1,96 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2018, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +import json +import uuid + +from pgadmin.browser.server_groups.servers.databases.schemas.tests import \ + utils as schema_utils +from pgadmin.browser.server_groups.servers.databases.tests import utils as \ + database_utils +from pgadmin.utils.route import BaseTestGenerator +from regression import parent_node_dict +from regression.python_test_utils import test_utils as utils +from . import utils as tables_utils + + +class TableNotNullUpdateTestCase(BaseTestGenerator): + """This class will add new collation under schema node.""" + scenarios = [ + ('Update Table with not null field', dict(url='/browser/table/obj/')), + ] + + def setUp(self): + self.db_name = parent_node_dict["database"][-1]["db_name"] + schema_info = parent_node_dict["schema"][-1] + self.server_id = schema_info["server_id"] + self.db_id = schema_info["db_id"] + db_con = database_utils.connect_database( + self, utils.SERVER_GROUP, self.server_id, self.db_id + ) + if not db_con['data']["connected"]: + raise Exception("Could not connect to database to add a table.") + self.schema_id = schema_info["schema_id"] + self.schema_name = schema_info["schema_name"] + schema_response = schema_utils.verify_schemas(self.server, + self.db_name, + self.schema_name) + if not schema_response: + raise Exception("Could not find the schema to add a table.") + + self.table_name = "test_table_column_put_%s" % (str(uuid.uuid4())[1:8]) + + custom_sql = 'column_1 "char" NOT NULL, ' \ + 'column_2 character varying(10) NOT NULL' + + self.table_id = tables_utils.create_table( + self.server, + self.db_name, + self.schema_name, + self.table_name, + custom_sql + ) + + def runTest(self): + """This function will fetch added table under schema node.""" + table_response = tables_utils.verify_table( + self.server, self.db_name, self.table_id + ) + if not table_response: + raise Exception("Could not find the table to update.") + + data = { + "id": self.table_id, + "columns": { + "changed": [ + { + "attnum": 1, + "attnotnull": False + }, + { + "attnum": 2, + "attnotnull": False + } + + ] + } + } + + response = self.tester.put( + self.url + str(utils.SERVER_GROUP) + '/' + + str(self.server_id) + '/' + str(self.db_id) + '/' + + str(self.schema_id) + '/' + str(self.table_id), + data=json.dumps(data), follow_redirects=True + ) + + self.assertEquals(response.status_code, 200) + + def tearDown(self): + # Disconnect the database + database_utils.disconnect_database(self, self.server_id, self.db_id) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py index 4cec323..b722147 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py @@ -15,7 +15,8 @@ import traceback from regression.python_test_utils import test_utils as utils -def create_table(server, db_name, schema_name, table_name): +def create_table(server, db_name, schema_name, table_name, + custom_column_sql=None): """ This function creates a table under provided schema. :param server: server details @@ -39,9 +40,13 @@ def create_table(server, db_name, schema_name, table_name): old_isolation_level = connection.isolation_level connection.set_isolation_level(0) pg_cursor = connection.cursor() - query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, name text," \ - " location text)" %\ - (schema_name, table_name) + if custom_column_sql: + query = "CREATE TABLE %s.%s(%s)" % \ + (schema_name, table_name, custom_column_sql) + else: + query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, " \ + "name text, location text)" % \ + (schema_name, table_name) pg_cursor.execute(query) connection.set_isolation_level(old_isolation_level) connection.commit() diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index 45403d1..f6d1b22 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -1748,39 +1748,20 @@ class BaseTableView(PGChildNodeView): old_data['atttypmod'] ) - def get_type_attr(key, data): - """Utility function""" - if key in data: - return data[key] - return None - # If the column data type has not changed then fetch # old length and precision if 'elemoid' in old_data and 'cltype' not in c: length, precision, typeval = \ self.get_length_precision(old_data['elemoid']) + # Set proper values for old data + self.set_length_precision( + length, precision, fulltype, old_data + ) - # If we have length & precision both - if length and precision: - matchObj = re.search(r'(\d+),(\d+)', fulltype) - if matchObj: - c['attlen'] = get_type_attr( - 'attlen', c - ) or matchObj.group(1) - c['attprecision'] = get_type_attr( - 'attprecision', c - ) or matchObj.group(2) - elif length: - # If we have length only - matchObj = re.search(r'(\d+)', fulltype) - if matchObj: - c['attlen'] = get_type_attr( - 'attlen', c - ) or matchObj.group(1) - c['attprecision'] = None - else: - c['attlen'] = None - c['attprecision'] = None + # Set proper values for in new data + self.set_length_precision( + length, precision, fulltype, c, old_data + ) if 'cltype' in c: typename = c['cltype'] diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py index 2395439..b995c40 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py @@ -464,22 +464,17 @@ class TypeView(PGChildNodeView, DataTypeReader): # Below logic will allow us to split length, precision from # type name for grid - import re - t_len = None - t_prec = None - - # If we have length & precision both - if is_tlength and is_precision: - matchObj = re.search(r'(\d+),(\d+)', row['fulltype']) - if matchObj: - t_len = matchObj.group(1) - t_prec = matchObj.group(2) - elif is_tlength: - # If we have length only - matchObj = re.search(r'(\d+)', row['fulltype']) - if matchObj: - t_len = matchObj.group(1) - t_prec = None + data = { + 'attlen': None, + 'attprecision': None + } + + self.set_length_precision( + is_tlength, is_precision, row['fulltype'], data + ) + + t_len = data['attlen'] + t_prec = data['attprecision'] type_name = DataTypeReader.parse_type_name(row['typname']) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py index f330ed9..95d3e1c 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py @@ -8,7 +8,7 @@ ########################################################################## """Schema collection node helper class""" - +import re import json from flask import render_template @@ -336,6 +336,50 @@ class DataTypeReader: return type_name + @classmethod + def set_length_precision(cls, length, precision, fulltype, data, + old_data=None): + """ + Parse length & precision from datatype and then assign it to datatype + according to client format + + Args: + length: Boolean flag for length + precision: Boolean flag for precision + fulltype: Type name with length & precision + data: New values + old_data: Old values + """ + # If we have length & precision both + + get_valid_value = lambda val: val if val and int(val) != -1 else None + + if length and precision: + matchObj = re.search(r'(\d+),(\d+)', fulltype) + if matchObj: + attlen = get_valid_value(data.get('attlen', None)) + data['attlen'] = attlen or matchObj.group(1) + attprecision = get_valid_value(data.get('attprecision', None)) + data['attprecision'] = attprecision or matchObj.group(2) + elif length: + # If we have length only + matchObj = re.search(r'(\d+)', fulltype) + if matchObj: + attlen = get_valid_value(data.get('attlen', None)) + data['attlen'] = attlen or matchObj.group(1) + data['attprecision'] = None + else: + # Use the old values to avoid unnecessary + if old_data: + if 'attlen' in old_data: + if old_data['attlen'] != '-1': + data['attlen'] = old_data.get('attlen', None) + if 'attprecision' in old_data: + if old_data['attprecision'] != '-1': + data['attprecision'] = old_data.get( + 'attprecision', None + ) + def trigger_definition(data): """ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-08 16:49 Joao De Almeida Pereira <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Joao De Almeida Pereira @ 2018-03-08 16:49 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers Hello Murtuza/Dave, Nice splitting of some of the functionality into functions, removing some of the complexity of the initial function. Good job. I made some changes because the linter was failing and also changed some variable names. These changes pass our CI and the linter. Thanks Joao On Thu, Mar 8, 2018 at 8:13 AM Murtuza Zabuawala < [email protected]> wrote: > Hi Dave, > > Please find updated patch. > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > On Thu, Mar 8, 2018 at 6:10 PM, Dave Page <[email protected]> wrote: > >> Can you rebase this please? >> >> Thanks. >> >> On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala < >> [email protected]> wrote: >> >>> Hi Dave, >>> >>> Please find updated patch & updated test case to cover that as well. >>> >>> >>> >>> -- >>> Regards, >>> Murtuza Zabuawala >>> EnterpriseDB: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >>> >>> On Wed, Mar 7, 2018 at 9:59 PM, Dave Page <[email protected]> wrote: >>> >>>> Hi >>>> >>>> On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala < >>>> [email protected]> wrote: >>>> >>>>> Hi Dave, >>>>> >>>>> PFA updated patch. >>>>> >>>>> >>>> Using your example on the ticket, I added a "character varying (32)" >>>> column with NOT NULL to the table. When I then edit the column, and turn >>>> off NOT NULL (making no other changes), the SQL generated is: >>>> >>>> ALTER TABLE public.test_drop >>>> ALTER COLUMN col2 TYPE character varying (32) COLLATE >>>> pg_catalog."default"; >>>> ALTER TABLE public.test_drop >>>> ALTER COLUMN col2 DROP NOT NULL; >>>> >>>> I didn't see that when turning off NOT NULL for col1. >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >>> >> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > Attachments: [application/octet-stream] RM_2989_version3.diff (13.8K, 3-RM_2989_version3.diff) download | inline diff: diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py index 2b1ac92a..f1ba6f6e 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/__init__.py @@ -364,20 +364,9 @@ class ColumnsView(PGChildNodeView, DataTypeReader): data['attlen'] = None data['attprecision'] = None - import re - - # If we have length & precision both - if length and precision: - matchObj = re.search(r'(\d+),(\d+)', fulltype) - if matchObj: - data['attlen'] = matchObj.group(1) - data['attprecision'] = matchObj.group(2) - elif length: - # If we have length only - matchObj = re.search(r'(\d+)', fulltype) - if matchObj: - data['attlen'] = matchObj.group(1) - data['attprecision'] = None + self.set_length_precision( + length, precision, fulltype, data + ) # We need to fetch inherited tables for each table SQL = render_template("/".join([self.template_path, diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py new file mode 100644 index 00000000..1321d6f6 --- /dev/null +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/test_table_column_update.py @@ -0,0 +1,96 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2018, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +import json +import uuid + +from pgadmin.browser.server_groups.servers.databases.schemas.tests import \ + utils as schema_utils +from pgadmin.browser.server_groups.servers.databases.tests import utils as \ + database_utils +from pgadmin.utils.route import BaseTestGenerator +from regression import parent_node_dict +from regression.python_test_utils import test_utils as utils +from . import utils as tables_utils + + +class TableNotNullUpdateTestCase(BaseTestGenerator): + """This class will add new collation under schema node.""" + scenarios = [ + ('Update Table with not null field', dict(url='/browser/table/obj/')), + ] + + def setUp(self): + self.db_name = parent_node_dict["database"][-1]["db_name"] + schema_info = parent_node_dict["schema"][-1] + self.server_id = schema_info["server_id"] + self.db_id = schema_info["db_id"] + db_con = database_utils.connect_database( + self, utils.SERVER_GROUP, self.server_id, self.db_id + ) + if not db_con['data']["connected"]: + raise Exception("Could not connect to database to add a table.") + self.schema_id = schema_info["schema_id"] + self.schema_name = schema_info["schema_name"] + schema_response = schema_utils.verify_schemas(self.server, + self.db_name, + self.schema_name) + if not schema_response: + raise Exception("Could not find the schema to add a table.") + + self.table_name = "test_table_column_put_%s" % (str(uuid.uuid4())[1:8]) + + custom_sql = 'column_1 "char" NOT NULL, ' \ + 'column_2 character varying(10) NOT NULL' + + self.table_id = tables_utils.create_table( + self.server, + self.db_name, + self.schema_name, + self.table_name, + custom_sql + ) + + def runTest(self): + """This function will fetch added table under schema node.""" + table_response = tables_utils.verify_table( + self.server, self.db_name, self.table_id + ) + if not table_response: + raise Exception("Could not find the table to update.") + + data = { + "id": self.table_id, + "columns": { + "changed": [ + { + "attnum": 1, + "attnotnull": False + }, + { + "attnum": 2, + "attnotnull": False + } + + ] + } + } + + response = self.tester.put( + self.url + str(utils.SERVER_GROUP) + '/' + + str(self.server_id) + '/' + str(self.db_id) + '/' + + str(self.schema_id) + '/' + str(self.table_id), + data=json.dumps(data), follow_redirects=True + ) + + self.assertEquals(response.status_code, 200) + + def tearDown(self): + # Disconnect the database + database_utils.disconnect_database(self, self.server_id, self.db_id) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py index 4cec323b..b722147c 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/tests/utils.py @@ -15,7 +15,8 @@ import traceback from regression.python_test_utils import test_utils as utils -def create_table(server, db_name, schema_name, table_name): +def create_table(server, db_name, schema_name, table_name, + custom_column_sql=None): """ This function creates a table under provided schema. :param server: server details @@ -39,9 +40,13 @@ def create_table(server, db_name, schema_name, table_name): old_isolation_level = connection.isolation_level connection.set_isolation_level(0) pg_cursor = connection.cursor() - query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, name text," \ - " location text)" %\ - (schema_name, table_name) + if custom_column_sql: + query = "CREATE TABLE %s.%s(%s)" % \ + (schema_name, table_name, custom_column_sql) + else: + query = "CREATE TABLE %s.%s(id serial UNIQUE NOT NULL, " \ + "name text, location text)" % \ + (schema_name, table_name) pg_cursor.execute(query) connection.set_isolation_level(old_isolation_level) connection.commit() diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py index 45403d11..f6d1b229 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/utils.py @@ -1748,39 +1748,20 @@ class BaseTableView(PGChildNodeView): old_data['atttypmod'] ) - def get_type_attr(key, data): - """Utility function""" - if key in data: - return data[key] - return None - # If the column data type has not changed then fetch # old length and precision if 'elemoid' in old_data and 'cltype' not in c: length, precision, typeval = \ self.get_length_precision(old_data['elemoid']) + # Set proper values for old data + self.set_length_precision( + length, precision, fulltype, old_data + ) - # If we have length & precision both - if length and precision: - matchObj = re.search(r'(\d+),(\d+)', fulltype) - if matchObj: - c['attlen'] = get_type_attr( - 'attlen', c - ) or matchObj.group(1) - c['attprecision'] = get_type_attr( - 'attprecision', c - ) or matchObj.group(2) - elif length: - # If we have length only - matchObj = re.search(r'(\d+)', fulltype) - if matchObj: - c['attlen'] = get_type_attr( - 'attlen', c - ) or matchObj.group(1) - c['attprecision'] = None - else: - c['attlen'] = None - c['attprecision'] = None + # Set proper values for in new data + self.set_length_precision( + length, precision, fulltype, c, old_data + ) if 'cltype' in c: typename = c['cltype'] diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py index 2395439c..b995c408 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/types/__init__.py @@ -464,22 +464,17 @@ class TypeView(PGChildNodeView, DataTypeReader): # Below logic will allow us to split length, precision from # type name for grid - import re - t_len = None - t_prec = None - - # If we have length & precision both - if is_tlength and is_precision: - matchObj = re.search(r'(\d+),(\d+)', row['fulltype']) - if matchObj: - t_len = matchObj.group(1) - t_prec = matchObj.group(2) - elif is_tlength: - # If we have length only - matchObj = re.search(r'(\d+)', row['fulltype']) - if matchObj: - t_len = matchObj.group(1) - t_prec = None + data = { + 'attlen': None, + 'attprecision': None + } + + self.set_length_precision( + is_tlength, is_precision, row['fulltype'], data + ) + + t_len = data['attlen'] + t_prec = data['attprecision'] type_name = DataTypeReader.parse_type_name(row['typname']) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py index f330ed96..8b73aa00 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/utils.py @@ -8,7 +8,7 @@ ########################################################################## """Schema collection node helper class""" - +import re import json from flask import render_template @@ -336,6 +336,56 @@ class DataTypeReader: return type_name + @classmethod + def set_length_precision(cls, length, precision, fulltype, data, + old_data=None): + """ + Parse length & precision from datatype and then assign it to datatype + according to client format + + Args: + length: Boolean flag for length + precision: Boolean flag for precision + fulltype: Type name with length & precision + data: New values + old_data: Old values + """ + # If we have length & precision both + + if length and precision: + match_obj = re.search(r'(\d+),(\d+)', fulltype) + if match_obj: + attribute_length = DataTypeReader.get_valid_length_value( + data.get('attlen', None)) + data['attlen'] = attribute_length or match_obj.group(1) + attribute_precision = DataTypeReader.get_valid_length_value( + data.get('attprecision', None)) + data['attprecision'] = attribute_precision or match_obj.group( + 2) + elif length: + # If we have length only + match_obj = re.search(r'(\d+)', fulltype) + if match_obj: + attribute_length = DataTypeReader.get_valid_length_value( + data.get('attlen', None)) + data['attlen'] = attribute_length or match_obj.group(1) + data['attprecision'] = None + else: + # Use the old values to avoid unnecessary + if old_data: + if 'attlen' in old_data: + if old_data['attlen'] != '-1': + data['attlen'] = old_data.get('attlen', None) + if 'attprecision' in old_data: + if old_data['attprecision'] != '-1': + data['attprecision'] = old_data.get( + 'attprecision', None + ) + + @classmethod + def get_valid_length_value(cls, val): + return val if val and int(val) != -1 else None + def trigger_definition(data): """ ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-08 18:00 Murtuza Zabuawala <[email protected]> parent: Joao De Almeida Pereira <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Murtuza Zabuawala @ 2018-03-08 18:00 UTC (permalink / raw) To: Joao De Almeida Pereira <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers Thank you Joao Regards, Murtuza On Thu, Mar 8, 2018 at 10:19 PM, Joao De Almeida Pereira < [email protected]> wrote: > Hello Murtuza/Dave, > > Nice splitting of some of the functionality into functions, removing some > of the complexity of the initial function. Good job. > > I made some changes because the linter was failing and also changed some > variable names. > These changes pass our CI and the linter. > > Thanks > Joao > > On Thu, Mar 8, 2018 at 8:13 AM Murtuza Zabuawala <murtuza.zabuawala@ > enterprisedb.com> wrote: > >> Hi Dave, >> >> Please find updated patch. >> >> -- >> Regards, >> Murtuza Zabuawala >> EnterpriseDB: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> >> >> On Thu, Mar 8, 2018 at 6:10 PM, Dave Page <[email protected]> wrote: >> >>> Can you rebase this please? >>> >>> Thanks. >>> >>> On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala <murtuza.zabuawala@ >>> enterprisedb.com> wrote: >>> >>>> Hi Dave, >>>> >>>> Please find updated patch & updated test case to cover that as well. >>>> >>>> >>>> >>>> -- >>>> Regards, >>>> Murtuza Zabuawala >>>> EnterpriseDB: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>>> >>>> On Wed, Mar 7, 2018 at 9:59 PM, Dave Page <[email protected]> wrote: >>>> >>>>> Hi >>>>> >>>>> On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala <murtuza.zabuawala@ >>>>> enterprisedb.com> wrote: >>>>> >>>>>> Hi Dave, >>>>>> >>>>>> PFA updated patch. >>>>>> >>>>>> >>>>> Using your example on the ticket, I added a "character varying (32)" >>>>> column with NOT NULL to the table. When I then edit the column, and turn >>>>> off NOT NULL (making no other changes), the SQL generated is: >>>>> >>>>> ALTER TABLE public.test_drop >>>>> ALTER COLUMN col2 TYPE character varying (32) COLLATE >>>>> pg_catalog."default"; >>>>> ALTER TABLE public.test_drop >>>>> ALTER COLUMN col2 DROP NOT NULL; >>>>> >>>>> I didn't see that when turning off NOT NULL for col1. >>>>> >>>>> -- >>>>> Dave Page >>>>> Blog: http://pgsnake.blogspot.com >>>>> Twitter: @pgsnake >>>>> >>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>> The Enterprise PostgreSQL Company >>>>> >>>> >>>> >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: [pgAdmin4][RM#2989] To fix the issue in Table node @ 2018-03-09 15:23 Dave Page <[email protected]> parent: Murtuza Zabuawala <[email protected]> 0 siblings, 0 replies; 12+ messages in thread From: Dave Page @ 2018-03-09 15:23 UTC (permalink / raw) To: Murtuza Zabuawala <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers Thanks, patch applied. On Thu, Mar 8, 2018 at 6:00 PM, Murtuza Zabuawala < [email protected]> wrote: > Thank you Joao > > Regards, > Murtuza > > > On Thu, Mar 8, 2018 at 10:19 PM, Joao De Almeida Pereira < > [email protected]> wrote: > >> Hello Murtuza/Dave, >> >> Nice splitting of some of the functionality into functions, removing some >> of the complexity of the initial function. Good job. >> >> I made some changes because the linter was failing and also changed some >> variable names. >> These changes pass our CI and the linter. >> >> Thanks >> Joao >> >> On Thu, Mar 8, 2018 at 8:13 AM Murtuza Zabuawala < >> [email protected]> wrote: >> >>> Hi Dave, >>> >>> Please find updated patch. >>> >>> -- >>> Regards, >>> Murtuza Zabuawala >>> EnterpriseDB: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >>> >>> On Thu, Mar 8, 2018 at 6:10 PM, Dave Page <[email protected]> wrote: >>> >>>> Can you rebase this please? >>>> >>>> Thanks. >>>> >>>> On Thu, Mar 8, 2018 at 9:00 AM, Murtuza Zabuawala < >>>> [email protected]> wrote: >>>> >>>>> Hi Dave, >>>>> >>>>> Please find updated patch & updated test case to cover that as well. >>>>> >>>>> >>>>> >>>>> -- >>>>> Regards, >>>>> Murtuza Zabuawala >>>>> EnterpriseDB: http://www.enterprisedb.com >>>>> The Enterprise PostgreSQL Company >>>>> >>>>> >>>>> On Wed, Mar 7, 2018 at 9:59 PM, Dave Page <[email protected]> wrote: >>>>> >>>>>> Hi >>>>>> >>>>>> On Wed, Mar 7, 2018 at 2:59 PM, Murtuza Zabuawala < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi Dave, >>>>>>> >>>>>>> PFA updated patch. >>>>>>> >>>>>>> >>>>>> Using your example on the ticket, I added a "character varying (32)" >>>>>> column with NOT NULL to the table. When I then edit the column, and turn >>>>>> off NOT NULL (making no other changes), the SQL generated is: >>>>>> >>>>>> ALTER TABLE public.test_drop >>>>>> ALTER COLUMN col2 TYPE character varying (32) COLLATE >>>>>> pg_catalog."default"; >>>>>> ALTER TABLE public.test_drop >>>>>> ALTER COLUMN col2 DROP NOT NULL; >>>>>> >>>>>> I didn't see that when turning off NOT NULL for col1. >>>>>> >>>>>> -- >>>>>> Dave Page >>>>>> Blog: http://pgsnake.blogspot.com >>>>>> Twitter: @pgsnake >>>>>> >>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>> The Enterprise PostgreSQL Company >>>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >>> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2018-03-09 15:23 UTC | newest] Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-03-06 09:22 [pgAdmin4][RM#2989] To fix the issue in Table node Murtuza Zabuawala <[email protected]> 2018-03-06 16:06 ` Joao De Almeida Pereira <[email protected]> 2018-03-07 12:42 ` Dave Page <[email protected]> 2018-03-07 12:44 ` Murtuza Zabuawala <[email protected]> 2018-03-07 14:59 ` Murtuza Zabuawala <[email protected]> 2018-03-07 16:29 ` Dave Page <[email protected]> 2018-03-08 09:00 ` Murtuza Zabuawala <[email protected]> 2018-03-08 12:40 ` Dave Page <[email protected]> 2018-03-08 13:13 ` Murtuza Zabuawala <[email protected]> 2018-03-08 16:49 ` Joao De Almeida Pereira <[email protected]> 2018-03-08 18:00 ` Murtuza Zabuawala <[email protected]> 2018-03-09 15:23 ` Dave Page <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox