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.
--- 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