[schema] fix unique together index handling
authorAurelien Campeas <aurelien.campeas@logilab.fr>
Wed, 08 Jan 2014 12:09:44 +0100
changeset 9375 8e88576787c3
parent 9374 1236d9058ad3
child 9376 2ed0d091e9b1
[schema] fix unique together index handling We now provide a more compact indexname, using the schema constraint entity type and the position of the columns set in the entity type unique constraints list. This avoids a nasty name truncation issue. The UniqueTogetherError object is made smarter: it computes the rtypes, abstracting the underlying backend (pg/sqlserver vs sqlite). The `user friendly` adapter is much simplified since there is no longer any truncation issue. Uses a new logilab.database version (ticket #151507) and a new yams version (ticket #189299) Closes #2514939 [jcr: disable hooks when temporarily dropping CWUniqueTogetherConstraint entities]
__pkginfo__.py
_exceptions.py
cubicweb.spec
debian/control
doc/3.18.rst
entities/adapters.py
hooks/syncschema.py
misc/migration/3.18.0_Any.py
schemas/bootstrap.py
server/migractions.py
server/schemaserial.py
server/sources/native.py
server/test/unittest_querier.py
--- a/__pkginfo__.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/__pkginfo__.py	Wed Jan 08 12:09:44 2014 +0100
@@ -51,7 +51,7 @@
     'Twisted': '',
     # XXX graphviz
     # server dependencies
-    'logilab-database': '>= 1.10',
+    'logilab-database': '>= 1.11',
     'passlib': '',
     }
 
--- a/_exceptions.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/_exceptions.py	Wed Jan 08 12:09:44 2014 +0100
@@ -1,4 +1,4 @@
-# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -19,6 +19,10 @@
 
 __docformat__ = "restructuredtext en"
 
+from warnings import warn
+
+from logilab.common.decorators import cachedproperty
+
 from yams import ValidationError as ValidationError
 
 # abstract exceptions #########################################################
@@ -81,6 +85,26 @@
 
 class UniqueTogetherError(RepositoryError):
     """raised when a unique_together constraint caused an IntegrityError"""
+    def __init__(self, session, **kwargs):
+        self.session = session
+        assert 'rtypes' in kwargs or 'cstrname' in kwargs
+        self.kwargs = kwargs
+
+    @cachedproperty
+    def rtypes(self):
+        if 'rtypes' in self.kwargs:
+            return self.kwargs['rtypes']
+        cstrname = unicode(self.kwargs['cstrname'])
+        cstr = self.session.find('CWUniqueTogetherConstraint', name=cstrname).one()
+        return sorted(rtype.name for rtype in cstr.relations)
+
+    @cachedproperty
+    def args(self):
+        warn('[3.18] UniqueTogetherError.args is deprecated, just use '
+             'the .rtypes accessor.',
+             DeprecationWarning)
+        # the first argument, etype, is never used and was never garanteed anyway
+        return None, self.rtypes
 
 
 # security exceptions #########################################################
--- a/cubicweb.spec	Tue Jan 07 17:45:48 2014 +0100
+++ b/cubicweb.spec	Wed Jan 08 12:09:44 2014 +0100
@@ -24,7 +24,7 @@
 Requires:       %{python}-logilab-mtconverter >= 0.8.0
 Requires:       %{python}-rql >= 0.31.2
 Requires:       %{python}-yams >= 0.39.0
-Requires:       %{python}-logilab-database >= 1.10.0
+Requires:       %{python}-logilab-database >= 1.11.0
 Requires:       %{python}-passlib
 Requires:       %{python}-lxml
 Requires:       %{python}-twisted-web
--- a/debian/control	Tue Jan 07 17:45:48 2014 +0100
+++ b/debian/control	Wed Jan 08 12:09:44 2014 +0100
@@ -51,7 +51,7 @@
  ${python:Depends},
  cubicweb-common (= ${source:Version}),
  cubicweb-ctl (= ${source:Version}),
