Use sql type instead of field related argument in the column creation

issue6348
review26231002

CHANGELOG

@@ -1,3 +1,4 @@
+* Use sql type in column creation
 * Use generic SQL type in field and let backend determine the SQLType
 * Add filter to xxx2Many fields
 * Add NULLS ordering

trytond/backend/mysql/table.py

@@ -1,11 +1,14 @@
 # This file is part of Tryton.  The COPYRIGHT file at the top level of
 # this repository contains the full copyright notices and license terms.
+import re
+import logging
 
 from trytond.transaction import Transaction
 from trytond.backend.table import TableHandlerInterface
-import logging
 
 logger = logging.getLogger(__name__)
+VARCHAR_SIZE_RE = re.compile('VARCHAR\(([0-9]+)\)')
+_NO_DEFAULT = object()
 
 
 class TableHandler(TableHandlerInterface):
@@ -178,11 +181,16 @@ class TableHandler(TableHandlerInterface):
                 self._column_definition(column_name, default=value)))
         self._update_definitions(columns=True)
 
-    def add_raw_column(self, column_name, column_type, column_format,
-            default_fun=None, field_size=None, migrate=True, string=''):
+    def add_column(self, column_name, sql_type, default=_NO_DEFAULT,
+            comment=''):
+        cursor = Transaction().connection.cursor()
+        database = Transaction().database
+
+        column_type = database.sql_type(sql_type)
+        match = VARCHAR_SIZE_RE.match(sql_type)
+        field_size = int(match.group(1)) if match else None
+
         if self.column_exist(column_name):
-            if not migrate:
-                return
             base_type = column_type[0].lower()
             convert = {
                 'char': 'varchar',
@@ -224,22 +232,17 @@ class TableHandler(TableHandlerInterface):
                         field_size)
             return
 
-        cursor = Transaction().connection.cursor()
         column_type = column_type[1]
         cursor.execute('ALTER TABLE `%s` ADD COLUMN `%s` %s' %
                 (self.table_name, column_name, column_type))
 
-        if column_format:
+        if default is not _NO_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:
-                default = None
-                if default_fun is not None:
-                    default = default_fun()
                 cursor.execute('UPDATE `' + self.table_name + '` '
-                    'SET `' + column_name + '` = %s',
-                    (column_format(default),))
+                    'SET `' + column_name + '` = %s', (default,))
 
         self._update_definitions(columns=True)
 

trytond/backend/postgresql/table.py

@@ -1,12 +1,16 @@
 # This file is part of Tryton.  The COPYRIGHT file at the top level of
 # this repository contains the full copyright notices and license terms.
+import re
+import logging
+
 from trytond.transaction import Transaction
 from trytond.backend.table import TableHandlerInterface
-import logging
 
 __all__ = ['TableHandler']
 
 logger = logging.getLogger(__name__)
+VARCHAR_SIZE_RE = re.compile('VARCHAR\(([0-9]+)\)')
+_NO_DEFAULT = object()
 
 
 class TableHandler(TableHandlerInterface):
@@ -234,23 +238,26 @@ class TableHandler(TableHandlerInterface):
                 'ALTER COLUMN "' + column_name + '" SET DEFAULT %s',
                 (value,))
 
-    def add_raw_column(self, column_name, column_type, column_format,
-            default_fun=None, field_size=None, migrate=True, string=''):
+    def add_column(self, column_name, sql_type, default=_NO_DEFAULT,
+            comment=''):
         cursor = Transaction().connection.cursor()
+        database = Transaction().database
 
-        def comment():
-            if self.is_owner:
+        column_type = database.sql_type(sql_type)
+        match = VARCHAR_SIZE_RE.match(sql_type)
+        field_size = int(match.group(1)) if match else None
+
+        def add_comment():
+            if comment and self.is_owner:
                 cursor.execute('COMMENT ON COLUMN "%s"."%s" IS \'%s\'' %
-                    (self.table_name, column_name, string.replace("'", "''")))
+                    (self.table_name, column_name, comment.replace("'", "''")))
         if self.column_exist(column_name):
             if (column_name in ('create_date', 'write_date')
                     and column_type[1].lower() != 'timestamp(6)'):
                 # Migrate dates from timestamp(0) to timestamp
                 cursor.execute('ALTER TABLE "' + self.table_name + '" '
                     'ALTER COLUMN "' + column_name + '" TYPE timestamp')
