[ms] simplify cw_source handling : don't try evil optmization that may hurt in some cases
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Mon, 25 Oct 2010 17:15:52 +0200
changeset 6633 7baea108d326
parent 6632 78878f5a8166
child 6634 0683748bca81
[ms] simplify cw_source handling : don't try evil optmization that may hurt in some cases
server/msplanner.py
server/test/unittest_msplanner.py
--- a/server/msplanner.py	Mon Oct 25 17:15:51 2010 +0200
+++ b/server/msplanner.py	Mon Oct 25 17:15:52 2010 +0200
@@ -519,17 +519,15 @@
                 neged = srel.neged(traverse_scope=True) or (rel and rel.neged(strict=True))
                 if neged:
                     for source in sources:
-                        self._remove_source_term(source, lhs, check=True)
+                        self._remove_source_term(source, lhs)
                 else:
                     for source, terms in sourcesterms.items():
                         if lhs in terms and not source in sources:
-                            self._remove_source_term(source, lhs, check=True)
+                            self._remove_source_term(source, lhs)
                 if rel is None:
                     self._remove_source_term(self.system_source, vref)
-                    srel.parent.remove(srel)
                 elif len(var.stinfo['relations']) == 2 and not var.stinfo['selected']:
                     self._remove_source_term(self.system_source, var)
-                    self.rqlst.undefine_variable(var)
         return termssources
 
     def _handle_cross_relation(self, rel, relsources, termssources):
@@ -776,16 +774,14 @@
             if not sourcesterms[source][term]:
                 self._remove_source_term(source, term)
 
-    def _remove_source_term(self, source, term, check=False):
-        poped = self._sourcesterms[source].pop(term, None)
-        if not self._sourcesterms[source]:
-            del self._sourcesterms[source]
-        if poped is not None and check:
-            for terms in self._sourcesterms.itervalues():
-                if term in terms:
-                    break
-            else:
-                raise BadRQLQuery('source conflict for term %s' % term.as_string())
+    def _remove_source_term(self, source, term):
+        try:
+            poped = self._sourcesterms[source].pop(term, None)
+        except KeyError:
+            pass
+        else:
+            if not self._sourcesterms[source]:
+                del self._sourcesterms[source]
 
     def crossed_relation(self, source, relation):
         return relation in self._crossrelations.get(source, ())
--- a/server/test/unittest_msplanner.py	Mon Oct 25 17:15:51 2010 +0200
+++ b/server/test/unittest_msplanner.py	Mon Oct 25 17:15:52 2010 +0200
@@ -1845,7 +1845,7 @@
 
     def test_source_specified_0_0(self):
         self._test('Card X WHERE X cw_source S, S eid 1',
-                   [('OneFetchStep', [('Any X WHERE X is Card',
+                   [('OneFetchStep', [('Any X WHERE X cw_source 1, X is Card',
                                        [{'X': 'Card'}])],
                      None, None,
                      [self.system],{}, [])
@@ -1853,7 +1853,7 @@
 
     def test_source_specified_0_1(self):
         self._test('Any X, S WHERE X is Card, X cw_source S, S eid 1',
-                   [('OneFetchStep', [('Any X,1 WHERE X is Card',
+                   [('OneFetchStep', [('Any X,1 WHERE X is Card, X cw_source 1',
                                        [{'X': 'Card'}])],
                      None, None,
                      [self.system],{}, [])
@@ -1861,8 +1861,8 @@
 
     def test_source_specified_1_0(self):
         self._test('Card X WHERE X cw_source S, S name "system"',
-                   [('OneFetchStep', [('Any X WHERE X is Card',
-                                       [{'X': 'Card'}])],
+                   [('OneFetchStep', [('Any X WHERE X cw_source S, S name "system", X is Card',
+                                       [{'X': 'Card', 'S': 'CWSource'}])],
                      None, None,
                      [self.system],{}, [])
                     ])
@@ -1875,15 +1875,28 @@
                      None, None, [self.system], {}, [])
                     ])
 
+    def test_source_specified_1_2(self):
+        sols = []
+        for sol in X_ALL_SOLS:
+            sol = sol.copy()
+            sol['S'] = 'CWSource'
+            sols.append(sol)
+        self._test('Any X WHERE X cw_source S, S name "cards"',
+                   [('OneFetchStep', [('Any X WHERE X cw_source S, S name "cards"',
+                                       sols)],
+                     None, None,
+                     [self.system],{}, [])
+                    ])
+
     def test_source_specified_2_0(self):
         self._test('Card X WHERE X cw_source S, NOT S eid 1',
-                   [('OneFetchStep', [('Any X WHERE X is Card',
-                                       [{'X': 'Card'}])],
+                   [('OneFetchStep', [('Any X WHERE X cw_source S, NOT S eid 1, X is Card, S is CWSource',
+                                       [{'X': 'Card', 'S': 'CWSource'}])],
                      None, None,
                      [self.cards],{}, [])
                     ])
         self._test('Card X WHERE NOT X cw_source S, S eid 1',
-                   [('OneFetchStep', [('Any X WHERE X is Card',
+                   [('OneFetchStep', [('Any X WHERE NOT EXISTS(X cw_source 1), X is Card',
                                        [{'X': 'Card'}])],
                      None, None,
                      [self.cards],{}, [])
@@ -1891,14 +1904,14 @@
 
     def test_source_specified_2_1(self):
         self._test('Card X WHERE X cw_source S, NOT S name "system"',
-                   [('OneFetchStep', [('Any X WHERE X is Card',
-                                       [{'X': 'Card'}])],
+                   [('OneFetchStep', [('Any X WHERE X cw_source S, NOT S name "system", X is Card, S is CWSource',
+                                       [{'X': 'Card', 'S': 'CWSource'}])],
                      None, None,
                      [self.cards],{}, [])
                     ])
         self._test('Card X WHERE NOT X cw_source S, S name "system"',
-                   [('OneFetchStep', [('Any X WHERE X is Card',
-                                       [{'X': 'Card'}])],
+                   [('OneFetchStep', [('Any X WHERE NOT EXISTS(X cw_source S), S name "system", X is Card, S is CWSource',
+                                       [{'X': 'Card', 'S': 'CWSource'}])],
                      None, None,
                      [self.cards],{}, [])
                     ])
@@ -1916,9 +1929,13 @@
         self.assertEqual(str(ex), 'source conflict for term X')
 
     def test_source_conflict_3(self):
-        ex = self.assertRaises(BadRQLQuery,
-                               self._test, 'CWSource X WHERE X cw_source S, S name "cards"', [])
-        self.assertEqual(str(ex), 'source conflict for term X')
+        self._test('CWSource X WHERE X cw_source S, S name "cards"',
+                   [('OneFetchStep',
+                     [(u'Any X WHERE X cw_source S, S name "cards", X is CWSource',
+                       [{'S': 'CWSource', 'X': 'CWSource'}])],
+                     None, None,
+                     [self.system],
+                     {}, [])])
 
     def test_ambigous_cross_relation_source_specified(self):
         self.repo._type_source_cache[999999] = ('Note', 'cards', 999999)