[repo] fix bug introduced by 4757:ec9c20c6b9f7, testing for select.selection is not enough to avoid the substep query, we should check there is no interesting restriction (test added)
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Thu, 04 Mar 2010 12:07:54 +0100
changeset 4795 f1c8bc628b45
parent 4793 fdb5476dee9a
child 4796 a20edc0f8b30
[repo] fix bug introduced by 4757:ec9c20c6b9f7, testing for select.selection is not enough to avoid the substep query, we should check there is no interesting restriction (test added)
server/ssplanner.py
server/test/unittest_querier.py
--- a/server/ssplanner.py	Thu Mar 04 10:58:28 2010 +0100
+++ b/server/ssplanner.py	Thu Mar 04 12:07:54 2010 +0100
@@ -76,6 +76,38 @@
                 eidconsts[lhs.variable] = eid
     return eidconsts
 
+def _build_substep_query(select, origrqlst):
+    """Finalize substep select query that should be executed to get proper
+    selection of stuff to insert/update.
+
+    Return None when no query actually needed, else the given select node that
+    will be used as substep query.
+
+    When select has nothing selected, search in origrqlst for restriction that
+    should be considered.
+    """
+    if select.selection:
+        if origrqlst.where is not None:
+            select.set_where(origrqlst.where.copy(select))
+        return select
+    if origrqlst.where is None:
+        return
+    for rel in origrqlst.where.iget_nodes(Relation):
+        # search for a relation which is neither a type restriction (is) nor an
+        # eid specification (not neged eid with constant node
+        if rel.neged(strict=True) or not (
+            rel.is_types_restriction() or
+            (rel.r_type == 'eid'
+             and isinstance(rel.get_variable_parts()[1], Constant))):
+            break
+    else:
+        return
+    select.set_where(origrqlst.where.copy(select))
+    if not select.selection:
+        # no selection, append one randomly
+        select.append_selected(rel.children[0].copy(select))
+    return select
+
 
 class SSPlanner(object):
     """SingleSourcePlanner: build execution plan for rql queries
@@ -144,9 +176,8 @@
                     else:
                         rdefs[i] = (rtype, InsertRelationsStep.RELATION, value)
             step = InsertRelationsStep(plan, edef, rdefs)
-            if select.selection:
-                if rqlst.where is not None:
-                    select.set_where(rqlst.where.copy(select))
+            select = _build_substep_query(select, rqlst)
+            if select is not None:
                 step.children += self._select_plan(plan, select, rqlst.solutions)
             yield step
 
@@ -241,9 +272,8 @@
         # the update step
         step = UpdateStep(plan, updatedefs, attributes)
         # when necessary add substep to fetch yet unknown values
-        if select.selection:
-            if rqlst.where is not None:
-                select.set_where(rqlst.where.copy(select))
+        select = _build_substep_query(select, rqlst)
+        if select is not None:
             # set distinct to avoid potential duplicate key error
             select.distinct = True
             step.children += self._select_plan(plan, select, rqlst.solutions)
--- a/server/test/unittest_querier.py	Thu Mar 04 10:58:28 2010 +0100
+++ b/server/test/unittest_querier.py	Thu Mar 04 12:07:54 2010 +0100
@@ -1028,6 +1028,10 @@
                       {'x': str(eid1), 'y': str(eid2)})
         rset = self.execute('Any X, Y WHERE X travaille Y')
         self.assertEqual(len(rset.rows), 1)
+        # test add of an existant relation but with NOT X rel Y protection
+        self.failIf(self.execute("SET X travaille Y WHERE X eid %(x)s, Y eid %(y)s,"
+                                 "NOT X travaille Y",
+                                 {'x': str(eid1), 'y': str(eid2)}))
 
     def test_update_2ter(self):
         rset = self.execute("INSERT Personne X, Societe Y: X nom 'bidule', Y nom 'toto'")