[rql2sql] fix bug avoiding outer join relation to be used as a variable principal. Closes #1659395 stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Tue, 10 May 2011 18:50:13 +0200
branchstable
changeset 7357 5ad3154a8810
parent 7354 f627ab500fda
child 7359 40490b9e0a6e
child 7362 b9813c9d32ac
[rql2sql] fix bug avoiding outer join relation to be used as a variable principal. Closes #1659395
devtools/repotest.py
server/rqlannotation.py
server/sources/rql2sql.py
server/test/unittest_rql2sql.py
--- a/devtools/repotest.py	Mon May 02 20:41:46 2011 +0200
+++ b/devtools/repotest.py	Tue May 10 18:50:13 2011 +0200
@@ -371,8 +371,13 @@
 _orig_select_principal = rqlannotation._select_principal
 
 def _select_principal(scope, relations):
+    def sort_key(something):
+        try:
+            return something.r_type
+        except AttributeError:
+            return (something[0].r_type, something[1])
     return _orig_select_principal(scope, relations,
-                                  _sort=lambda rels: sorted(rels, key=lambda x: x.r_type))
+                                  _sort=lambda rels: sorted(rels, key=sort_key))
 
 try:
     from cubicweb.server.msplanner import PartPlanInformation
--- a/server/rqlannotation.py	Mon May 02 20:41:46 2011 +0200
+++ b/server/rqlannotation.py	Tue May 10 18:50:13 2011 +0200
@@ -78,18 +78,19 @@
                 continue
             lhs, rhs = rel.get_parts()
             onlhs = ref is lhs
+            role = 'subject' if onlhs else 'object'
             if rel.r_type == 'eid':
                 if not (onlhs and len(stinfo['relations']) > 1):
                     break
                 if not stinfo['constnode']:
-                    joins.add(rel)
+                    joins.add( (rel, role) )
                 continue
             elif rel.r_type == 'identity':
                 # identity can't be used as principal, so check other relation are used
                 # XXX explain rhs.operator == '='
                 if rhs.operator != '=' or len(stinfo['relations']) <= 1: #(stinfo['constnode'] and rhs.operator == '='):
                     break
-                joins.add(rel)
+                joins.add( (rel, role) )
                 continue
             rschema = getrschema(rel.r_type)
             if rel.optional:
@@ -116,7 +117,7 @@
                     # need join anyway if the variable appears in a final or
                     # inlined relation
                     break
-                joins.add(rel)
+                joins.add( (rel, role) )
                 continue
             if not stinfo['constnode']:
                 if rschema.inlined and rel.neged(strict=True):
@@ -129,7 +130,7 @@
                         break
                 elif rschema.symmetric and stinfo['selected']:
                     break
-            joins.add(rel)
+            joins.add( (rel, role) )
         else:
             # if there is at least one ambigous relation and no other to
             # restrict types, can't be invariant since we need to filter out
@@ -169,10 +170,15 @@
     diffscope_rels = {}
     ored_rels = set()
     diffscope_rels = set()
-    for rel in _sort(relations):
+    for rel, role in _sort(relations):
         # note: only eid and has_text among all final relations may be there
         if rel.r_type in ('eid', 'identity'):
             continue
+        if rel.optional is not None and len(relations) > 1:
+            if role == 'subject' and rel.optional == 'right':
+                continue
+            if role == 'object' and rel.optional == 'left':
+                continue
         if rel.ored(traverse_scope=True):
             ored_rels.add(rel)
         elif rel.scope is scope:
--- a/server/sources/rql2sql.py	Mon May 02 20:41:46 2011 +0200
+++ b/server/sources/rql2sql.py	Tue May 10 18:50:13 2011 +0200
@@ -588,16 +588,16 @@
                 rconditions.append(condition)
             else:
                 lconditions.append(condition)
-        else:
-            if louter is not None:
-                raise BadRQLQuery()
+        elif louter is None:
             # merge chains
             self.outer_chains.remove(lchain)
+            rchain += lchain
             self.mark_as_used_in_outer_join(leftalias)
-            rchain += lchain
             for alias, (aouter, aconditions, achain) in outer_tables.iteritems():
                 if achain is lchain:
                     outer_tables[alias] = (aouter, aconditions, rchain)
+        else:
+            raise BadRQLQuery()
 
     # sql generation helpers ###################################################
 
--- a/server/test/unittest_rql2sql.py	Mon May 02 20:41:46 2011 +0200
+++ b/server/test/unittest_rql2sql.py	Tue May 10 18:50:13 2011 +0200
@@ -835,9 +835,9 @@
 WHERE _X.cw_eid=12'''
     ),
     ("Any P WHERE X eid 12, P? concerne X, X todo_by S",
-     '''SELECT rel_concerne0.eid_from
-FROM todo_by_relation AS rel_todo_by1 LEFT OUTER JOIN concerne_relation AS rel_concerne0 ON (rel_concerne0.eid_to=12)
-WHERE rel_todo_by1.eid_from=12'''
+     '''SELECT rel_concerne1.eid_from
+FROM todo_by_relation AS rel_todo_by0 LEFT OUTER JOIN concerne_relation AS rel_concerne1 ON (rel_concerne1.eid_to=12)
+WHERE rel_todo_by0.eid_from=12'''
     ),
 
     ('Any GN, TN ORDERBY GN WHERE T tags G?, T name TN, G name GN',
@@ -931,6 +931,10 @@
 WHERE _S.cw_ambiguous_inlined=_A.cw_eid) AS _T0 ON (_X.cw_multisource_inlined_rel=_T0.C0)'''
     ),
 
+    ('Any X,T,OT WHERE X tags T, OT? tags X, X is Tag, X eid 123',
+     '''SELECT rel_tags0.eid_from, rel_tags0.eid_to, rel_tags1.eid_from
+FROM tags_relation AS rel_tags0 LEFT OUTER JOIN tags_relation AS rel_tags1 ON (rel_tags1.eid_to=123)
+WHERE rel_tags0.eid_from=123'''),
     ]
 
 VIRTUAL_VARS = [
@@ -1601,7 +1605,7 @@
                     '''SELECT 1
 WHERE NOT (EXISTS(SELECT 1 FROM in_group_relation AS rel_in_group0))''')
 
-    def test_nonregr_subquery_missing_join(self):
+    def test_nonregr_outer_join_multiple(self):
         self._check('Any COUNT(P1148),G GROUPBY G '
                     'WHERE G owned_by D, D eid 1122, K1148 bookmarked_by P1148, '
                     'K1148 eid 1148, P1148? in_group G',
@@ -1611,7 +1615,7 @@
 GROUP BY _G.cw_eid'''
                     )
 
-    def test_nonregr_subquery_missing_join2(self):
+    def test_nonregr_outer_join_multiple2(self):
         self._check('Any COUNT(P1148),G GROUPBY G '
                     'WHERE G owned_by D, D eid 1122, K1148 bookmarked_by P1148?, '
                     'K1148 eid 1148, P1148? in_group G',