[ms] #1382452: incorrect results with multi-sources stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Tue, 04 Jan 2011 09:07:22 +0100
branchstable
changeset 6758 28b11ecf319b
parent 6757 bc878ec35794
child 6759 5d016d5bacca
[ms] #1382452: incorrect results with multi-sources when some relation is crossed among sources with non-invariant variables, we've to introduce an intermediary steps to fetch all relations from all sources, then make the join with intermediary tables containing possible variables values.
devtools/repotest.py
server/msplanner.py
server/test/data/extern_mapping.py
server/test/unittest_msplanner.py
server/test/unittest_multisources.py
--- a/devtools/repotest.py	Tue Dec 21 21:20:19 2010 +0100
+++ b/devtools/repotest.py	Tue Jan 04 09:07:22 2011 +0100
@@ -141,7 +141,7 @@
 from rql import RQLHelper
 
 from cubicweb.devtools.fake import FakeRepo, FakeSession
-from cubicweb.server import set_debug
+from cubicweb.server import set_debug, debugged
 from cubicweb.server.querier import QuerierHelper
 from cubicweb.server.session import Session
 from cubicweb.server.sources.rql2sql import SQLGenerator, remove_unused_solutions
@@ -171,6 +171,8 @@
 
     def set_debug(self, debug):
         set_debug(debug)
+    def debugged(self, debug):
+        return debugged(debug)
 
     def _prepare(self, rql):
         #print '******************** prepare', rql
@@ -222,6 +224,8 @@
 
     def set_debug(self, debug):
         set_debug(debug)
+    def debugged(self, debug):
+        return debugged(debug)
 
     def _rqlhelper(self):
         rqlhelper = self.repo.vreg.rqlhelper
--- a/server/msplanner.py	Tue Dec 21 21:20:19 2010 +0100
+++ b/server/msplanner.py	Tue Jan 04 09:07:22 2011 +0100
@@ -856,6 +856,25 @@
                 needsel = set()
                 if not self._sourcesterms:
                     terms += scope.defined_vars.values() + scope.aliases.values()
+                    if isinstance(term, Relation) and len(sources) > 1:
+                        variants = set()
+                        partterms = [term]
+                        for vref in term.get_nodes(VariableRef):
+                            if not vref.variable._q_invariant:
+                                variants.add(vref.name)
+                        if len(variants) == 2:
+                            # we need an extra-step to fetch relations from each source
+                            # before a join with prefetched inputs
+                            # (see test_crossed_relation_noeid_needattr in
+                            #  unittest_msplanner / unittest_multisources)
+                            needsel2 = needsel.copy()
+                            needsel2.update(variants)
+                            lhs, rhs = term.get_variable_parts()
+                            steps.append( (sources, [term, getattr(lhs, 'variable', lhs),
+                                                     getattr(rhs, 'variable', rhs)],
+                                           solindices, scope,
+                                           needsel2, False) )
+                            sources = [self.system_source]
                     final = True
                 else:
                     # suppose this is a final step until the contrary is proven
@@ -1548,7 +1567,7 @@
 
     def visit_relation(self, node, newroot, terms):
         if not node.is_types_restriction():
-            if node in self.skip and self.solindices.issubset(self.skip[node]):
+            if not node in terms and node in self.skip and self.solindices.issubset(self.skip[node]):
                 if not self.schema.rschema(node.r_type).final:
                     # can't really skip the relation if one variable is selected
                     # and only referenced by this relation
--- a/server/test/data/extern_mapping.py	Tue Dec 21 21:20:19 2010 +0100
+++ b/server/test/data/extern_mapping.py	Tue Jan 04 09:07:22 2011 +0100
@@ -15,8 +15,9 @@
 #
 # You should have received a copy of the GNU Lesser General Public License along
 # with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
-"""
+"""mapping file for source used in unittest_multisources.py"""
 
-"""
 support_entities = {'Card': True, 'Affaire': True, 'State': True}
 support_relations = {'in_state': True, 'documented_by': True, 'multisource_inlined_rel': True}
+
+cross_relations = set( ('documented_by',) )
--- a/server/test/unittest_msplanner.py	Tue Dec 21 21:20:19 2010 +0100
+++ b/server/test/unittest_msplanner.py	Tue Jan 04 09:07:22 2011 +0100
@@ -1607,20 +1607,50 @@
                     ('FetchStep',  [('Any Y,T WHERE Y type T, Y is Note', [{'T': 'String', 'Y': 'Note'}])],
                      [self.cards, self.system], None,
                      {'T': 'table1.C1', 'Y': 'table1.C0', 'Y.type': 'table1.C1'},  []),
