[rql rewrite] when a subquery has to be introduced, properly return the snippet expression so that further expressions are properly ored. Closes #2207180 stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Fri, 24 Feb 2012 12:35:24 +0100
branchstable
changeset 8264 a4b009ba92ce
parent 8263 a73ad255ff63
child 8269 80d37fb56312
[rql rewrite] when a subquery has to be introduced, properly return the snippet expression so that further expressions are properly ored. Closes #2207180
rqlrewrite.py
test/unittest_rqlrewrite.py
--- a/rqlrewrite.py	Fri Feb 24 12:44:28 2012 +0100
+++ b/rqlrewrite.py	Fri Feb 24 12:35:24 2012 +0100
@@ -1,4 +1,4 @@
-# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -228,19 +228,19 @@
                                            if not r in sti['rhsrelations'])
                 else:
                     vi['rhs_rels'] = vi['lhs_rels'] = {}
-        parent = None
+        previous = None
         inserted = False
         for rqlexpr in rqlexprs:
             self.current_expr = rqlexpr
             if varexistsmap is None:
                 try:
-                    new = self.insert_snippet(varmap, rqlexpr.snippet_rqlst, parent)
+                    new = self.insert_snippet(varmap, rqlexpr.snippet_rqlst, previous)
                 except Unsupported:
                     continue
                 inserted = True
                 if new is not None and self._insert_scope is None:
                     self.exists_snippet[rqlexpr] = new
-                parent = parent or new
+                previous = previous or new
             else:
                 # called to reintroduce snippet due to ambiguity creation,
                 # so skip snippets which are not introducing this ambiguity
@@ -251,16 +251,21 @@
             # no rql expression found matching rql solutions. User has no access right
             raise Unauthorized() # XXX may also be because of bad constraints in schema definition
 
-    def insert_snippet(self, varmap, snippetrqlst, parent=None):
+    def insert_snippet(self, varmap, snippetrqlst, previous=None):
         new = snippetrqlst.where.accept(self)
         existing = self.existingvars
         self.existingvars = None
         try:
-            return self._insert_snippet(varmap, parent, new)
+            return self._insert_snippet(varmap, previous, new)
         finally:
             self.existingvars = existing
 
-    def _insert_snippet(self, varmap, parent, new):
+    def _insert_snippet(self, varmap, previous, new):
+        """insert `new` snippet into the syntax tree, which have been rewritten
+        using `varmap`. In cases where an action is protected by several rql
+        expresssion, `previous` will be the first rql expression which has been
+        inserted, and so should be ORed with the following expressions.
+        """
         if new is not None:
             if self._insert_scope is None:
                 insert_scope = None
@@ -274,28 +279,28 @@
                 insert_scope = self._insert_scope
             if self._insert_scope is None and any(vi.get('stinfo', {}).get('optrelations')
                                                   for vi in self.varinfos):
-                assert parent is None
-                self._insert_scope = self.snippet_subquery(varmap, new)
+                assert previous is None
+                self._insert_scope, new = self.snippet_subquery(varmap, new)
                 self.insert_pending()
                 #self._insert_scope = None
-                return
+                return new
             if not isinstance(new, (n.Exists, n.Not)):
                 new = n.Exists(new)
-            if parent is None:
+            if previous is None:
                 insert_scope.add_restriction(new)
             else:
-                grandpa = parent.parent
-                or_ = n.Or(parent, new)
-                grandpa.replace(parent, or_)
+                grandpa = previous.parent
+                or_ = n.Or(previous, new)
+                grandpa.replace(previous, or_)
             if not self.removing_ambiguity:
                 try:
                     self.compute_solutions()
                 except Unsupported:
                     # some solutions have been lost, can't apply this rql expr
-                    if parent is None:
+                    if previous is None:
                         self.current_statement().remove_node(new, undefine=True)
                     else:
-                        grandpa.replace(or_, parent)
+                        grandpa.replace(or_, previous)
                         self._cleanup_inserted(new)
                     raise
                 else:
@@ -419,7 +424,7 @@
             # some solutions have been lost, can't apply this rql expr
             self.select.remove_subquery(self.select.with_[-1])
             raise
-        return subselect
+        return subselect, snippetrqlst
 
     def remove_ambiguities(self, snippets, newsolutions):
         # the snippet has introduced some ambiguities, we have to resolve them
--- a/test/unittest_rqlrewrite.py	Fri Feb 24 12:44:28 2012 +0100
+++ b/test/unittest_rqlrewrite.py	Fri Feb 24 12:35:24 2012 +0100
@@ -183,9 +183,9 @@
         self.assertEqual(rqlst.as_string(),
                          "Any A,C,T WHERE A documented_by C?, A is Affaire "
                          "WITH C,T BEING (Any C,T WHERE C title T, "
-                         "EXISTS(C in_state B, D in_group F, G require_state B, G name 'read', G require_group F), "
-                         "D eid %(A)s, C is Card, "
-                         "EXISTS(C in_state E, E name 'public'))")
+                         "(EXISTS(C in_state B, D in_group F, G require_state B, G name 'read', G require_group F)) "
+                         "OR (EXISTS(C in_state E, E name 'public')), "
+                         "D eid %(A)s, C is Card)")
 
     def test_optional_var_4(self):
         constraint1 = 'A created_by U, X documented_by A'
@@ -199,8 +199,8 @@
                              u'Any X,LA,Y WHERE LA? documented_by X, LA concerne Y, B eid %(C)s, '
                              'EXISTS(X created_by B), EXISTS(Y created_by B), '
                              'X is Card, Y is IN(Division, Note, Societe) '
-                             'WITH LA BEING (Any LA WHERE EXISTS(A created_by B, LA documented_by A), '
-                             'B eid %(D)s, LA is Affaire, EXISTS(E created_by B, LA concerne E))')
+                             'WITH LA BEING (Any LA WHERE (EXISTS(A created_by B, LA documented_by A)) OR (EXISTS(E created_by B, LA concerne E)), '
+                             'B eid %(D)s, LA is Affaire)')
 
     def test_optional_var_inlined(self):
         c1 = ('X require_permission P')
@@ -431,6 +431,18 @@
         self.assertEqual(rqlst.as_string(),
                          u'Any A WHERE NOT EXISTS(A documented_by C, EXISTS(C owned_by B, B login "hop", B is CWUser), C is Card), A is Affaire')
 
+    def test_rqlexpr_multiexpr_outerjoin(self):
+        c1 = RRQLExpression('X owned_by Z, Z login "hop"', 'X')
+        c2 = RRQLExpression('X owned_by Z, Z login "hip"', 'X')
+        c3 = RRQLExpression('X owned_by Z, Z login "momo"', 'X')
+        rqlst = rqlhelper.parse('Any A WHERE A documented_by C?', annotate=False)
+        rewrite(rqlst, {('C', 'X'): (c1, c2, c3)}, {}, 'X')
+        self.assertEqual(rqlst.as_string(),
+                         u'Any A WHERE A documented_by C?, A is Affaire '
+                         'WITH C BEING (Any C WHERE ((EXISTS(C owned_by B, B login "hop")) '
+                         'OR (EXISTS(C owned_by D, D login "momo"))) '
+                         'OR (EXISTS(C owned_by A, A login "hip")), C is Card)')
+
 
 if __name__ == '__main__':
     unittest_main()