- python-logilab-database (>= 1.10.0),
+ python-logilab-database (>= 1.11.0),
  cubicweb-postgresql-support
  | cubicweb-mysql-support
  | python-pysqlite2,
--- a/doc/3.18.rst	Tue Jan 07 17:45:48 2014 +0100
+++ b/doc/3.18.rst	Wed Jan 08 12:09:44 2014 +0100
@@ -1,6 +1,9 @@
 What's new in CubicWeb 3.18?
 ============================
 
+The migration script does not handle sqlite nor mysql instances.
+
+
 New functionalities
 --------------------
 
@@ -28,6 +31,10 @@
   may have some consequences for applications that do low-level database
   manipulations or at times disable (some) hooks.
 
+* `unique together` constraints (multi-columns unicity constraints)
+  get a `name` attribute that maps the CubicWeb contraint entities to
+  corresponding backend index.
+
 
 Deprecation
 ---------------------
--- a/entities/adapters.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/entities/adapters.py	Wed Jan 08 12:09:44 2014 +0100
@@ -1,4 +1,4 @@
-# copyright 2010-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2010-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -362,23 +362,12 @@
     __select__ = match_exception(UniqueTogetherError)
 
     def raise_user_exception(self):
-        etype, rtypes = self.exc.args
-        # Because of index name size limits (e.g: postgres around 64,
-        # sqlserver around 128), we cannot be sure of what we got,
-        # especially for the rtypes part.
-        # Hence we will try to validate them, and handle invalid ones
-        # in the most user-friendly manner ...
         _ = self._cw._
-        schema = self.entity._cw.vreg.schema
+        rtypes = self.exc.rtypes
         rtypes_msg = {}
         for rtype in rtypes:
-            if rtype in schema:
-                rtypes_msg[rtype] = _('%s is part of violated unicity constraint') % rtype
-        globalmsg = _('some relations %sviolate a unicity constraint')
-        if len(rtypes) != len(rtypes_msg): # we got mangled/missing rtypes
-            globalmsg = globalmsg % _('(not all shown here) ')
-        else:
-            globalmsg = globalmsg % ''
+            rtypes_msg[rtype] = _('%s is part of violated unicity constraint') % rtype
+        globalmsg = _('some relations violate a unicity constraint')
         rtypes_msg['unicity constraint'] = globalmsg
         raise ValidationError(self.entity.eid, rtypes_msg)
 
--- a/hooks/syncschema.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/hooks/syncschema.py	Wed Jan 08 12:09:44 2014 +0100
@@ -714,44 +714,38 @@
 
 class CWUniqueTogetherConstraintAddOp(MemSchemaOperation):
     entity = None # make pylint happy
+
     def precommit_event(self):
         session = self.session
         prefix = SQL_PREFIX
-        table = '%s%s' % (prefix, self.entity.constraint_of[0].name)
-        cols = ['%s%s' % (prefix, r.name) for r in self.entity.relations]
-        dbhelper= session.cnxset.source('system').dbhelper
-        sqls = dbhelper.sqls_create_multicol_unique_index(table, cols)
+        entity = self.entity
+        table = '%s%s' % (prefix, entity.constraint_of[0].name)
+        cols = ['%s%s' % (prefix, r.name) for r in entity.relations]
+        dbhelper = session.cnxset.source('system').dbhelper
+        sqls = dbhelper.sqls_create_multicol_unique_index(table, cols, entity.name)
         for sql in sqls:
             session.system_sql(sql)
 
-    # XXX revertprecommit_event
-
     def postcommit_event(self):
-        eschema = self.session.vreg.schema.schema_by_eid(self.entity.constraint_of[0].eid)
-        attrs = [r.name for r in self.entity.relations]
+        entity = self.entity
+        eschema = self.session.vreg.schema.schema_by_eid(entity.constraint_of[0].eid)
+        attrs = [r.name for r in entity.relations]
         eschema._unique_together.append(attrs)
 
 
 class CWUniqueTogetherConstraintDelOp(MemSchemaOperation):
