symmetric relations: replace bogus rql2sql translation by a hook
authorAurelien Campeas <aurelien.campeas@logilab.fr>
Thu, 14 Nov 2013 17:17:02 +0100
changeset 9361 0542a85fe667
parent 9360 eda5071e30a1
child 9362 2f0129d17fa9
symmetric relations: replace bogus rql2sql translation by a hook The hook ensures X r Y => Y r X iff r is symmetric. The rql-no-hook data importer receives a small amendment but note that: * there exist no test for it * its actual semantics are undefined Hence we cannot prove this hunk breaks nothing, because we cannot prove anything. Closes #3259713.
dataimport.py
doc/3.18.rst
hooks/integrity.py
hooks/test/data/schema.py
hooks/test/unittest_hooks.py
misc/migration/3.18.0_Any.py
server/repository.py
server/rqlannotation.py
server/sources/rql2sql.py
server/test/unittest_rql2sql.py
--- a/dataimport.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/dataimport.py	Thu Nov 14 17:17:02 2013 +0100
@@ -802,6 +802,9 @@
         assert not rtype.startswith('reverse_')
         self.add_relation(self.session, eid_from, rtype, eid_to,
                           self.rschema(rtype).inlined)
+        if self.rschema[rtype].symmetric:
+            self.add_relation(self.session, eid_to, rtype, eid_from,
+                              self.rschema(rtype).inlined)
         self._nb_inserted_relations += 1
 
     @property
@@ -928,6 +931,9 @@
         # XXX Could subjtype be inferred ?
         self.source.add_relation(self.session, subj_eid, rtype, obj_eid,
                                  self.rschema(rtype).inlined, **kwargs)
+        if self.rschema[rtype].symmetric:
+            self.source.add_relation(self.session, obj_eid, rtype, subj_eid,
+                                     self.rschema(rtype).inlined, **kwargs)
 
     def drop_indexes(self, etype):
         """Drop indexes for a given entity type"""
--- a/doc/3.18.rst	Thu Dec 12 14:25:24 2013 +0100
+++ b/doc/3.18.rst	Thu Nov 14 17:17:02 2013 +0100
@@ -23,6 +23,12 @@
   numpy arrays, and fixes a bug where default values whose truth value
   was False were not properly migrated.
 
+* `symmetric` relations are no more handled by an rql rewrite but are
+  now handled with hooks (from the `activeintegrity` category); this
+  may have some consequences for applications that do low-level database
+  manipulations or at times disable (some) hooks.
+
+
 Deprecation
 ---------------------
 
--- a/hooks/integrity.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/hooks/integrity.py	Thu Nov 14 17:17:02 2013 +0100
@@ -109,6 +109,30 @@
     category = 'integrity'
 
 
+class EnsureSymmetricRelationsAdd(hook.Hook):
+    """ ensure X r Y => Y r X iff r is symmetric """
+    __regid__ = 'cw.add_ensure_symmetry'
+    category = 'activeintegrity'
+    events = ('after_add_relation',)
+    # __select__ is set in the registration callback
+
+    def __call__(self):
+        self._cw.repo.system_source.add_relation(self._cw, self.eidto,
+                                                 self.rtype, self.eidfrom)
+
+
+class EnsureSymmetricRelationsDelete(hook.Hook):
+    """ ensure X r Y => Y r X iff r is symmetric """
+    __regid__ = 'cw.delete_ensure_symmetry'
+    category = 'activeintegrity'
+    events = ('after_delete_relation',)
+    # __select__ is set in the registration callback
+
+    def __call__(self):
+        self._cw.repo.system_source.delete_relation(self._cw, self.eidto,
+                                                    self.rtype, self.eidfrom)
+
+
 class CheckCardinalityHookBeforeDeleteRelation(IntegrityHook):
     """check cardinalities are satisfied"""
     __regid__ = 'checkcard_before_delete_relation'
@@ -348,3 +372,11 @@
         elif composite == 'object':
             _DelayedDeleteSEntityOp.get_instance(self._cw).add_data(
                 (self.eidfrom, rtype))