-            comment()
-            if not migrate:
-                return
+            add_comment()
             base_type = column_type[0].lower()
             if base_type != self._columns[column_name]['typname']:
                 if (self._columns[column_name]['typname'], base_type) in [
@@ -290,19 +297,15 @@ class TableHandler(TableHandlerInterface):
         column_type = column_type[1]
         cursor.execute('ALTER TABLE "%s" ADD COLUMN "%s" %s'
             % (self.table_name, column_name, column_type))
-        comment()
+        add_comment()
 
-        if column_format:
+        if default is not _NO_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:
-                default = None
-                if default_fun is not None:
-                    default = default_fun()
                 cursor.execute('UPDATE "' + self.table_name + '" '
-                    'SET "' + column_name + '" = %s',
-                    (column_format(default),))
+                    'SET "' + column_name + '" = %s', (default,))
 
         self._update_definitions(columns=True)
 

trytond/backend/sqlite/table.py

@@ -10,6 +10,8 @@ import warnings
 __all__ = ['TableHandler']
 
 logger = logging.getLogger(__name__)
+VARCHAR_SIZE_RE = re.compile('VARCHAR\(([0-9]+)\)')
+_NO_DEFAULT = object()
 
 
 class TableHandler(TableHandlerInterface):
@@ -107,8 +109,7 @@ class TableHandler(TableHandlerInterface):
                     in self._columns.iteritems():
                 if column == old_name:
                     column = new_name
-                new_table.add_raw_column(column, typname, False,
-                    field_size=size)
+                new_table._add_raw_column(column, typname, field_size=size)
             new_columns = new_table._columns.keys()
             old_columns = [x if x != old_name else new_name
                 for x in new_columns]
@@ -176,11 +177,19 @@ class TableHandler(TableHandlerInterface):
     def db_default(self, column_name, value):
         warnings.warn('Unable to set default on column with SQLite backend')
 
-    def add_raw_column(self, column_name, column_type, column_format,
-            default_fun=None, field_size=None, migrate=True, string=''):
+    def add_column(self, column_name, sql_type, default=_NO_DEFAULT,
+            comment=''):
+        database = Transaction().database
+        column_type = database.sql_type(sql_type)
+        match = VARCHAR_SIZE_RE.match(sql_type)
+        field_size = int(match.group(1)) if match else None
+
+        self._add_raw_column(column_name, column_type, default, field_size,
+            comment)
+
+    def _add_raw_column(self, column_name, column_type, default=_NO_DEFAULT,
+            field_size=None, string=''):
         if self.column_exist(column_name):
