# HG changeset patch # User Sylvain Thénault # Date 1282294797 -7200 # Node ID 087c5a168010120d32f62447c499e2b4fbf2415b # Parent 15fa8425b6e74fb6c6b61824a7bf414fa0db07f9 [ms] more planning bug fixes * fix sourcesterms w/ constant: all occurences of the same constant should be there once one of them is * fix handling of constant and relation in expand_terms: * we should always consider system source there for constants * for relation, we should check its lhs and rhs are in selected terms, not that the relation is in linkedterms (this has no chance to be true) This fixes 3 tests that were either passing erroneously (syt should be punished for copy/paste output as test reulst without triple checking it) or were not optimal. diff -r 15fa8425b6e7 -r 087c5a168010 server/msplanner.py --- a/server/msplanner.py Fri Aug 20 08:36:58 2010 +0200 +++ b/server/msplanner.py Fri Aug 20 10:59:57 2010 +0200 @@ -110,6 +110,11 @@ # str() Constant.value to ensure generated table name won't be unicode Constant._ms_table_key = lambda x: str(x.value) +Variable._ms_may_be_processed = lambda x, terms, linkedterms: any( + t for t in terms if t in linkedterms.get(x, ())) +Relation._ms_may_be_processed = lambda x, terms, linkedterms: all( + getattr(hs, 'variable', hs) in terms for hs in x.get_variable_parts()) + def ms_scope(term): rel = None scope = term.scope @@ -411,7 +416,8 @@ for const in vconsts: self._set_source_for_term(source, const) elif not self._sourcesterms: - self._set_source_for_term(source, const) + for const in vconsts: + self._set_source_for_term(source, const) elif source in self._sourcesterms: source_scopes = frozenset(ms_scope(t) for t in self._sourcesterms[source]) for const in vconsts: @@ -480,6 +486,7 @@ # not supported by the source, so we can stop here continue self._sourcesterms.setdefault(ssource, {})[rel] = set(self._solindices) + solindices = None for term in crossvars: if len(termssources[term]) == 1 and iter(termssources[term]).next()[0].uri == 'system': for ov in crossvars: @@ -487,8 +494,14 @@ ssset = frozenset((ssource,)) self._remove_sources(ov, termssources[ov] - ssset) break + if solindices is None: + solindices = set(sol for s, sol in termssources[term] + if s is source) + else: + solindices &= set(sol for s, sol in termssources[term] + if s is source) else: - self._sourcesterms.setdefault(source, {})[rel] = set(self._solindices) + self._sourcesterms.setdefault(source, {})[rel] = solindices def _remove_invalid_sources(self, termssources): """removes invalid sources from `sourcesterms` member according to @@ -801,10 +814,13 @@ rhsvar = rhs.variable except AttributeError: rhsvar = rhs - if lhsvar in terms and not rhsvar in terms: - needsel.add(lhsvar.name) - elif rhsvar in terms and not lhsvar in terms: - needsel.add(rhsvar.name) + try: + if lhsvar in terms and not rhsvar in terms: + needsel.add(lhsvar.name) + elif rhsvar in terms and not lhsvar in terms: + needsel.add(rhsvar.name) + except AttributeError: + continue # not an attribute, no selection needed if final and source.uri != 'system': # check rewritten constants for vconsts in select.stinfo['rewritten'].itervalues(): @@ -939,13 +955,14 @@ exclude[vars[1]] = vars[0] except IndexError: pass - accept_term = lambda x: (not any(s for s in sources if not x in sourcesterms.get(s, ())) - and any(t for t in terms if t in linkedterms.get(x, ())) + accept_term = lambda x: (not any(s for s in sources + if not x in sourcesterms.get(s, ())) + and x._ms_may_be_processed(terms, linkedterms) and not exclude.get(x) in terms) if isinstance(term, Relation) and term in cross_rels: cross_terms = cross_rels.pop(term) base_accept_term = accept_term - accept_term = lambda x: (base_accept_term(x) or x in cross_terms) + accept_term = lambda x: (accept_term(x) or x in cross_terms) for refed in cross_terms: if not refed in candidates: terms.append(refed) @@ -956,7 +973,11 @@ modified = False for term in candidates[:]: if isinstance(term, Constant): - if sorted(set(x[0] for x in self._term_sources(term))) != sources: + termsources = set(x[0] for x in self._term_sources(term)) + # ensure system source is there for constant + if self.system_source in sources: + termsources.add(self.system_source) + if sorted(termsources) != sources: continue terms.append(term) candidates.remove(term) @@ -1614,6 +1635,8 @@ for vref in supportedvars: if not vref in newroot.get_selected_variables(): newroot.append_selected(VariableRef(newroot.get_variable(vref.name))) + elif term in self.terms: + newroot.append_selected(term.copy(newroot)) def add_necessary_selection(self, newroot, terms): selected = tuple(newroot.get_selected_variables()) diff -r 15fa8425b6e7 -r 087c5a168010 server/sources/rql2sql.py --- a/server/sources/rql2sql.py Fri Aug 20 08:36:58 2010 +0200 +++ b/server/sources/rql2sql.py Fri Aug 20 10:59:57 2010 +0200 @@ -1154,9 +1154,13 @@ if constant.type == 'Boolean': value = self.dbhelper.boolean_value(value) if constant.type == 'Substitute': - _id = constant.value - if isinstance(_id, unicode): - _id = _id.encode() + try: + # we may found constant from simplified var in varmap + return self._mapped_term(constant, '%%(%s)s' % value)[0] + except KeyError: + _id = constant.value + if isinstance(_id, unicode): + _id = _id.encode() else: _id = str(id(constant)).replace('-', '', 1) self._query_attrs[_id] = value @@ -1262,12 +1266,19 @@ break return scope + def _mapped_term(self, term, key): + """return sql and table alias to the `term`, mapped as `key` or raise + KeyError when the key is not found in the varmap + """ + sql = self._varmap[key] + tablealias = sql.split('.', 1)[0] + scope = self._temp_table_scope(term.stmt, tablealias) + self.add_table(tablealias, scope=scope) + return sql, tablealias + def _var_info(self, var): try: - sql = self._varmap[var.name] - tablealias = sql.split('.', 1)[0] - scope = self._temp_table_scope(var.stmt, tablealias) - self.add_table(tablealias, scope=scope) + return self._mapped_term(var, var.name) except KeyError: scope = self._state.scopes[var.scope] etype = self._state.solution[var.name] diff -r 15fa8425b6e7 -r 087c5a168010 server/test/unittest_msplanner.py --- a/server/test/unittest_msplanner.py Fri Aug 20 08:36:58 2010 +0200 +++ b/server/test/unittest_msplanner.py Fri Aug 20 10:59:57 2010 +0200 @@ -1541,20 +1541,11 @@ def test_crossed_relation_eid_2_needattr(self): repo._type_source_cache[999999] = ('Note', 'cards', 999999) self._test('Any Y,T WHERE X eid %(x)s, X multisource_crossed_rel Y, Y type T', - [('FetchStep', [('Any Y,T WHERE Y type T, Y is Note', [{'T': 'String', 'Y': 'Note'}])], - [self.cards, self.system], None, - {'T': 'table0.C1', 'Y': 'table0.C0', 'Y.type': 'table0.C1'}, []), - ('UnionStep', None, None, - [('OneFetchStep', [('Any Y,T WHERE 999999 multisource_crossed_rel Y, Y type T, Y is Note', - [{'T': 'String', 'Y': 'Note'}])], - None, None, [self.cards], None, - []), - ('OneFetchStep', [('Any Y,T WHERE 999999 multisource_crossed_rel Y, Y type T, Y is Note', - [{'T': 'String', 'Y': 'Note'}])], - None, None, [self.system], - {'T': 'table0.C1', 'Y': 'table0.C0', 'Y.type': 'table0.C1'}, - [])] - )], + [('OneFetchStep', [('Any Y,T WHERE 999999 multisource_crossed_rel Y, Y type T, Y is Note', + [{'T': 'String', 'Y': 'Note'}])], + None, None, [self.cards, self.system], {}, + []), + ], {'x': 999999,}) def test_crossed_relation_eid_not_1(self): @@ -1789,39 +1780,22 @@ self.cards.cross_relations.add('see_also') try: self._test('Any X,AA ORDERBY AA WHERE E eid %(x)s, E see_also X, X modification_date AA', - [('FetchStep', [('Any X,AA WHERE X modification_date AA, X is Note', - [{'AA': 'Datetime', 'X': 'Note'}])], - [self.cards, self.system], None, - {'AA': 'table0.C1', 'X': 'table0.C0', - 'X.modification_date': 'table0.C1'}, - []), - ('AggrStep', 'SELECT table1.C0, table1.C1 FROM table1 ORDER BY table1.C1', + [('AggrStep', + 'SELECT table0.C0, table0.C1 FROM table0 ORDER BY table0.C1', None, - [('FetchStep', [('Any X,AA WHERE 999999 see_also X, X modification_date AA, X is IN(Bookmark)', - [{'AA': 'Datetime', 'X': 'Bookmark'}])], - [self.cards, self.system], - {}, - {'AA': 'table1.C1', - 'X': 'table1.C0', 'X.modification_date': 'table1.C1'}, + [('FetchStep', + [('Any X,AA WHERE 999999 see_also X, X modification_date AA, X is Note', + [{'AA': 'Datetime', 'X': 'Note'}])], [self.cards, self.system], {}, + {'AA': 'table0.C1', 'X': 'table0.C0', + 'X.modification_date': 'table0.C1'}, []), ('FetchStep', - [('Any X,AA WHERE 999999 see_also X, X modification_date AA, X is IN(Note)', - [{'AA': 'Datetime', 'X': 'Note'}])], - [self.cards], - None, - {'AA': 'table1.C1', - 'X': 'table1.C0', 'X.modification_date': 'table1.C1'}, - []), - ('FetchStep', - [('Any X,AA WHERE 999999 see_also X, X modification_date AA, X is IN(Note)', - [{'AA': 'Datetime', 'X': 'Note'}])], - [self.system], - {'AA': 'table0.C1', - 'X': 'table0.C0', 'X.modification_date': 'table0.C1'}, - {'AA': 'table1.C1', - 'X': 'table1.C0', 'X.modification_date': 'table1.C1'}, - [])] - )], + [('Any X,AA WHERE 999999 see_also X, X modification_date AA, X is Bookmark', + [{'AA': 'Datetime', 'X': 'Bookmark'}])], + [self.system], {}, + {'AA': 'table0.C1', 'X': 'table0.C0', + 'X.modification_date': 'table0.C1'}, + [])])], {'x': 999999}) finally: del self.cards.support_relations['see_also'] @@ -1943,11 +1917,16 @@ def test_nonregr8(self): repo._type_source_cache[999999] = ('Note', 'cards', 999999) self._test('Any X,Z WHERE X eid %(x)s, X multisource_rel Y, Z concerne X', - [('FetchStep', [('Any WHERE 999999 multisource_rel Y, Y is Note', [{'Y': 'Note'}])], - [self.cards], None, {}, []), + [('FetchStep', [('Any 999999 WHERE 999999 multisource_rel Y, Y is Note', + [{'Y': 'Note'}])], + [self.cards], + None, {u'%(x)s': 'table0.C0'}, + []), ('OneFetchStep', [('Any 999999,Z WHERE Z concerne 999999, Z is Affaire', [{'Z': 'Affaire'}])], - None, None, [self.system], {}, [])], + None, None, [self.system], + {u'%(x)s': 'table0.C0'}, []), + ], {'x': 999999}) def test_nonregr9(self):