-    entity = oldcstr = None # for pylint
-    cols = [] # for pylint
+    entity = cstrname = None # for pylint
+    cols = () # for pylint
+
     def precommit_event(self):
         session = self.session
         prefix = SQL_PREFIX
         table = '%s%s' % (prefix, self.entity.type)
-        dbhelper= session.cnxset.source('system').dbhelper
+        dbhelper = session.cnxset.source('system').dbhelper
         cols = ['%s%s' % (prefix, c) for c in self.cols]
-        sqls = dbhelper.sqls_drop_multicol_unique_index(table, cols)
+        sqls = dbhelper.sqls_drop_multicol_unique_index(table, cols, self.cstrname)
         for sql in sqls:
-            try:
-                session.system_sql(sql)
-            except Exception as exc: # should be ProgrammingError
-                if sql.startswith('DROP'):
-                    self.error('execute of `%s` failed (cause: %s)', sql, exc)
-                    continue
-                raise
-
-    # XXX revertprecommit_event
+            session.system_sql(sql)
 
     def postcommit_event(self):
         eschema = self.session.vreg.schema.schema_by_eid(self.entity.eid)
@@ -1171,9 +1165,9 @@
         schema = self._cw.vreg.schema
         cstr = self._cw.entity_from_eid(self.eidfrom)
         entity = schema.schema_by_eid(self.eidto)
-        cols = [r.name for r in cstr.relations]
+        cols = tuple(r.name for r in cstr.relations)
         CWUniqueTogetherConstraintDelOp(self._cw, entity=entity,
-                                        oldcstr=cstr, cols=cols)
+                                        cstrname=cstr.name, cols=cols)
 
 
 # permissions synchronization hooks ############################################
--- a/misc/migration/3.18.0_Any.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/misc/migration/3.18.0_Any.py	Wed Jan 08 12:09:44 2014 +0100
@@ -1,3 +1,9 @@
+driver = config.sources()['system']['db-driver']
+if not (driver == 'postgres' or driver.startswith('sqlserver')):
+    import sys
+    print >>sys.stderr, 'This migration is not supported for backends other than sqlserver or postgres (yet).'
+    sys.exit(1)
+
 sync_schema_props_perms('defaultval')
 
 def convert_defaultval(cwattr, default):
@@ -31,47 +37,23 @@
     return Binary.zpickle(default)
 
 dbh = repo.system_source.dbhelper
-driver = config.sources()['system']['db-driver']
 
-if driver == 'postgres' or driver.startswith('sqlserver'):
-
-    sql('ALTER TABLE cw_cwattribute ADD new_defaultval %s' % dbh.TYPE_MAPPING['Bytes'])
 
-    for cwattr in rql('CWAttribute X').entities():
-        olddefault = cwattr.defaultval
-        if olddefault is not None:
-            req = "UPDATE cw_cwattribute SET new_defaultval = %(val)s WHERE cw_eid = %(eid)s"
-            args = {'val': dbh.binary_value(convert_defaultval(cwattr, olddefault).getvalue()), 'eid': cwattr.eid}
-            sql(req, args, ask_confirm=False)
+sql('ALTER TABLE cw_cwattribute ADD new_defaultval %s' % dbh.TYPE_MAPPING['Bytes'])
 