-                    ('UnionStep', None,  None,
-                     [('OneFetchStep', [('Any X,Y,T WHERE X multisource_crossed_rel Y, Y type T, X type T, X is Note, Y is Note',
-                                         [{'T': 'String', 'X': 'Note', 'Y': 'Note'}])],
-                       None, None, [self.cards], None,
-                       []),
-                      ('OneFetchStep', [('Any X,Y,T WHERE X multisource_crossed_rel Y, Y type T, X type T, X is Note, Y is Note',
-                                         [{'T': 'String', 'X': 'Note', 'Y': 'Note'}])],
-                       None, None, [self.system],
-                       {'T': 'table1.C1', 'X': 'table0.C0', 'X.type': 'table0.C1',
-                        'Y': 'table1.C0', 'Y.type': 'table1.C1'},
-                       [])]
-                     )],
+                    ('FetchStep', [('Any X,Y WHERE X multisource_crossed_rel Y, X is Note, Y is Note',
+                                    [{'X': 'Note', 'Y': 'Note'}])],
+                     [self.cards, self.system], None,
+                     {'X': 'table2.C0', 'Y': 'table2.C1'},
+                     []),
+                    ('OneFetchStep', [('Any X,Y,T WHERE X multisource_crossed_rel Y, Y type T, X type T, '
+                                       'X is Note, Y is Note, Y identity A, X identity B, A is Note, B is Note',
+                                       [{u'A': 'Note', u'B': 'Note', 'T': 'String', 'X': 'Note', 'Y': 'Note'}])],
+                     None, None,
+                     [self.system],
+                     {'A': 'table1.C0',
+                      'B': 'table0.C0',
+                      'T': 'table1.C1',
+                      'X': 'table2.C0',
+                      'X.type': 'table0.C1',
+                      'Y': 'table2.C1',
+                      'Y.type': 'table1.C1'},
+                     []),
+                    ],
                     {'x': 999999,})
 
+    def test_crossed_relation_noeid_needattr(self):
+        # http://www.cubicweb.org/ticket/1382452
+        self._test('DISTINCT Any DEP WHERE DEP is Note, P type "cubicweb-foo", P multisource_crossed_rel DEP, DEP type LIKE "cubicweb%"',
+                   [('FetchStep', [(u'Any DEP WHERE DEP type LIKE "cubicweb%", DEP is Note',
+                                    [{'DEP': 'Note'}])],
+                     [self.cards, self.system], None,
+                     {'DEP': 'table0.C0'},
+                     []),
+                    ('FetchStep', [(u'Any P WHERE P type "cubicweb-foo", P is Note', [{'P': 'Note'}])],
+                     [self.cards, self.system], None, {'P': 'table1.C0'},
+                     []),
+                    ('FetchStep', [('Any DEP,P WHERE P multisource_crossed_rel DEP, DEP is Note, P is Note',
+                                    [{'DEP': 'Note', 'P': 'Note'}])],
+                     [self.cards, self.system], None, {'DEP': 'table2.C0', 'P': 'table2.C1'},
+                     []),
+                    ('OneFetchStep',
+                     [('DISTINCT Any DEP WHERE P multisource_crossed_rel DEP, DEP is Note, '
+                       'P is Note, DEP identity A, P identity B, A is Note, B is Note',
+                       [{u'A': 'Note', u'B': 'Note', 'DEP': 'Note', 'P': 'Note'}])],
+                     None, None, [self.system],
+                     {'A': 'table0.C0', 'B': 'table1.C0', 'DEP': 'table2.C0', 'P': 'table2.C1'},
+                     [])])
+
     # edition queries tests ###################################################
 
     def test_insert_simplified_var_1(self):
--- a/server/test/unittest_multisources.py	Tue Dec 21 21:20:19 2010 +0100
+++ b/server/test/unittest_multisources.py	Tue Jan 04 09:07:22 2011 +0100
@@ -76,7 +76,9 @@
     TestServerConfiguration.no_sqlite_wrap = False
 
 class TwoSourcesTC(CubicWebTC):
-
+    """Main repo -> extern-multi -> extern
+                  \-------------/
+    """
     @classmethod
     def _refresh_repo(cls):
         super(TwoSourcesTC, cls)._refresh_repo()
@@ -323,6 +325,19 @@
         cu.execute('DELETE Card X WHERE X eid %(x)s', {'x':ceid})
         cnx3.commit()
 
+    def test_crossed_relation_noeid_needattr(self):
+        """http://www.cubicweb.org/ticket/1382452"""
+        aff1 = self.sexecute('INSERT Affaire X: X ref "AFFREF"')[0][0]
+        # link within extern source
+        ec1 = self.sexecute('Card X WHERE X wikiid "zzz"')[0][0]
+        self.sexecute('SET A documented_by C WHERE E eid %(a)s, C eid %(c)s',
+                      {'a': aff1, 'c': ec1})
+        # link from system to extern source
+        self.sexecute('SET A documented_by C WHERE E eid %(a)s, C eid %(c)s',
+                      {'a': aff1, 'c': self.ic2})
+        rset = self.sexecute('DISTINCT Any DEP WHERE P ref "AFFREF", P documented_by DEP, DEP wikiid LIKE "z%"')
+        self.assertEqual(sorted(rset.rows), [[ec1], [self.ic2]])
+
     def test_nonregr1(self):
         ueid = self.session.user.eid
         affaire = self.sexecute('Affaire X WHERE X ref "AFFREF"').get_entity(0, 0)