From 826f30219dfbc1fc27acf5ff9193dd4b6c2a6620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Sat, 1 Apr 2017 16:14:36 +0200 Subject: [PATCH] Re-use method as default parameter of TableHandler add_column There is no guarantee that the method will not raise a database error on the first registration because it can rely on other tables being registered. So the method must be called only when needed. This is when the table already exists and has records. In this case other tables also already exist. issue6409 --- trytond/backend/mysql/table.py | 8 +++----- trytond/backend/postgresql/table.py | 8 +++----- trytond/backend/sqlite/table.py | 10 ++++------ trytond/backend/table.py | 5 ++--- trytond/model/modelsql.py | 16 +++++++--------- 5 files changed, 19 insertions(+), 28 deletions(-) diff --git a/trytond/backend/mysql/table.py b/trytond/backend/mysql/table.py index 78298abb94..a16f40945d 100644 --- a/trytond/backend/mysql/table.py +++ b/trytond/backend/mysql/table.py @@ -8,7 +8,6 @@ from trytond.backend.table import TableHandlerInterface logger = logging.getLogger(__name__) VARCHAR_SIZE_RE = re.compile('VARCHAR\(([0-9]+)\)') -_NO_DEFAULT = object() class TableHandler(TableHandlerInterface): @@ -181,8 +180,7 @@ class TableHandler(TableHandlerInterface): self._column_definition(column_name, default=value))) self._update_definitions(columns=True) - def add_column(self, column_name, sql_type, default=_NO_DEFAULT, - comment=''): + def add_column(self, column_name, sql_type, default=None, comment=''): cursor = Transaction().connection.cursor() database = Transaction().database @@ -236,13 +234,13 @@ class TableHandler(TableHandlerInterface): cursor.execute('ALTER TABLE `%s` ADD COLUMN `%s` %s' % (self.table_name, column_name, column_type)) - if default is not _NO_DEFAULT: + if default: # check if table is non-empty: cursor.execute('SELECT 1 FROM `%s` limit 1' % self.table_name) if cursor.rowcount: # Populate column with default values: cursor.execute('UPDATE `' + self.table_name + '` ' - 'SET `' + column_name + '` = %s', (default,)) + 'SET `' + column_name + '` = %s', (default(),)) self._update_definitions(columns=True) diff --git a/trytond/backend/postgresql/table.py b/trytond/backend/postgresql/table.py index c49ddd67ac..f2cacb8a30 100644 --- a/trytond/backend/postgresql/table.py +++ b/trytond/backend/postgresql/table.py @@ -10,7 +10,6 @@ __all__ = ['TableHandler'] logger = logging.getLogger(__name__) VARCHAR_SIZE_RE = re.compile('VARCHAR\(([0-9]+)\)') -_NO_DEFAULT = object() class TableHandler(TableHandlerInterface): @@ -238,8 +237,7 @@ class TableHandler(TableHandlerInterface): 'ALTER COLUMN "' + column_name + '" SET DEFAULT %s', (value,)) - def add_column(self, column_name, sql_type, default=_NO_DEFAULT, - comment=''): + def add_column(self, column_name, sql_type, default=None, comment=''): cursor = Transaction().connection.cursor() database = Transaction().database @@ -299,13 +297,13 @@ class TableHandler(TableHandlerInterface): % (self.table_name, column_name, column_type)) add_comment() - if default is not _NO_DEFAULT: + if default: # check if table is non-empty: cursor.execute('SELECT 1 FROM "%s" limit 1' % self.table_name) if cursor.rowcount: # Populate column with default values: cursor.execute('UPDATE "' + self.table_name + '" ' - 'SET "' + column_name + '" = %s', (default,)) + 'SET "' + column_name + '" = %s', (default(),)) self._update_definitions(columns=True) diff --git a/trytond/backend/sqlite/table.py b/trytond/backend/sqlite/table.py index 0a48b09750..c45502cf86 100644 --- a/trytond/backend/sqlite/table.py +++ b/trytond/backend/sqlite/table.py @@ -11,7 +11,6 @@ __all__ = ['TableHandler'] logger = logging.getLogger(__name__) VARCHAR_SIZE_RE = re.compile('VARCHAR\(([0-9]+)\)') -_NO_DEFAULT = object() class TableHandler(TableHandlerInterface): @@ -177,8 +176,7 @@ class TableHandler(TableHandlerInterface): def db_default(self, column_name, value): warnings.warn('Unable to set default on column with SQLite backend') - def add_column(self, column_name, sql_type, default=_NO_DEFAULT, - comment=''): + def add_column(self, column_name, sql_type, default=None, comment=''): database = Transaction().database column_type = database.sql_type(sql_type) match = VARCHAR_SIZE_RE.match(sql_type) @@ -187,7 +185,7 @@ class TableHandler(TableHandlerInterface): self._add_raw_column(column_name, column_type, default, field_size, comment) - def _add_raw_column(self, column_name, column_type, default=_NO_DEFAULT, + def _add_raw_column(self, column_name, column_type, default=None, field_size=None, string=''): if self.column_exist(column_name): base_type = column_type[0].upper() @@ -231,13 +229,13 @@ class TableHandler(TableHandlerInterface): cursor.execute(('ALTER TABLE "%s" ADD COLUMN "%s" %s') % (self.table_name, column_name, column_type)) - if default is not _NO_DEFAULT: + if default: # check if table is non-empty: cursor.execute('SELECT 1 FROM "%s" limit 1' % self.table_name) if cursor.fetchone(): # Populate column with default values: cursor.execute('UPDATE "' + self.table_name + '" ' - 'SET "' + column_name + '" = ?', (default,)) + 'SET "' + column_name + '" = ?', (default(),)) self._update_definitions(columns=True) diff --git a/trytond/backend/table.py b/trytond/backend/table.py index 7711d43239..53261f6e00 100644 --- a/trytond/backend/table.py +++ b/trytond/backend/table.py @@ -113,14 +113,13 @@ class TableHandlerInterface(object): ''' raise NotImplementedError - def add_column(self, column_name, abstract_type, default=object(), - comment=''): + def add_column(self, column_name, abstract_type, default=None, comment=''): ''' Add a column :param column_name: the column name :param abstract_type: the abstract type that will represent this column - :param default: the default value to use + :param default: the method that return default value to use :param comment: An optional comment on the column ''' raise NotImplementedError diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index cfb733fd59..0cc37230bc 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -150,17 +150,15 @@ class ModelSQL(ModelStorage): if not sql_type: continue - # Using a dict to specify the default values allows to not override - # the 'default' default value specified in the backend which is an - # object() hack - kwargs = {} + default = None if field_name in cls._defaults: - default_ = cls._clean_defaults({ - field_name: cls._defaults[field_name](), - })[field_name] - kwargs['default'] = field.sql_format(default_) + def default(): + default_ = cls._clean_defaults({ + field_name: cls._defaults[field_name](), + })[field_name] + return field.sql_format(default_) - table.add_column(field_name, field._sql_type, **kwargs) + table.add_column(field_name, field._sql_type, default=default) if cls._history: history_table.add_column(field_name, field._sql_type)