-    sql('ALTER TABLE cw_cwattribute DROP COLUMN cw_defaultval')
-    if config.sources()['system']['db-driver'] == 'postgres':
-        sql('ALTER TABLE cw_cwattribute RENAME COLUMN new_defaultval TO cw_defaultval')
-    else:
-        sql("sp_rename 'cw_cwattribute.new_defaultval', 'cw_defaultval', 'COLUMN'")
-
-elif driver == 'sqlite':
-
-    import re
-    create = sql("SELECT sql FROM sqlite_master WHERE name = 'cw_CWAttribute'")[0][0]
-    create = re.sub('cw_defaultval varchar[^,]*,', 'cw_defaultval bytea,', create, re.I)
-    create = re.sub('cw_CWAttribute', 'tmp_cw_CWAttribute', create, re.I)
-    sql(create)
-    sql("INSERT INTO tmp_cw_CWAttribute SELECT * FROM cw_CWAttribute")
-    for cwattr in rql('CWAttribute X').entities():
-        olddefault = cwattr.defaultval
-        if olddefault is None:
-            continue
-        req = "UPDATE tmp_cw_CWAttribute SET cw_defaultval = %(val)s WHERE cw_eid = %(eid)s"
-        args = {'val': dbh.binary_value(convert_defaultval(cwattr, olddefault).getvalue()),
-                'eid': cwattr.eid}
+for cwattr in rql('CWAttribute X').entities():
+    olddefault = cwattr.defaultval
+    if olddefault is not None:
+        req = "UPDATE cw_cwattribute SET new_defaultval = %(val)s WHERE cw_eid = %(eid)s"
+        args = {'val': dbh.binary_value(convert_defaultval(cwattr, olddefault).getvalue()), 'eid': cwattr.eid}
         sql(req, args, ask_confirm=False)
 
-    sql('DROP TABLE cw_CWAttribute')
-    sql('ALTER TABLE tmp_cw_CWAttribute RENAME TO cw_CWAttribute')
+sql('ALTER TABLE cw_cwattribute DROP COLUMN cw_defaultval')
+if driver == 'postgres':
+    sql('ALTER TABLE cw_cwattribute RENAME COLUMN new_defaultval TO cw_defaultval')
+else: # sqlserver
+    sql("sp_rename 'cw_cwattribute.new_defaultval', 'cw_defaultval', 'COLUMN'")
 
