# HG changeset patch # User Sylvain Thénault # Date 1305046213 -7200 # Node ID 5ad3154a88103c204ad9b2fefa30a8fd4cf99f45 # Parent f627ab500fdaa831fad59b8b3dd437aad76320f6 [rql2sql] fix bug avoiding outer join relation to be used as a variable principal. Closes #1659395 diff -r f627ab500fda -r 5ad3154a8810 devtools/repotest.py --- a/devtools/repotest.py Mon May 02 20:41:46 2011 +0200 +++ b/devtools/repotest.py Tue May 10 18:50:13 2011 +0200 @@ -371,8 +371,13 @@ _orig_select_principal = rqlannotation._select_principal def _select_principal(scope, relations): + def sort_key(something): + try: + return something.r_type + except AttributeError: + return (something[0].r_type, something[1]) return _orig_select_principal(scope, relations, - _sort=lambda rels: sorted(rels, key=lambda x: x.r_type)) + _sort=lambda rels: sorted(rels, key=sort_key)) try: from cubicweb.server.msplanner import PartPlanInformation diff -r f627ab500fda -r 5ad3154a8810 server/rqlannotation.py --- a/server/rqlannotation.py Mon May 02 20:41:46 2011 +0200 +++ b/server/rqlannotation.py Tue May 10 18:50:13 2011 +0200 @@ -78,18 +78,19 @@ continue lhs, rhs = rel.get_parts() onlhs = ref is lhs + role = 'subject' if onlhs else 'object' if rel.r_type == 'eid': if not (onlhs and len(stinfo['relations']) > 1): break if not stinfo['constnode']: - joins.add(rel) + joins.add( (rel, role) ) continue elif rel.r_type == 'identity': # identity can't be used as principal, so check other relation are used # XXX explain rhs.operator == '=' if rhs.operator != '=' or len(stinfo['relations']) <= 1: #(stinfo['constnode'] and rhs.operator == '='): break - joins.add(rel) + joins.add( (rel, role) ) continue rschema = getrschema(rel.r_type) if rel.optional: @@ -116,7 +117,7 @@ # need join anyway if the variable appears in a final or # inlined relation break - joins.add(rel) + joins.add( (rel, role) ) continue if not stinfo['constnode']: if rschema.inlined and rel.neged(strict=True): @@ -129,7 +130,7 @@ break elif rschema.symmetric and stinfo['selected']: break - joins.add(rel) + joins.add( (rel, role) ) else: # if there is at least one ambigous relation and no other to # restrict types, can't be invariant since we need to filter out @@ -169,10 +170,15 @@ diffscope_rels = {} ored_rels = set() diffscope_rels = set() - for rel in _sort(relations): + for rel, role in _sort(relations): # note: only eid and has_text among all final relations may be there if rel.r_type in ('eid', 'identity'): continue + if rel.optional is not None and len(relations) > 1: + if role == 'subject' and rel.optional == 'right': + continue + if role == 'object' and rel.optional == 'left': + continue if rel.ored(traverse_scope=True): ored_rels.add(rel) elif rel.scope is scope: diff -r f627ab500fda -r 5ad3154a8810 server/sources/rql2sql.py --- a/server/sources/rql2sql.py Mon May 02 20:41:46 2011 +0200 +++ b/server/sources/rql2sql.py Tue May 10 18:50:13 2011 +0200 @@ -588,16 +588,16 @@ rconditions.append(condition) else: lconditions.append(condition) - else: - if louter is not None: - raise BadRQLQuery() + elif louter is None: # merge chains self.outer_chains.remove(lchain) + rchain += lchain self.mark_as_used_in_outer_join(leftalias) - rchain += lchain for alias, (aouter, aconditions, achain) in outer_tables.iteritems(): if achain is lchain: outer_tables[alias] = (aouter, aconditions, rchain) + else: + raise BadRQLQuery() # sql generation helpers ################################################### diff -r f627ab500fda -r 5ad3154a8810 server/test/unittest_rql2sql.py --- a/server/test/unittest_rql2sql.py Mon May 02 20:41:46 2011 +0200 +++ b/server/test/unittest_rql2sql.py Tue May 10 18:50:13 2011 +0200 @@ -835,9 +835,9 @@ WHERE _X.cw_eid=12''' ), ("Any P WHERE X eid 12, P? concerne X, X todo_by S", - '''SELECT rel_concerne0.eid_from -FROM todo_by_relation AS rel_todo_by1 LEFT OUTER JOIN concerne_relation AS rel_concerne0 ON (rel_concerne0.eid_to=12) -WHERE rel_todo_by1.eid_from=12''' + '''SELECT rel_concerne1.eid_from +FROM todo_by_relation AS rel_todo_by0 LEFT OUTER JOIN concerne_relation AS rel_concerne1 ON (rel_concerne1.eid_to=12) +WHERE rel_todo_by0.eid_from=12''' ), ('Any GN, TN ORDERBY GN WHERE T tags G?, T name TN, G name GN', @@ -931,6 +931,10 @@ WHERE _S.cw_ambiguous_inlined=_A.cw_eid) AS _T0 ON (_X.cw_multisource_inlined_rel=_T0.C0)''' ), + ('Any X,T,OT WHERE X tags T, OT? tags X, X is Tag, X eid 123', + '''SELECT rel_tags0.eid_from, rel_tags0.eid_to, rel_tags1.eid_from +FROM tags_relation AS rel_tags0 LEFT OUTER JOIN tags_relation AS rel_tags1 ON (rel_tags1.eid_to=123) +WHERE rel_tags0.eid_from=123'''), ] VIRTUAL_VARS = [ @@ -1601,7 +1605,7 @@ '''SELECT 1 WHERE NOT (EXISTS(SELECT 1 FROM in_group_relation AS rel_in_group0))''') - def test_nonregr_subquery_missing_join(self): + def test_nonregr_outer_join_multiple(self): self._check('Any COUNT(P1148),G GROUPBY G ' 'WHERE G owned_by D, D eid 1122, K1148 bookmarked_by P1148, ' 'K1148 eid 1148, P1148? in_group G', @@ -1611,7 +1615,7 @@ GROUP BY _G.cw_eid''' ) - def test_nonregr_subquery_missing_join2(self): + def test_nonregr_outer_join_multiple2(self): self._check('Any COUNT(P1148),G GROUPBY G ' 'WHERE G owned_by D, D eid 1122, K1148 bookmarked_by P1148?, ' 'K1148 eid 1148, P1148? in_group G',