-            if not migrate:
-                return
             base_type = column_type[0].upper()
             if base_type != self._columns[column_name]['typname']:
                 if (self._columns[column_name]['typname'], base_type) in [
@@ -219,21 +228,16 @@ class TableHandler(TableHandlerInterface):
 
         cursor = Transaction().connection.cursor()
         column_type = column_type[1]
-        default = ''
-        cursor.execute(('ALTER TABLE "%s" ADD COLUMN "%s" %s' + default) %
+        cursor.execute(('ALTER TABLE "%s" ADD COLUMN "%s" %s') %
                        (self.table_name, column_name, column_type))
 
-        if column_format:
+        if default is not _NO_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:
-                default = None
-                if default_fun is not None:
-                    default = default_fun()
                 cursor.execute('UPDATE "' + self.table_name + '" '
-                    'SET "' + column_name + '" = ?',
-                    (column_format(default),))
+                    'SET "' + column_name + '" = ?', (default,))
 
         self._update_definitions(columns=True)
 
@@ -302,8 +306,7 @@ class TableHandler(TableHandlerInterface):
         for name, (notnull, hasdef, size, typname) \
                 in self._columns.iteritems():
             if name != column_name:
-                new_table.add_raw_column(name, typname, True,
-                    field_size=size)
+                new_table._add_raw_column(name, typname, field_size=size)
         columns_name = [x for x in new_table._columns.keys()]
         cursor.execute(('INSERT INTO "%s" (' +
                         ','.join('"%s"' % c for c in columns_name) +

trytond/backend/table.py

@@ -113,19 +113,15 @@ class TableHandlerInterface(object):
         '''
         raise NotImplementedError
 
-    def add_raw_column(self, column_name, column_type, column_format,
-            default_fun=None, field_size=None, migrate=True,
-            string=''):
+    def add_column(self, column_name, abstract_type, default=object(),
+            comment=''):
         '''
         Add a column
 
         :param column_name: the column name
-        :param column_type: the column definition
-        :param column_format: the function to format default value
-        :param default_fun: the function that return the default value
-        :param field_size: the size of the column if there is one
-        :param migrate: boolean to try to migrate the column if exists
-        :param string: the label of the column
+        :param abstract_type: the abstract type that will represent this column
+        :param default: the default value to use
+        :param comment: An optional comment on the column
         '''
         raise NotImplementedError
 

trytond/ir/translation.py

@@ -105,10 +105,7 @@ class Translation(ModelSQL, ModelView):
         # Migration from 1.8: new field src_md5
         src_md5_exist = table.column_exist('src_md5')
         if not src_md5_exist:
-            table.add_raw_column('src_md5',
-                cls.src_md5.sql_type(),
-                cls.src_md5.sql_format, None,
-                cls.src_md5.size, string=cls.src_md5.string)
+            table.add_column('src_md5', cls.src_md5._sql_type)
         table.drop_constraint('translation_uniq')
         table.index_action(['lang', 'type', 'name', 'src'], 'remove')
 

trytond/model/modelsql.py

@@ -149,34 +149,20 @@ class ModelSQL(ModelStorage):
             sql_type = field.sql_type()
             if not sql_type:
                 continue
-            default_fun = None
+
+            # 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 = {}
             if field_name in cls._defaults:
-                default_fun = cls._defaults[field_name]
+                default_ = cls._clean_defaults({
+                        field_name: cls._defaults[field_name](),
+                        })[field_name]
+                kwargs['default'] = field.sql_format(default_)
 
-                def unpack_wrapper(fun):
-                    def unpack_result(*a):
-                        try:
-                            # XXX ugly hack: some default fct try
-                            # to access the non-existing table
-                            result = fun(*a)
-                        except Exception:
-                            return None
-                        clean_results = cls._clean_defaults(
-                            {field_name: result})
-                        return clean_results[field_name]
-                    return unpack_result
-                default_fun = unpack_wrapper(default_fun)
-
-            if hasattr(field, 'size') and isinstance(field.size, int):
-                field_size = field.size
-            else:
-                field_size = None
-
-            table.add_raw_column(field_name, sql_type, field.sql_format,
-                default_fun, field_size, string=field.string)
+            table.add_column(field_name, field._sql_type, **kwargs)
             if cls._history:
-                history_table.add_raw_column(field_name, sql_type, None,
-                    string=field.string)
+                history_table.add_column(field_name, field._sql_type)
 
             if isinstance(field, (fields.Integer, fields.Float)):
                 # migration from tryton 2.2
@@ -255,16 +241,11 @@ class ModelSQL(ModelStorage):
     def _update_history_table(cls):
         TableHandler = backend.get('TableHandler')
         if cls._history:
-            table = TableHandler(cls)
             history_table = TableHandler(cls, history=True)
-            for column_name in table._columns:
-                string = ''
-                if column_name in cls._fields:
-                    string = cls._fields[column_name].string
-                history_table.add_raw_column(column_name,
-                    (table._columns[column_name]['typname'],
-                        table._columns[column_name]['typname']),
-                    None, string=string)
+            for field_name, field in cls._fields.iteritems():
+                if not field.sql_type():
+                    continue
+                history_table.add_column(field_name, field._sql_type)
 
     @classmethod
     def _get_error_messages(cls):