-else:
-    assert False, 'upgrade not supported on this database backend'
 
 # Set object type to "Bytes" for CWAttribute's "defaultval" attribute
 rql('SET X to_entity B WHERE X is CWAttribute, X from_entity Y, Y name "CWAttribute", '
@@ -83,7 +65,6 @@
 
 commit()
 
-
 for rschema in schema.relations():
     if rschema.symmetric:
         subjects = set(repr(e.type) for e in rschema.subjects())
@@ -100,3 +81,40 @@
         with session.deny_all_hooks_but():
             rql('SET X %(r)s Y WHERE Y %(r)s X, NOT X %(r)s Y' % {'r': rschema.type})
     commit()
+
+
+# multi columns unique constraints regeneration
+from cubicweb.server import schemaserial
+
+# syncschema hooks would try to remove indices but
+# 1) we already do that below
+# 2) the hook expects the CWUniqueTogetherConstraint.name attribute that hasn't
+#    yet been added
+with session.allow_all_hooks_but('syncschema'):
+    rql('DELETE CWUniqueTogetherConstraint C')
+commit()
+
+add_attribute('CWUniqueTogetherConstraint', 'name')
+
+# low-level wipe code for postgres & sqlserver, plain sql ...
+if driver == 'postgres':
+    for indexname, in sql('select indexname from pg_indexes'):
+        if indexname.startswith('unique_'):
+            print 'dropping index', indexname
+            sql('DROP INDEX %s' % indexname)
+    commit()
+elif driver.startswith('sqlserver'):
+    for viewname, in sql('select name from sys.views'):
+        if viewname.startswith('utv_'):
+            print 'dropping view (index should be cascade-deleted)', viewname
+            sql('DROP VIEW %s' % viewname)
+    commit()
+
+# recreate the constraints, hook will lead to low-level recreation
+for eschema in sorted(schema.entities()):
+    if eschema._unique_together:
+        rql_args = schemaserial.uniquetogether2rqls(eschema)
+        for rql, args in rql_args:
+            args['x'] = eschema.eid
+            session.execute(rql, args)
+        commit()
--- a/schemas/bootstrap.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/schemas/bootstrap.py	Wed Jan 08 12:09:44 2014 +0100
@@ -158,6 +158,7 @@
 class CWUniqueTogetherConstraint(EntityType):
     """defines a sql-level multicolumn unique index"""
     __permissions__ = PUB_SYSTEM_ENTITY_PERMS
+    name = String(required=True, unique=True, maxsize=64)
     constraint_of = SubjectRelation('CWEType', cardinality='1*', composite='object',
                                     inlined=True)
     relations = SubjectRelation('CWRType', cardinality='+*',
--- a/server/migractions.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/server/migractions.py	Wed Jan 08 12:09:44 2014 +0100
@@ -44,7 +44,7 @@
 from logilab.common.decorators import cached, clear_cache
 
 from yams.constraints import SizeConstraint
-from yams.schema2sql import eschema2sql, rschema2sql
+from yams.schema2sql import eschema2sql, rschema2sql, unique_index_name
 from yams.schema import RelationDefinitionSchema
 
 from cubicweb import CW_SOFTWARE_ROOT, AuthenticationError, ExecutionError
@@ -559,39 +559,41 @@
                         self._synchronize_rdef_schema(subj, rschema, obj,
                                                       syncprops=syncprops, syncperms=syncperms)
         if syncprops: # need to process __unique_together__ after rdefs were processed
-            repo_unique_together = set([frozenset(ut)
-                                        for ut in repoeschema._unique_together])
-            unique_together = set([frozenset(ut)
-                                   for ut in eschema._unique_together])
-            for ut in repo_unique_together - unique_together:
-                restrictions = []
-                substs = {'x': repoeschema.eid}
-                for i, col in enumerate(ut):
-                    restrictions.append('C relations T%(i)d, '
-                                       'T%(i)d name %%(T%(i)d)s' % {'i': i})
-                    substs['T%d'%i] = col
-                self.rqlexec('DELETE CWUniqueTogetherConstraint C '
-                             'WHERE C constraint_of E, '
-                             '      E eid %%(x)s,'
-                             '      %s' % ', '.join(restrictions),
-                             substs)
-            def possible_unique_constraint(ut):
-                for name in ut:
+            # mappings from constraint name to columns
+            # filesystem (fs) and repository (repo) wise
+            fs = {}
+            repo = {}
+            for cols in eschema._unique_together or ():
+                fs[unique_index_name(repoeschema, cols)] = sorted(cols)
+            schemaentity = self.session.entity_from_eid(repoeschema.eid)
+            for entity in schemaentity.related('constraint_of', 'object',
+                                               targettypes=('CWUniqueTogetherConstraint',)).entities():
+                repo[entity.name] = sorted(rel.name for rel in entity.relations)
+            added = set(fs) - set(repo)
+            removed = set(repo) - set(fs)
+
+            for name in removed:
+                self.rqlexec('DELETE CWUniqueTogetherConstraint C WHERE C name %(name)s',
+                             {'name': name})
+
+            def possible_unique_constraint(cols):
+                for name in cols:
                     rschema = repoeschema.subjrels.get(name)
                     if rschema is None:
                         print 'dont add %s unique constraint on %s, missing %s' % (
-                            ','.join(ut), eschema, name)
+                            ','.join(cols), eschema, name)
                         return False
                     if not (rschema.final or rschema.inlined):
-                        (eschema, name)
                         print 'dont add %s unique constraint on %s, %s is neither final nor inlined' % (
-                            ','.join(ut), eschema, name)
+                            ','.join(cols), eschema, name)
                         return False
                 return True
-            for ut in unique_together - repo_unique_together:
-                if possible_unique_constraint(ut):
-                    rql, substs = ss.uniquetogether2rql(eschema, ut)
+
+            for name in added:
+                if possible_unique_constraint(fs[name]):
+                    rql, substs = ss._uniquetogether2rql(eschema, fs[name])
                     substs['x'] = repoeschema.eid
