[multi-sources] fix planning of crossed relation w/ some complex queries
see ticket #59168 on our extranet for an example symptom.
--- a/devtools/repotest.py Fri Jan 07 15:21:56 2011 +0100
+++ b/devtools/repotest.py Fri Jan 07 18:51:47 2011 +0100
@@ -378,7 +378,7 @@
def _merge_input_maps(*args, **kwargs):
return sorted(_orig_merge_input_maps(*args, **kwargs))
-def _choose_term(self, sourceterms):
+def _choose_term(self, source, sourceterms):
# predictable order for test purpose
def get_key(x):
try:
@@ -391,7 +391,7 @@
except AttributeError:
# const
return x.value
- return _orig_choose_term(self, DumbOrderedDict2(sourceterms, get_key))
+ return _orig_choose_term(self, source, DumbOrderedDict2(sourceterms, get_key))
from cubicweb.server.sources.pyrorql import PyroRQLSource
_orig_syntax_tree_search = PyroRQLSource.syntax_tree_search
--- a/server/msplanner.py Fri Jan 07 15:21:56 2011 +0100
+++ b/server/msplanner.py Fri Jan 07 18:51:47 2011 +0100
@@ -824,7 +824,7 @@
while sourceterms:
# take a term randomly, and all terms supporting the
# same solutions
- term, solindices = self._choose_term(sourceterms)
+ term, solindices = self._choose_term(source, sourceterms)
if source.uri == 'system':
# ensure all variables are available for the latest step
# (missing one will be available from temporary tables
@@ -854,7 +854,7 @@
# set of terms which should be additionaly selected when
# possible
needsel = set()
- if not self._sourcesterms:
+ if not self._sourcesterms and scope is select:
terms += scope.defined_vars.values() + scope.aliases.values()
if isinstance(term, Relation) and len(sources) > 1:
variants = set()
@@ -867,13 +867,10 @@
# 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) )
+ solindices, scope, variants, False) )
sources = [self.system_source]
final = True
else:
@@ -906,7 +903,7 @@
break
else:
if not scope is select:
- self._exists_relation(rel, terms, needsel)
+ self._exists_relation(rel, terms, needsel, source)
# if relation is supported by all sources and some of
# its lhs/rhs variable isn't in "terms", and the
# other end *is* in "terms", mark it have to be
@@ -950,9 +947,14 @@
self._cleanup_sourcesterms(sources, solindices)
steps.append((sources, terms, solindices, scope, needsel, final)
)
+ if not steps[-1][-1]:
+ # add a final step
+ terms = select.defined_vars.values() + select.aliases.values()
+ steps.append( ([self.system_source], terms, set(self._solindices),
+ select, set(), True) )
return steps
- def _exists_relation(self, rel, terms, needsel):
+ def _exists_relation(self, rel, terms, needsel, source):
rschema = self._schema.rschema(rel.r_type)
lhs, rhs = rel.get_variable_parts()
try:
@@ -965,13 +967,24 @@
# variable is refed by an outer scope and should be substituted
# using an 'identity' relation (else we'll get a conflict of
# temporary tables)
- if rhsvar in terms and not lhsvar in terms and ms_scope(lhsvar) is lhsvar.stmt:
- self._identity_substitute(rel, lhsvar, terms, needsel)
- elif lhsvar in terms and not rhsvar in terms and ms_scope(rhsvar) is rhsvar.stmt:
- self._identity_substitute(rel, rhsvar, terms, needsel)
+ relscope = ms_scope(rel)
+ lhsscope = ms_scope(lhsvar)
+ rhsscope = ms_scope(rhsvar)
+ if rhsvar in terms and not lhsvar in terms and lhsscope is lhsvar.stmt:
+ self._identity_substitute(rel, lhsvar, terms, needsel, relscope)
+ elif lhsvar in terms and not rhsvar in terms and rhsscope is rhsvar.stmt:
+ self._identity_substitute(rel, rhsvar, terms, needsel, relscope)
+ elif self.crossed_relation(source, rel):
+ if lhsscope is not relscope:
+ self._identity_substitute(rel, lhsvar, terms, needsel,
+ relscope, lhsscope)
+ if rhsscope is not relscope:
+ self._identity_substitute(rel, rhsvar, terms, needsel,
+ relscope, rhsscope)
- def _identity_substitute(self, relation, var, terms, needsel):
- newvar = self._insert_identity_variable(ms_scope(relation), var)
+ def _identity_substitute(self, relation, var, terms, needsel, exist,
+ idrelscope=None):
+ newvar = self._insert_identity_variable(exist, var, idrelscope)
# ensure relation is using '=' operator, else we rely on a
# sqlgenerator side effect (it won't insert an inequality operator
# in this case)
@@ -979,12 +992,28 @@
terms.append(newvar)
needsel.add(newvar.name)
- def _choose_term(self, sourceterms):
+ def _choose_term(self, source, sourceterms):
"""pick one term among terms supported by a source, which will be used
as a base to generate an execution step
"""
secondchoice = None
if len(self._sourcesterms) > 1:
+ # first, return non invariant variable of crossed relation, then the
+ # crossed relation itself
+ for term in sourceterms:
+ if (isinstance(term, Relation)
+ and self.crossed_relation(source, term)
+ and not ms_scope(term) is self.rqlst):
+ for vref in term.get_variable_parts():
+ try:
+ var = vref.variable
+ except AttributeError:
+ # Constant
+ continue
+ if ((len(var.stinfo['relations']) > 1 or var.stinfo['selected'])
+ and var in sourceterms):
+ return var, sourceterms.pop(var)
+ return term, sourceterms.pop(term)
# priority to variable from subscopes
for term in sourceterms:
if not ms_scope(term) is self.rqlst:
--- a/server/test/unittest_msplanner.py Fri Jan 07 15:21:56 2011 +0100
+++ b/server/test/unittest_msplanner.py Fri Jan 07 18:51:47 2011 +0100
@@ -2473,6 +2473,37 @@
)]
)
+ def test_version_crossed_depends_on_4(self):
+ self._test('Any X,AD,AE WHERE EXISTS(E multisource_crossed_rel X), X in_state AD, AD name AE, E is Note',
+ [('FetchStep',
+ [('Any X,AD,AE WHERE X in_state AD, AD name AE, AD is State, X is Note',
+ [{'X': 'Note', 'AD': 'State', 'AE': 'String'}])],
+ [self.cards, self.cards2, self.system], None,
+ {'X': 'table0.C0',
+ 'AD': 'table0.C1',
+ 'AD.name': 'table0.C2',
+ 'AE': 'table0.C2'},
+ []),
+ ('FetchStep',
+ [('Any A WHERE E multisource_crossed_rel A, A is Note, E is Note',
+ [{'A': 'Note', 'E': 'Note'}])],
+ [self.cards, self.cards2, self.system], None,
+ {'A': 'table1.C0'},
+ []),
+ ('OneFetchStep',
+ [('Any X,AD,AE WHERE EXISTS(X identity A), AD name AE, A is Note, AD is State, X is Note',
+ [{'A': 'Note', 'AD': 'State', 'AE': 'String', 'X': 'Note'}])],
+ None, None,
+ [self.system],
+ {'A': 'table1.C0',
+ 'AD': 'table0.C1',
+ 'AD.name': 'table0.C2',
+ 'AE': 'table0.C2',
+ 'X': 'table0.C0'},
+ []
+ )]
+ )
+
def test_nonregr_dont_cross_rel_source_filtering_1(self):
self.repo._type_source_cache[999999] = ('Note', 'cards', 999999)
self._test('Any S WHERE E eid %(x)s, E in_state S, NOT S name "moved"',