+
+def registration_callback(vreg):
+    vreg.register_all(globals().values(), __name__)
+    symmetric_rtypes = [rschema.type for rschema in vreg.schema.relations()
+                        if rschema.symmetric]
+    EnsureSymmetricRelationsAdd.__select__ = hook.Hook.__select__ & hook.match_rtype(*symmetric_rtypes)
+    EnsureSymmetricRelationsDelete.__select__ = hook.Hook.__select__ & hook.match_rtype(*symmetric_rtypes)
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hooks/test/data/schema.py	Thu Nov 14 17:17:02 2013 +0100
@@ -0,0 +1,25 @@
+# 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.
+#
+# CubicWeb is free software: you can redistribute it and/or modify it under the
+# terms of the GNU Lesser General Public License as published by the Free
+# Software Foundation, either version 2.1 of the License, or (at your option)
+# any later version.
+#
+# CubicWeb is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Lesser General Public License along
+# with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
+
+from yams.buildobjs import RelationDefinition
+
+class friend(RelationDefinition):
+    subject = ('CWUser', 'CWGroup')
+    object = ('CWUser', 'CWGroup')
+    symmetric = True
+
--- a/hooks/test/unittest_hooks.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/hooks/test/unittest_hooks.py	Thu Nov 14 17:17:02 2013 +0100
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# 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.
@@ -39,6 +39,37 @@
         rset = self.execute('Any S WHERE X sender S, X eid %s' % eeid)
         self.assertEqual(len(rset), 1)
 