+                    substs['name'] = name
                     self.rqlexec(rql, substs)
 
     def _synchronize_rdef_schema(self, subjtype, rtype, objtype,
--- a/server/schemaserial.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/server/schemaserial.py	Wed Jan 08 12:09:44 2014 +0100
@@ -25,7 +25,8 @@
 
 from logilab.common.shellutils import ProgressBar
 
-from yams import BadSchemaDefinition, schema as schemamod, buildobjs as ybo
+from yams import (BadSchemaDefinition, schema as schemamod, buildobjs as ybo,
+                  schema2sql as y2sql)
 
 from cubicweb import CW_SOFTWARE_ROOT, Binary, typed_eid
 from cubicweb.schema import (KNOWN_RPROPERTIES, CONSTRAINTS, ETYPE_NAME_MAP,
@@ -367,8 +368,8 @@
             pb.update()
     # serialize unique_together constraints
     for eschema in eschemas:
-        for unique_together in eschema._unique_together:
-            execschemarql(execute, eschema, [uniquetogether2rql(eschema, unique_together)])
+        if eschema._unique_together:
+            execschemarql(execute, eschema, uniquetogether2rqls(eschema))
     # serialize yams inheritance relationships
     for rql, kwargs in specialize2rql(schema):
         execute(rql, kwargs, build_descr=False)
@@ -427,7 +428,15 @@
         values = {'x': eschema.eid, 'et': specialized_type.eid}
         yield 'SET X specializes ET WHERE X eid %(x)s, ET eid %(et)s', values
 
-def uniquetogether2rql(eschema, unique_together):
+def uniquetogether2rqls(eschema):
+    rql_args = []
+    for columns in eschema._unique_together:
+        rql, args = _uniquetogether2rql(eschema, columns)
+        args['name'] = y2sql.unique_index_name(eschema, columns)
+        rql_args.append((rql, args))
+    return rql_args
+
+def _uniquetogether2rql(eschema, unique_together):
     relations = []
     restrictions = []
     substs = {}
@@ -439,10 +448,8 @@
         restrictions.append('%(rtype)s name %%(%(rtype)s)s' % {'rtype': rtype})
     relations = ', '.join(relations)
     restrictions = ', '.join(restrictions)
-    rql = ('INSERT CWUniqueTogetherConstraint C: '
-           '    C constraint_of X, %s  '
-           'WHERE '
-           '    X eid %%(x)s, %s')
+    rql = ('INSERT CWUniqueTogetherConstraint C: C name %%(name)s, C constraint_of X, %s '
+           'WHERE X eid %%(x)s, %s')
     return rql % (relations, restrictions), substs
 
 
--- a/server/sources/native.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/server/sources/native.py	Wed Jan 08 12:09:44 2014 +0100
@@ -757,24 +757,17 @@
             if ex.__class__.__name__ == 'IntegrityError':
                 # need string comparison because of various backends
                 for arg in ex.args:
-                    if 'SQL Server' in arg:
-                        mo = re.search("'unique_cw_[^ ]+'", arg)
-                    else: # postgres
-                        mo = re.search('"unique_cw_[^ ]+"', arg)
+                    # postgres, sqlserver
+                    mo = re.search("unique_[a-z0-9]{32}", arg)
                     if mo is not None:
-                        index_name = mo.group(0)[1:-1] # eat the surrounding " pair
-                        elements = index_name.split('_cw_')[1:]
-                        etype = elements[0]
-                        rtypes = elements[1:]
-                        raise UniqueTogetherError(etype, rtypes)
+                        raise UniqueTogetherError(session, cstrname=mo.group(0))
                     # sqlite
                     mo = re.search('columns (.*) are not unique', arg)
                     if mo is not None: # sqlite in use
                         # we left chop the 'cw_' prefix of attribute names
                         rtypes = [c.strip()[3:]
                                   for c in mo.group(1).split(',')]
-                        etype = '???'
-                        raise UniqueTogetherError(etype, rtypes)
+                        raise UniqueTogetherError(session, rtypes=rtypes)
             raise
         return cursor
 
--- a/server/test/unittest_querier.py	Tue Jan 07 17:45:48 2014 +0100
+++ b/server/test/unittest_querier.py	Wed Jan 08 12:09:44 2014 +0100
@@ -576,7 +576,7 @@
         self.assertListEqual(rset.rows,
                               [[u'description_format', 12],
                                [u'description', 13],
-                               [u'name', 16],
+                               [u'name', 17],
                                [u'created_by', 43],
                                [u'creation_date', 43],
                                [u'cw_source', 43],