+    def test_symmetric(self):
+        req = self.request()
+        u1 = self.create_user(req, u'1')
+        u2 = self.create_user(req, u'2')
+        u3 = self.create_user(req, u'3')
+        ga = req.create_entity('CWGroup', name=u'A')
+        gb = req.create_entity('CWGroup', name=u'B')
+        u1.cw_set(friend=u2)
+        u2.cw_set(friend=u3)
+        ga.cw_set(friend=gb)
+        ga.cw_set(friend=u1)
+        self.commit()
+        req = self.request()
+        for l1, l2 in ((u'1', u'2'),
+                       (u'2', u'3')):
+            self.assertTrue(req.execute('Any U1,U2 WHERE U1 friend U2, U1 login %(l1)s, U2 login %(l2)s',
+                                        {'l1': l1, 'l2': l2}))
+            self.assertTrue(req.execute('Any U1,U2 WHERE U2 friend U1, U1 login %(l1)s, U2 login %(l2)s',
+                                        {'l1': l1, 'l2': l2}))
+        self.assertTrue(req.execute('Any GA,GB WHERE GA friend GB, GA name "A", GB name "B"'))
+        self.assertTrue(req.execute('Any GA,GB WHERE GB friend GA, GA name "A", GB name "B"'))
+        self.assertTrue(req.execute('Any GA,U1 WHERE GA friend U1, GA name "A", U1 login "1"'))
+        self.assertTrue(req.execute('Any GA,U1 WHERE U1 friend GA, GA name "A", U1 login "1"'))
+        self.assertFalse(req.execute('Any GA,U WHERE GA friend U, GA name "A", U login "2"'))
+        for l1, l2 in ((u'1', u'3'),
+                       (u'3', u'1')):
+            self.assertFalse(req.execute('Any U1,U2 WHERE U1 friend U2, U1 login %(l1)s, U2 login %(l2)s',
+                                         {'l1': l1, 'l2': l2}))
+            self.assertFalse(req.execute('Any U1,U2 WHERE U2 friend U1, U1 login %(l1)s, U2 login %(l2)s',
+                                         {'l1': l1, 'l2': l2}))
+
     def test_html_tidy_hook(self):
         req = self.request()
         entity = req.create_entity('Workflow', name=u'wf1',
--- a/misc/migration/3.18.0_Any.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/misc/migration/3.18.0_Any.py	Thu Nov 14 17:17:02 2013 +0100
@@ -82,3 +82,10 @@
 schema.del_relation_def('CWAttribute', 'defaultval', 'String')
 
 commit()
+
+
+for rschema in schema.relations():
+    if rschema.symmetric:
+        with session.allow_all_hooks_but('activeintegrity'):
+            rql('SET X %(r)s Y WHERE Y %(r)s X, NOT X %(r)s Y' % {'r': rschema.type})
+    commit()
--- a/server/repository.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/server/repository.py	Thu Nov 14 17:17:02 2013 +0100
@@ -1595,10 +1595,6 @@
         source.delete_relation(session, subject, rtype, object)
         rschema = self.schema.rschema(rtype)
         session.update_rel_cache_del(subject, rtype, object, rschema.symmetric)
-        if rschema.symmetric:
-            # on symmetric relation, we can't now in which sense it's
-            # stored so try to delete both
-            source.delete_relation(session, object, rtype, subject)
         if source.should_call_hooks:
             self.hm.call_hooks('after_delete_relation', session,
                                eidfrom=subject, rtype=rtype, eidto=object)
--- a/server/rqlannotation.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/server/rqlannotation.py	Thu Nov 14 17:17:02 2013 +0100
@@ -130,8 +130,6 @@
                     # can use N.ecrit_par as principal
                     if (stinfo['selected'] or len(stinfo['relations']) > 1):
                         break
-                elif rschema.symmetric and stinfo['selected']:
-                    break
             joins.add( (rel, role) )
         else:
             # if there is at least one ambigous relation and no other to
--- a/server/sources/rql2sql.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/server/sources/rql2sql.py	Thu Nov 14 17:17:02 2013 +0100
@@ -242,12 +242,6 @@
         rhsconst = None # ColumnAlias
     return lhs, lhsconst, rhs, rhsconst
 
-def switch_relation_field(sql, table=''):
-    switchedsql = sql.replace(table + '.eid_from', '__eid_from__')
-    switchedsql = switchedsql.replace(table + '.eid_to',
-                                      table + '.eid_from')
-    return switchedsql.replace('__eid_from__', table + '.eid_to')
-
 def sort_term_selection(sorts, rqlst, groups):
     # XXX beurk
     if isinstance(rqlst, list):
@@ -1132,8 +1126,6 @@
         sqls += self._process_relation_term(relation, rid, lhsvar, lhsconst, 'eid_from')
         sqls += self._process_relation_term(relation, rid, rhsvar, rhsconst, 'eid_to')
         sql = ' AND '.join(sqls)
-        if rschema.symmetric:
-            sql = '(%s OR %s)' % (sql, switch_relation_field(sql))
         return sql
 
     def _visit_outer_join_relation(self, relation, rschema):
--- a/server/test/unittest_rql2sql.py	Thu Dec 12 14:25:24 2013 +0100
+++ b/server/test/unittest_rql2sql.py	Thu Nov 14 17:17:02 2013 +0100
@@ -1070,68 +1070,6 @@
 FROM cw_Personne AS _P'''),
     ]
 
-SYMMETRIC = [
-    ('Any P WHERE X eid 0, X connait P',
-     '''SELECT DISTINCT _P.cw_eid
-FROM connait_relation AS rel_connait0, cw_Personne AS _P
-WHERE (rel_connait0.eid_from=0 AND rel_connait0.eid_to=_P.cw_eid OR rel_connait0.eid_to=0 AND rel_connait0.eid_from=_P.cw_eid)'''
-     ),
-
-    ('Any P WHERE X connait P',
-    '''SELECT DISTINCT _P.cw_eid
-FROM connait_relation AS rel_connait0, cw_Personne AS _P
-WHERE (rel_connait0.eid_to=_P.cw_eid OR rel_connait0.eid_from=_P.cw_eid)'''
-    ),
-
-    ('Any X WHERE X connait P',
-    '''SELECT DISTINCT _X.cw_eid
-FROM connait_relation AS rel_connait0, cw_Personne AS _X
-WHERE (rel_connait0.eid_from=_X.cw_eid OR rel_connait0.eid_to=_X.cw_eid)'''
-     ),
-
-    ('Any P WHERE X eid 0, NOT X connait P',
-     '''SELECT _P.cw_eid
-FROM cw_Personne AS _P
-WHERE NOT (EXISTS(SELECT 1 FROM connait_relation AS rel_connait0 WHERE (rel_connait0.eid_from=0 AND rel_connait0.eid_to=_P.cw_eid OR rel_connait0.eid_to=0 AND rel_connait0.eid_from=_P.cw_eid)))'''),
-
-    ('Any P WHERE NOT X connait P',
-    '''SELECT _P.cw_eid
-FROM cw_Personne AS _P
-WHERE NOT (EXISTS(SELECT 1 FROM connait_relation AS rel_connait0 WHERE (rel_connait0.eid_to=_P.cw_eid OR rel_connait0.eid_from=_P.cw_eid)))'''),
-
-    ('Any X WHERE NOT X connait P',
-    '''SELECT _X.cw_eid
-FROM cw_Personne AS _X
-WHERE NOT (EXISTS(SELECT 1 FROM connait_relation AS rel_connait0 WHERE (rel_connait0.eid_from=_X.cw_eid OR rel_connait0.eid_to=_X.cw_eid)))'''),
-
-    ('Any P WHERE X connait P, P nom "nom"',
-     '''SELECT DISTINCT _P.cw_eid
-FROM connait_relation AS rel_connait0, cw_Personne AS _P
-WHERE (rel_connait0.eid_to=_P.cw_eid OR rel_connait0.eid_from=_P.cw_eid) AND _P.cw_nom=nom'''),
-
-    ('Any X WHERE X connait P, P nom "nom"',
-     '''SELECT DISTINCT _X.cw_eid
-FROM connait_relation AS rel_connait0, cw_Personne AS _P, cw_Personne AS _X
-WHERE (rel_connait0.eid_from=_X.cw_eid AND rel_connait0.eid_to=_P.cw_eid OR rel_connait0.eid_to=_X.cw_eid AND rel_connait0.eid_from=_P.cw_eid) AND _P.cw_nom=nom'''
-    ),
-
-    ('DISTINCT Any P WHERE P connait S OR S connait P, S nom "chouette"',
-     '''SELECT DISTINCT _P.cw_eid
-FROM connait_relation AS rel_connait0, cw_Personne AS _P, cw_Personne AS _S
-WHERE (rel_connait0.eid_from=_P.cw_eid AND rel_connait0.eid_to=_S.cw_eid OR rel_connait0.eid_to=_P.cw_eid AND rel_connait0.eid_from=_S.cw_eid) AND _S.cw_nom=chouette'''
-     )
-    ]
-
-SYMMETRIC_WITH_LIMIT = [
-        ('Any X ORDERBY X DESC LIMIT 9 WHERE E eid 0, E connait X',
-    '''SELECT DISTINCT _X.cw_eid
-FROM connait_relation AS rel_connait0, cw_Personne AS _X
-WHERE (rel_connait0.eid_from=0 AND rel_connait0.eid_to=_X.cw_eid OR rel_connait0.eid_to=0 AND rel_connait0.eid_from=_X.cw_eid)
-ORDER BY 1 DESC
-LIMIT 9'''
-     ),
-]
-
 INLINE = [
 
     ('Any P WHERE N eid 1, N ecrit_par P, NOT P owned_by P2',
@@ -1578,10 +1516,6 @@
         rqlst = self._prepare(rql)
         self.assertRaises(BadRQLQuery, self.o.generate, rqlst)
 
-    def test_symmetric(self):
-        for t in self._parse(SYMMETRIC + SYMMETRIC_WITH_LIMIT):
-            yield t
-
     def test_inline(self):
         for t in self._parse(INLINE):
             yield t
@@ -1806,10 +1740,6 @@
                     '''SELECT DATEPART(WEEKDAY, _P.cw_creation_date)
 FROM cw_Personne AS _P''')
 
-    def test_symmetric(self):
-        for t in self._parse(SYMMETRIC):
-            yield t
-
     def test_basic_parse(self):
         for t in self._parse(BASIC):# + BASIC_WITH_LIMIT):
             yield t